Skip to content

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

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 24 additions & 6 deletions src/QLPlan.actor.h
Original file line number Diff line number Diff line change
Expand Up @@ -241,15 +241,33 @@ struct IndexScanPlan : ConcretePlan<IndexScanPlan> {
std::vector<std::string> matchedPrefix)
: cx(cx), index(index), begin(begin), end(end), matchedPrefix(matchedPrefix) {}
bson::BSONObj describe() override {
std::string bound_begin = begin.present() ? FDB::printable(begin.get()) : "-inf";
std::string bound_end = end.present() ? FDB::printable(end.get()) : "+inf";
// #17: Added decode_bytes and decode_key_part to update explain output with user readable keys

std::string bound_begin;
std::string bound_end;
bson::BSONObjBuilder bound;
size_t pos;

DataKey begin_key = DataKey::decode_bytes(begin.get());
DataKey end_key = DataKey::decode_bytes(end.get());

for (int i = 0; i < begin_key.size(); ++i) {
bound_begin = DataValue::decode_key_part(begin_key[i]).toString();
bound_end = DataValue::decode_key_part(end_key[i]).toString();
if ((DVTypeCode)end_key[i][0] == DVTypeCode::SPL_CHAR) {
pos = bound_end.find('-', 0);
if (pos != std::string::npos) {
bound_end.erase(pos, 1);
}
}
bound.append(matchedPrefix[i], BSON("begin" << bound_begin << "end" << bound_end));
}

return BSON(
// clang-format off
"type" << "index scan" <<
"index name" << index->indexName <<
"bounds" << BSON(
"begin" << bound_begin <<
"end" << bound_end)
"bounds" << bound.obj()
// clang-format on
);
}
Expand Down Expand Up @@ -708,4 +726,4 @@ ACTOR Future<std::pair<int64_t, Reference<ScanReturnedContext>>> executeUntilCom
Reference<Plan> deletePlan(Reference<Plan> subPlan, Reference<UnboundCollectionContext> cx, int64_t limit);
Reference<Plan> flushChanges(Reference<Plan> subPlan);

