Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resolves #17: Update explain() output with user readable keys #208

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

drkannan
Copy link
Contributor

This PR resolves #17

Explain() output will be much more useful if the keys are user readable, instead of FDB keys. Used the existing decode_key_part function to achieve this.

@fdb-build
Copy link

Can one of the admins verify this patch?

3 similar comments
@fdb-build
Copy link

Can one of the admins verify this patch?

@fdb-build
Copy link

Can one of the admins verify this patch?

@fdb-build
Copy link

Can one of the admins verify this patch?

@senthil-db-expert
Copy link

@apkar could you please verify this patch

@apkar
Copy link
Contributor

apkar commented Aug 29, 2019

@fdb-build test this please

@apkar apkar self-requested a review August 29, 2019 17:24
@apkar apkar self-assigned this Aug 29, 2019
std::string bound_end = end.present() ? FDB::printable(end.get()) : "+inf";
// #17: Added decode_key_part to update explain output with user readable keys
std::string bound_begin = begin.present() ? DataValue::decode_key_part(begin.get()).toString() : "-inf";
std::string bound_end = end.present() ? DataValue::decode_key_part(begin.get()).toString() : "+inf";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be end.get() ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, this logic would work if there is only one dimension in the index. As decode_key_part assumes there is only one element in the key. It checks type on the first byte only. If you have a compound index, then the key could be multi element tuple and this would fail.

For example, following explain() would fail.

db.coll.create_index([('a', 1), ('b', 1)])
db.coll.find({'a': 'A', 'b': 64}).explain()

You can use decode_bytes() to create DataKey. Once you have DataKey, you can iterate over different elements in the key to print them by their type.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @apkar . Will look into it.

@AlvinMooreSr
Copy link
Contributor

@fdb-build test this please

@apkar
Copy link
Contributor

apkar commented Sep 6, 2019

@fdb-build test this please

@apkar
Copy link
Contributor

apkar commented Sep 6, 2019

@senthil-db-expert Can you give some explanation on the usage of DVTypeCode::SPL_CHAR please? Also can you add some tests please? test_planner.py is probably the best place.

@senthil-db-expert
Copy link

@apkar Added explanations and test cases.


enum class DVTypeCode : uint8_t {
MIN_KEY = 0,
NULL_ELEMENT = 20,
NUMBER = 30,
SPL_CHAR = 31, // ASCII value for SPL_CHAR = 31 is '\0x1f'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Predicates like $gt, $gte, $lt, $lte etc.. use RANGE type predicate. To create a unended bound we create element with type code + 1, to match everything in that range or vice versa. Range can be on any type, not necessary just on numbers. For example, this code breaks on a query like

db.coll.find({'a': {'$gt': 'hello'}}).explain()

Which is a valid query. You can look at the following code to get better understanding of how ranges are created with one bound.

https://github.com/FoundationDB/fdb-document-layer/blob/master/src/ExtOperator.actor.cpp#L72

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@apkar We think RANGE would have some limitations for end key calculations. In Explain, we will not be able to give detailed output for inputs like $gt 10, $gt abc etc... because RANGE adds +1 to the typecode and returns as string. Whereas, if we add typecode + 1 for each of the enum in DVTypeCode, then we can handle end limit for any type and also the decode_bytes would work as expected. This will be similar to OBJECT-PACKED_OBJECT and ARRAY-PACKED_ARRAY. What do you think?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

explain() response should have user readable keys
5 participants