From 9694b36455f48ecfb163dd8bed5412723dbdb230 Mon Sep 17 00:00:00 2001 From: Kannan Dorairaj Date: Mon, 26 Aug 2019 13:28:17 +0530 Subject: [PATCH 1/4] Resolves #17: Update explain() output with user readable keys --- src/QLPlan.actor.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/QLPlan.actor.h b/src/QLPlan.actor.h index 6b522ab..6eeee42 100644 --- a/src/QLPlan.actor.h +++ b/src/QLPlan.actor.h @@ -241,8 +241,8 @@ struct IndexScanPlan : ConcretePlan { std::vector 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"; + 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"; return BSON( // clang-format off "type" << "index scan" << @@ -708,4 +708,4 @@ ACTOR Future>> executeUntilCom Reference deletePlan(Reference subPlan, Reference cx, int64_t limit); Reference flushChanges(Reference subPlan); -#endif /* _QL_PLAN_ACTOR_H_ */ \ No newline at end of file +#endif /* _QL_PLAN_ACTOR_H_ */ From d423bd5af8d2838314700bfca15193c98ff980fe Mon Sep 17 00:00:00 2001 From: Kannan Dorairaj Date: Mon, 26 Aug 2019 15:25:14 +0530 Subject: [PATCH 2/4] Added comment in modified code --- src/QLPlan.actor.h | 1 + 1 file changed, 1 insertion(+) diff --git a/src/QLPlan.actor.h b/src/QLPlan.actor.h index 6eeee42..5b0a32f 100644 --- a/src/QLPlan.actor.h +++ b/src/QLPlan.actor.h @@ -241,6 +241,7 @@ struct IndexScanPlan : ConcretePlan { std::vector matchedPrefix) : cx(cx), index(index), begin(begin), end(end), matchedPrefix(matchedPrefix) {} bson::BSONObj describe() override { + // #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"; return BSON( From eb13a6cba042e351323c2dd22f15a5cf6a19e71d Mon Sep 17 00:00:00 2001 From: Kannan Dorairaj Date: Fri, 6 Sep 2019 11:35:09 +0530 Subject: [PATCH 3/4] Addressed review comments --- src/QLPlan.actor.h | 24 +++++++++++++++++++----- src/QLTypes.cpp | 8 +++++--- src/QLTypes.h | 1 + 3 files changed, 25 insertions(+), 8 deletions(-) diff --git a/src/QLPlan.actor.h b/src/QLPlan.actor.h index 5b0a32f..f925cc5 100644 --- a/src/QLPlan.actor.h +++ b/src/QLPlan.actor.h @@ -242,15 +242,29 @@ struct IndexScanPlan : ConcretePlan { : cx(cx), index(index), begin(begin), end(end), matchedPrefix(matchedPrefix) {} bson::BSONObj describe() override { // #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"; + 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(); + 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 ); } diff --git a/src/QLTypes.cpp b/src/QLTypes.cpp index 1416951..d70ec8d 100644 --- a/src/QLTypes.cpp +++ b/src/QLTypes.cpp @@ -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; @@ -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); @@ -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 s = makeString(12); @@ -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; @@ -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: diff --git a/src/QLTypes.h b/src/QLTypes.h index 208d376..f699838 100644 --- a/src/QLTypes.h +++ b/src/QLTypes.h @@ -85,6 +85,7 @@ enum class DVTypeCode : uint8_t { MIN_KEY = 0, NULL_ELEMENT = 20, NUMBER = 30, + SPL_CHAR = 31, STRING = 40, OBJECT = 50, PACKED_OBJECT = 51, From 901c10a3ea1c73b03e8e04de9cd5db8d2e0bbd8d Mon Sep 17 00:00:00 2001 From: Kannan Dorairaj Date: Wed, 11 Sep 2019 11:32:29 +0530 Subject: [PATCH 4/4] Addressed review comments --- src/QLPlan.actor.h | 11 +-- src/QLTypes.h | 9 ++- test/correctness/smoke/test_planner.py | 92 ++++++++++++++++++++++++++ 3 files changed, 107 insertions(+), 5 deletions(-) diff --git a/src/QLPlan.actor.h b/src/QLPlan.actor.h index f925cc5..979029e 100644 --- a/src/QLPlan.actor.h +++ b/src/QLPlan.actor.h @@ -241,7 +241,8 @@ struct IndexScanPlan : ConcretePlan { std::vector matchedPrefix) : cx(cx), index(index), begin(begin), end(end), matchedPrefix(matchedPrefix) {} bson::BSONObj describe() override { - // #17: Added decode_key_part to update explain output with user readable keys + // #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; @@ -253,9 +254,11 @@ struct IndexScanPlan : ConcretePlan { 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(); - pos = bound_end.find('-', 0); - if (pos != std::string::npos) { - bound_end.erase(pos, 1); + 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)); } diff --git a/src/QLTypes.h b/src/QLTypes.h index f699838..c47e84c 100644 --- a/src/QLTypes.h +++ b/src/QLTypes.h @@ -80,12 +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, + SPL_CHAR = 31, // ASCII value for SPL_CHAR = 31 is '\0x1f' STRING = 40, OBJECT = 50, PACKED_OBJECT = 51, diff --git a/test/correctness/smoke/test_planner.py b/test/correctness/smoke/test_planner.py index 9463a92..c8c3deb 100644 --- a/test/correctness/smoke/test_planner.py +++ b/test/correctness/smoke/test_planner.py @@ -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'] @@ -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'])