3309 Ole John Aske 2010-10-11
spj-svs: Added missing handling NULL values in pruned scan keys.
Pruned keys may be constructed from linkedValues which may contain a NULL value. As a
pruned key implies equality on the fields in the key, a NULL value in the prune key can
never match any existing tuple on the datanodes.
We can therefore eliminate any pruned SCANREQ with NULL value in the prune key.
In addition to the fix above, this commit also adds:
- Checking for errors returns from ::expand().
- add ndbrequire(!hasNull) to those places where we do not expect (and does not handle)
NULL values in the key produced by ::expand().
- Some more DEBUG output for debugging pruned scans.
- Moves the 'prune pattern' (SI_PRUNE_PATTERN) to be the last part of the 'optional'
part of a Qserialized ueryTree node - Required as 'parent' relationship for
the node should be established by ::parseDA() before we could ::expand()
the pattern for the pruned key.
modified:
mysql-test/suite/ndb/r/ndb_join_pushdown.result
mysql-test/suite/ndb/t/ndb_join_pushdown.test
storage/ndb/src/kernel/blocks/dbspj/Dbspj.hpp
storage/ndb/src/kernel/blocks/dbspj/DbspjMain.cpp
storage/ndb/src/ndbapi/NdbQueryBuilder.cpp
3308 Ole John Aske 2010-10-11
spj-svs: Fixed an undefined memory read
Fixed a situation where an empty 'm_spjProjection' will cause
NdbQueryOperationDefImpl::appendChildProjection() to set the QueryTree flag 'NI_LINKED_ATTR' without
appending a projection list to the serialized query tree.
This fix will ensure that a serialized m_spjProjection list with size==0 will be included in these cases.
No testcase as I can't think of any deterministic ways to make a testcase for an undefined memory read....
modified:
storage/ndb/src/ndbapi/NdbQueryBuilder.cpp
=== modified file 'mysql-test/suite/ndb/r/ndb_join_pushdown.result'
--- a/mysql-test/suite/ndb/r/ndb_join_pushdown.result 2010-10-11 09:48:36 +0000
+++ b/mysql-test/suite/ndb/r/ndb_join_pushdown.result 2010-10-11 12:11:05 +0000
@@ -3701,8 +3701,8 @@ pk u a b pk u a b pk u a b
drop table t1;
create table t1(
d int not null,
-e int not null,
-f int not null,
+e int null,
+f int null,
a int not null,
b int not null,
c int not null,
@@ -3883,8 +3883,32 @@ select straight_join * from t1 x, t1 y w
id select_type table type possible_keys key key_len ref rows Extra
1 SIMPLE x ALL PRIMARY NULL NULL NULL 8 Parent of 2 pushed join@1
1 SIMPLE y ref ix1 ix1 4 test.x.e 1 Child of pushed join@1; Using where
+insert into t1(a,b,c,d,e,f) values
+(8, 9, 0, 1, NULL, 3),
+(9, 9, 0, 1, 2, NULL);
+alter table t1 partition by key (b);
+select straight_join * from t1 x, t1 y where y.a=x.d and y.b=x.e;
+d e f a b c d e f a b c
+1 2 3 1 2 3 1 2 3 1 2 3
+1 2 3 1 2 3 1 2 3 1 2 4
+1 2 3 1 2 4 1 2 3 1 2 3
+1 2 3 1 2 4 1 2 3 1 2 4
+1 2 3 2 3 4 1 2 3 1 2 3
+1 2 3 2 3 4 1 2 3 1 2 4
+1 2 3 3 4 5 1 2 3 1 2 3
+1 2 3 3 4 5 1 2 3 1 2 4
+1 2 3 4 5 6 1 2 3 1 2 3
+1 2 3 4 5 6 1 2 3 1 2 4
+1 2 3 5 6 7 1 2 3 1 2 3
+1 2 3 5 6 7 1 2 3 1 2 4
+1 2 3 6 7 8 1 2 3 1 2 3
+1 2 3 6 7 8 1 2 3 1 2 4
+1 2 3 7 8 9 1 2 3 1 2 3
+1 2 3 7 8 9 1 2 3 1 2 4
+1 2 NULL 9 9 0 1 2 3 1 2 3
+1 2 NULL 9 9 0 1 2 3 1 2 4
pruned
-12
+14
const_pruned
6
drop table t1;
@@ -4005,13 +4029,13 @@ and spj_counts_at_end.counter_name <> 'L
and spj_counts_at_end.counter_name <> 'SCAN_BATCHES_RETURNED';
counter_name spj_counts_at_end.val - spj_counts_at_startup.val
CONST_PRUNED_RANGE_SCANS_RECEIVED 6
-LOCAL_TABLE_SCANS_SENT 192
-PRUNED_RANGE_SCANS_RECEIVED 14
+LOCAL_TABLE_SCANS_SENT 194
+PRUNED_RANGE_SCANS_RECEIVED 16
RANGE_SCANS_RECEIVED 195
READS_NOT_FOUND 403
READS_RECEIVED 61
-SCAN_ROWS_RETURNED 63563
-TABLE_SCANS_RECEIVED 192
+SCAN_ROWS_RETURNED 63591
+TABLE_SCANS_RECEIVED 194
select sum(spj_counts_at_end.val - spj_counts_at_startup.val) as 'LOCAL+REMOTE READS_SENT'
from spj_counts_at_end, spj_counts_at_startup
where spj_counts_at_end.counter_name = spj_counts_at_startup.counter_name
@@ -4022,15 +4046,15 @@ LOCAL+REMOTE READS_SENT
drop table spj_counts_at_startup;
drop table spj_counts_at_end;
scan_count
-1985
+1990
pruned_scan_count
7
sorted_scan_count
44
pushed_queries_defined
-334
+335
pushed_queries_dropped
11
pushed_queries_executed
-258
+259
set ndb_join_pushdown = @save_ndb_join_pushdown;
=== modified file 'mysql-test/suite/ndb/t/ndb_join_pushdown.test'
--- a/mysql-test/suite/ndb/t/ndb_join_pushdown.test 2010-10-11 09:48:36 +0000
+++ b/mysql-test/suite/ndb/t/ndb_join_pushdown.test 2010-10-11 12:11:05 +0000
@@ -2708,8 +2708,8 @@ drop table t1;
# Test pruned index scan:
create table t1(
d int not null,
- e int not null,
- f int not null,
+ e int null,
+ f int null,
a int not null,
b int not null,
c int not null,
@@ -2784,6 +2784,17 @@ create index ix1 on t1(b,d,a);
explain
select straight_join * from t1 x, t1 y where y.a=x.d and y.b=x.e;
+###########
+# Partition keys may evaluate to NULL-values:
+###########
+insert into t1(a,b,c,d,e,f) values
+ (8, 9, 0, 1, NULL, 3),
+ (9, 9, 0, 1, 2, NULL);
+
+alter table t1 partition by key (b);
+--sorted_result
+select straight_join * from t1 x, t1 y where y.a=x.d and y.b=x.e;
+
# Verify pruned execution by comparing the NDB$INFO counters
--disable_query_log
--eval select sum(val) - $pruned_range AS pruned from ndbinfo.counters where block_name='DBSPJ' and counter_name='PRUNED_RANGE_SCANS_RECEIVED'
=== modified file 'storage/ndb/src/kernel/blocks/dbspj/Dbspj.hpp'
--- a/storage/ndb/src/kernel/blocks/dbspj/Dbspj.hpp 2010-09-24 08:46:53 +0000
+++ b/storage/ndb/src/kernel/blocks/dbspj/Dbspj.hpp 2010-10-11 12:11:05 +0000
@@ -1067,8 +1067,8 @@ private:
DABuffer & pattern, Uint32 len,
DABuffer & param, Uint32 cnt);
Uint32 parseDA(Build_context&, Ptr<Request>, Ptr<TreeNode>,
- DABuffer tree, Uint32 treeBits,
- DABuffer param, Uint32 paramBits);
+ DABuffer & tree, Uint32 treeBits,
+ DABuffer & param, Uint32 paramBits);
Uint32 createEmptySection(Uint32 & ptrI);
=== modified file 'storage/ndb/src/kernel/blocks/dbspj/DbspjMain.cpp'
--- a/storage/ndb/src/kernel/blocks/dbspj/DbspjMain.cpp 2010-10-04 17:00:36 +0000
+++ b/storage/ndb/src/kernel/blocks/dbspj/DbspjMain.cpp 2010-10-11 12:11:05 +0000
@@ -3189,7 +3189,7 @@ Dbspj::lookup_parent_row(Signal* signal,
{
releaseSection(ptrI);
}
- return; // Bailout, KEYREQ would have returned KEYREF(626)
+ return; // Bailout, KEYREQ would have returned KEYREF(626) anyway
}
else // isLookup()
{
@@ -3204,7 +3204,8 @@ Dbspj::lookup_parent_row(Signal* signal,
*/
jam();
}
- }
+ } // keyIsNull
+
/**
* NOTE:
* The logic below contradicts 'keyIsNull' logic above and should
@@ -4319,6 +4320,11 @@ Dbspj::parseScanIndex(Build_context& ctx
data.m_frags_outstanding = 0;
data.m_frags_not_complete = 0;
+ err = parseDA(ctx, requestPtr, treeNodePtr,
+ tree, treeBits, param, paramBits);
+ if (unlikely(err != 0))
+ break;
+
if (treeBits & Node::SI_PRUNE_PATTERN)
{
Uint32 len_cnt = * tree.ptr ++;
@@ -4332,6 +4338,7 @@ Dbspj::parseScanIndex(Build_context& ctx
if (treeBits & Node::SI_PRUNE_LINKED)
{
jam();
+ DEBUG("LINKED-PRUNE PATTERN w/ " << cnt << " PARAM values");
data.m_prunePattern.init();
Local_pattern_store pattern(pool, data.m_prunePattern);
@@ -4340,12 +4347,17 @@ Dbspj::parseScanIndex(Build_context& ctx
* Expand pattern into a new pattern (with linked values)
*/
err = expand(pattern, treeNodePtr, tree, len, param, cnt);
+ if (unlikely(err != 0))
+ break;
+
treeNodePtr.p->m_bits |= TreeNode::T_PRUNE_PATTERN;
c_Counters.incr_counter(CI_PRUNED_RANGE_SCANS_RECEIVED, 1);
}
else
{
jam();
+ DEBUG("FIXED-PRUNE w/ " << cnt << " PARAM values");
+
/**
* Expand pattern directly into
* This means a "fixed" pruning from here on
@@ -4354,9 +4366,20 @@ Dbspj::parseScanIndex(Build_context& ctx
Uint32 prunePtrI = RNIL;
bool hasNull;
err = expand(prunePtrI, tree, len, param, cnt, hasNull);
- data.m_constPrunePtrI = prunePtrI;
+ if (unlikely(err != 0))
+ break;
-// ndbrequire(!hasNull); /* todo: can we take advantage of NULLs in range scan? */
+ if (unlikely(hasNull))
+ {
+ /* API should have elliminated requests w/ const-NULL keys */
+ jam();
+ DEBUG("BEWARE: T_CONST_PRUNE-key contain NULL values");
+// treeNodePtr.p->m_bits |= TreeNode::T_NULL_PRUNE;
+// break;
+ ndbrequire(false);
+ }
+ ndbrequire(prunePtrI != RNIL); /* todo: can we allow / take advantage of NULLs in range scan? */
+ data.m_constPrunePtrI = prunePtrI;
/**
* We may not compute the partition for the hash-key here
@@ -4365,7 +4388,7 @@ Dbspj::parseScanIndex(Build_context& ctx
treeNodePtr.p->m_bits |= TreeNode::T_CONST_PRUNE;
c_Counters.incr_counter(CI_CONST_PRUNED_RANGE_SCANS_RECEIVED, 1);
}
- }
+ } //SI_PRUNE_PATTERN
if ((treeNodePtr.p->m_bits & TreeNode::T_CONST_PRUNE) == 0 &&
((treeBits & Node::SI_PARALLEL) ||
@@ -4375,8 +4398,7 @@ Dbspj::parseScanIndex(Build_context& ctx
treeNodePtr.p->m_bits |= TreeNode::T_SCAN_PARALLEL;
}
- return parseDA(ctx, requestPtr, treeNodePtr,
- tree, treeBits, param, paramBits);
+ return 0;
} while(0);
DEBUG_CRASH();
@@ -4659,7 +4681,19 @@ Dbspj::scanIndex_parent_row(Signal* sign
DEBUG_CRASH();
break;
}
-// ndbrequire(!hasNull);
+
+ if (unlikely(hasNull))
+ {
+ jam();
+ DEBUG("T_PRUNE_PATTERN-key contain NULL values");
+
+ // Ignore this request as 'NULL == <column>' will never give a match
+ if (pruneKeyPtrI != RNIL)
+ {
+ releaseSection(pruneKeyPtrI);
+ }
+ return; // Bailout, SCANREQ would have returned 0 rows anyway
+ }
// TODO we need a different variant of computeHash here,
// since pruneKeyPtrI does not contain full primary key
@@ -4741,7 +4775,7 @@ Dbspj::scanIndex_parent_row(Signal* sign
// Fixed key...fix later...
ndbrequire(false);
}
-// ndbrequire(!hasNull);
+// ndbrequire(!hasNull); // FIXME, can't ignore request as we already added it to keyPattern
fragPtr.p->m_rangePtrI = ptrI;
scanIndex_fixupBound(fragPtr, ptrI, rowRef.m_src_correlation);
@@ -6296,8 +6330,8 @@ Uint32
Dbspj::parseDA(Build_context& ctx,
Ptr<Request> requestPtr,
Ptr<TreeNode> treeNodePtr,
- DABuffer tree, Uint32 treeBits,
- DABuffer param, Uint32 paramBits)
+ DABuffer& tree, Uint32 treeBits,
+ DABuffer& param, Uint32 paramBits)
{
Uint32 err;
Uint32 attrInfoPtrI = RNIL;
@@ -6417,7 +6451,15 @@ Dbspj::parseDA(Build_context& ctx,
bool hasNull;
Uint32 keyInfoPtrI = RNIL;
err = expand(keyInfoPtrI, tree, len, param, cnt, hasNull);
-// ndbrequire(!hasNull);
+ if (unlikely(hasNull))
+ {
+ /* API should have elliminated requests w/ const-NULL keys */
+ jam();
+ DEBUG("BEWARE: FIXED-key contain NULL values");
+// treeNodePtr.p->m_bits |= TreeNode::T_NULL_PRUNE;
+// break;
+ ndbrequire(false);
+ }
treeNodePtr.p->m_send.m_keyInfoPtrI = keyInfoPtrI;
}
@@ -6529,7 +6571,11 @@ Dbspj::parseDA(Build_context& ctx,
m_dependency_map_pool);
Local_pattern_store pattern(pool,treeNodePtr.p->m_attrParamPattern);
err = expand(pattern, treeNodePtr, tree, len_pattern, param, cnt);
-
+ if (unlikely(err))
+ {
+ DEBUG_CRASH();
+ break;
+ }
/**
* This node constructs a new attr-info for each send
*/
@@ -6544,6 +6590,11 @@ Dbspj::parseDA(Build_context& ctx,
*/
bool hasNull;
err = expand(attrParamPtrI, tree, len_pattern, param, cnt, hasNull);
+ if (unlikely(err))
+ {
+ DEBUG_CRASH();
+ break;
+ }
// ndbrequire(!hasNull);
}
}
=== modified file 'storage/ndb/src/ndbapi/NdbQueryBuilder.cpp'
--- a/storage/ndb/src/ndbapi/NdbQueryBuilder.cpp 2010-10-11 09:55:24 +0000
+++ b/storage/ndb/src/ndbapi/NdbQueryBuilder.cpp 2010-10-11 12:11:05 +0000
@@ -2511,9 +2511,6 @@ NdbQueryScanOperationDefImpl::serialize(
serializedDef.alloc(QN_ScanFragNode::NodeSize);
Uint32 requestInfo = 0;
- // Optional prefix part0: Pattern to creating a prune key for range scan
- requestInfo |= appendPrunePattern(serializedDef);
-
// Optional part1: Make list of parent nodes.
requestInfo |= appendParentList (serializedDef);
@@ -2523,6 +2520,9 @@ NdbQueryScanOperationDefImpl::serialize(
// Part3: Columns required by SPJ to instantiate descendant child operations.
requestInfo |= appendChildProjection(serializedDef);
+ // Part4: Pattern to creating a prune key for range scan
+ requestInfo |= appendPrunePattern(serializedDef);
+
const Uint32 length = serializedDef.getSize() - startPos;
if (unlikely(length > 0xFFFF))
{
Attachment: [text/bzr-bundle] bzr/ole.john.aske@oracle.com-20101011121105-t18b2uru9vxxowha.bundle
| Thread |
|---|
| • bzr push into mysql-5.1-telco-7.0-spj-scan-vs-scan branch(ole.john.aske:3308 to 3309) | Ole John Aske | 11 Oct |