#endif /* _QL_PLAN_ACTOR_H_ */
#endif /* _QL_PLAN_ACTOR_H_ */
8 changes: 5 additions & 3 deletions src/QLTypes.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ DVTypeCode DataValue::getSortType() const {
bson::BSONType DataValue::getBSONType() const {
switch (getSortType()) {
case DVTypeCode::NUMBER:
case DVTypeCode::SPL_CHAR:
return (bson::BSONType)representation[11];
case DVTypeCode::STRING:
return bson::BSONType::String;
Expand Down Expand Up @@ -152,7 +153,7 @@ DataValue DataValue::decode_key_part(StringRef key) {
try {
if (type == DVTypeCode::STRING || type == DVTypeCode::PACKED_ARRAY || type == DVTypeCode::PACKED_OBJECT) {
return DataValue(StringRef(unescape_nulls(key)));
} else if (type == DVTypeCode::NUMBER) {
} else if (type == DVTypeCode::NUMBER || type == DVTypeCode::SPL_CHAR) {
return decode_key_part(key, bson::BSONType::NumberDouble);
} else {
return DataValue(key);
Expand All @@ -164,7 +165,7 @@ DataValue DataValue::decode_key_part(StringRef key) {
}

DataValue DataValue::decode_key_part(StringRef numKey, bson::BSONType numCode) {
if ((DVTypeCode)numKey[0] == DVTypeCode::NUMBER) {
if ((DVTypeCode)numKey[0] == DVTypeCode::NUMBER || (DVTypeCode)numKey[0] == DVTypeCode::SPL_CHAR) {
if (numCode == bson::BSONType::NumberInt || numCode == bson::BSONType::NumberLong ||
numCode == bson::BSONType::NumberDouble) {
Standalone<StringRef> s = makeString(12);
Expand Down Expand Up @@ -530,7 +531,7 @@ int64_t DataValue::getLong() const {

double DataValue::getDouble() const {
ASSERT(representation.size() == 12);
ASSERT(getSortType() == DVTypeCode::NUMBER);
ASSERT((getSortType() == DVTypeCode::NUMBER) || (getSortType() == DVTypeCode::SPL_CHAR));
ASSERT(representation[11] == 1);

LongDouble r;
Expand Down Expand Up @@ -627,6 +628,7 @@ static size_t find_string_terminator(const uint8_t* bytes, size_t max_length_plu
static int getKeyPartLength(const uint8_t* ptr, size_t max_length_plus_one) {
switch (DVTypeCode(*ptr)) {
case DVTypeCode::NUMBER:
case DVTypeCode::SPL_CHAR:
return 11;

case DVTypeCode::STRING:
Expand Down
8 changes: 8 additions & 0 deletions src/QLTypes.h
Original file line number Diff line number Diff line change
Expand Up @@ -80,11 +80,19 @@ typedef struct LongDouble final {
#else
typedef long double LongDouble;
#endif
// #17: Added decode_bytes and decode_key_part to update explain output with user readable keys

/* In case query has '$gt' or '$lt' operator then field elements are represent in range,
* like '//x01e < x < 1' (for '$lt') and '1 < x < //x01f' (for '$gt'), where //x01e represent.
* To determine those bytes, enable a check for '0x1f' in 'DVTypeCode' as SPL_CHAR,
* for '0x1e' in 'DVTypeCode' already enabled as 'NUMBER'.
*/

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?

STRING = 40,
OBJECT = 50,
PACKED_OBJECT = 51,
Expand Down
92 changes: 92 additions & 0 deletions test/correctness/smoke/test_planner.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,28 @@ def only_index_named(index_name, explanation):
else:
return False

#issue-17: Added decode_bytes and decode_key_part to update explain output with user readable keys
# To check begin and end keys in bounds in explanation
@staticmethod
def check_bound(index_name, begin, end, explanation):
this_type = explanation['type']
if this_type == 'union':
return all([Predicates.check_bound(index_name, begin, end, p) for p in explanation['plans']])
elif 'source_plan' in explanation:
return Predicates.check_bound(index_name, begin, end, explanation['source_plan'])
elif index_name in explanation['bounds']:
bound = explanation['bounds'][index_name]
if bound['begin'] == begin and bound['end'] == end:
return True
elif bound['begin'] == begin and bound['end'] == 'nan.0':
# This check needed because in MacOS decode byte '0x1f' returns 'nan.0' instead of 'inf.0'.
return True
else:
return False
else:
return False


@staticmethod
def pk_lookup(explanation):
this_type = explanation['type']
Expand Down Expand Up @@ -437,3 +459,73 @@ def test_compound_index_multi_match(fixture_collection):
ret = fixture_collection.find(query).explain()
assert Predicates.only_index_named('da', ret['explanation'])
assert Predicates.no_table_scan(ret['explanation'])

#issue-17: Added decode_bytes and decode_key_part to update explain output with user readable keys
# check bound range test
def test_bounds_in_basic_query_explain(fixture_collection):
fixture_collection.create_index([('a', 1), ('b', 1)])
query = {
'a' : 'A',
'b' : 64
}
ret = fixture_collection.find(query).explain()
# Predicates.check_bound(index_name, begin_key, end_key, explanation)

# Check whether, index name 'a', begin key 'A' and end key in 'A' are present in explanation
assert Predicates.check_bound('a', '"A"', '"A"', ret['explanation'])
# Check whether, index name 'b', begin key '64.0' and end key in '64.0' are present in explanation
assert Predicates.check_bound('b', '64.0', '64.0', ret['explanation'])


def test_bounds_in_operator_query_explain(fixture_collection):
fixture_collection.create_index(keys=[('d', 1), ('b', 1), ('c', 1)], name='compound')
fixture_collection.create_index(keys=[('d', 1)], name='simple')
query = {
'$and':
[{'d':
{'$gt': 1}
}
]}
ret = fixture_collection.find(query).explain()
explanation = ret['explanation']
# Predicates.check_bound(index_name, begin_key, end_key, explanation)

# Check whether, index name 'd', begin key '1.0' and end key in 'inf.0' are present in explanation
# For range 1.0 < x < inf.0
assert Predicates.check_bound('d', '1.0', 'inf.0', ret['explanation'])

query = {
'$and':
[{'d':
{'$lt': 1}
}
]}
ret = fixture_collection.find(query).explain()
explanation = ret['explanation']
# Check whether, index name 'd', begin key '-inf.0' and end key in '1.0' are present in explanation
# For range -inf.0 < x < 1.0
assert Predicates.check_bound('d', '-inf.0', '1.0', ret['explanation'])

query = {
'$and':
[{'d':
{'$lt': -5}
}
]}
ret = fixture_collection.find(query).explain()
explanation = ret['explanation']
# Check whether, index name 'd', begin key '-inf.0' and end key in '-5.0' are present in explanation
# For range -inf.0 < x < -5.0
assert Predicates.check_bound('d', '-inf.0', '-5.0', ret['explanation'])

query = {
'$and':
[{'d':
{'$gt': -5}
}
]}
ret = fixture_collection.find(query).explain()
explanation = ret['explanation']
# Check whether, index name 'd', begin key '-5.0' and end key in 'inf.0' are present in explanation
# For range -5.0 < x < inf.0
assert Predicates.check_bound('d', '-5.0', 'inf.0', ret['explanation'])