List:Commits« Previous MessageNext Message »
From:Ole John Aske Date:October 11 2010 12:11pm
Subject:bzr commit into mysql-5.1-telco-7.0-spj-scan-vs-scan branch
(ole.john.aske:3309)
View as plain text  
#At file:///net/fimafeng09/export/home/tmp/oleja/mysql/mysql-5.1-telco-7.0-spj-scan-scan/ based on revid:ole.john.aske@stripped

 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
=== 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 commit into mysql-5.1-telco-7.0-spj-scan-vs-scan branch(ole.john.aske:3309) Ole John Aske11 Oct