From: Date: March 19 2008 3:11pm Subject: bk commit into 5.1 tree (frazer:1.2544) BUG#35137 List-Archive: http://lists.mysql.com/commits/44229 X-Bug: 35137 Message-Id: <200803191411.m2JEB4lm032137@forth.ndb.mysql.com> Below is the list of changes that have just been committed into a local 5.1 repository of frazer. When frazer does a push these changes will be propagated to the main repository and, within 24 hours after the push, to the public repository. For information on how to access the public repository see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html ChangeSet@stripped, 2008-03-19 14:10:47+00:00, frazer@stripped +5 -0 Bug#35137 : Query crashed mysqld Nested MRR reads failed as the second MRR range released the first MRR range's unprocessed operations. Fix is to process all outstanding operations upfront before returning first result. New testcase added to ndb_read_multi_range include/my_base.h@stripped, 2008-03-19 14:10:20+00:00, frazer@stripped +1 -0 New per-range flag mysql-test/suite/ndb/r/ndb_read_multi_range.result@stripped, 2008-03-19 14:10:22+00:00, frazer@stripped +11 -0 New testcase for nested MRR mysql-test/suite/ndb/t/ndb_read_multi_range.test@stripped, 2008-03-19 14:10:28+00:00, frazer@stripped +13 -0 New testcase for nested MRR sql/ha_ndbcluster.cc@stripped, 2008-03-19 14:10:31+00:00, frazer@stripped +96 -44 Remove need to keep operation list for MRR iteration by reading into buffer up-front. Get rid of unnecessary operation-release-on-execute special case for MRR. Fix apparent issue with SKIP_RANGE and buffer increment in read_multi_range_next() sql/ha_ndbcluster.h@stripped, 2008-03-19 14:10:33+00:00, frazer@stripped +3 -4 Remove force flag from execute() methods, remove current MRR operation member diff -Nrup a/include/my_base.h b/include/my_base.h --- a/include/my_base.h 2008-02-05 13:41:55 +00:00 +++ b/include/my_base.h 2008-03-19 14:10:20 +00:00 @@ -500,6 +500,7 @@ enum data_file_type { #define NULL_RANGE 64 #define GEOM_FLAG 128 #define SKIP_RANGE 256 +#define EMPTY_RANGE 512 typedef struct st_key_range { diff -Nrup a/mysql-test/suite/ndb/r/ndb_read_multi_range.result b/mysql-test/suite/ndb/r/ndb_read_multi_range.result --- a/mysql-test/suite/ndb/r/ndb_read_multi_range.result 2007-08-21 13:32:26 +01:00 +++ b/mysql-test/suite/ndb/r/ndb_read_multi_range.result 2008-03-19 14:10:22 +00:00 @@ -492,3 +492,14 @@ id 3 drop trigger kaboom; drop table t1; +create table t1 (a int primary key, b int, key b_idx (b)) engine ndb; +insert into t1 values(1,1), (2,2), (3,3), (4,4), (5,5); +select one.a +from t1 one left join t1 two +on (two.b = one.b) +where one.a in (3, 4) +order by a; +a +3 +4 +drop table t1; diff -Nrup a/mysql-test/suite/ndb/t/ndb_read_multi_range.test b/mysql-test/suite/ndb/t/ndb_read_multi_range.test --- a/mysql-test/suite/ndb/t/ndb_read_multi_range.test 2007-08-21 13:32:26 +01:00 +++ b/mysql-test/suite/ndb/t/ndb_read_multi_range.test 2008-03-19 14:10:28 +00:00 @@ -338,3 +338,16 @@ select * from t2 order by id; drop trigger kaboom; drop table t1; + +#bug#35137 - self join + mrr + +create table t1 (a int primary key, b int, key b_idx (b)) engine ndb; +insert into t1 values(1,1), (2,2), (3,3), (4,4), (5,5); + +select one.a +from t1 one left join t1 two +on (two.b = one.b) +where one.a in (3, 4) +order by a; + +drop table t1; diff -Nrup a/sql/ha_ndbcluster.cc b/sql/ha_ndbcluster.cc --- a/sql/ha_ndbcluster.cc 2008-03-12 14:22:51 +00:00 +++ b/sql/ha_ndbcluster.cc 2008-03-19 14:10:31 +00:00 @@ -276,10 +276,9 @@ int execute_no_commit_ignore_no_key(ha_n } inline -int execute_no_commit(ha_ndbcluster *h, NdbTransaction *trans, - bool force_release) +int execute_no_commit(ha_ndbcluster *h, NdbTransaction *trans) { - h->release_completed_operations(trans, force_release); + h->release_completed_operations(trans); return h->m_ignore_no_key ? execute_no_commit_ignore_no_key(h,trans) : trans->execute(NdbTransaction::NoCommit, @@ -304,10 +303,9 @@ int execute_commit(THD *thd, NdbTransact } inline -int execute_no_commit_ie(ha_ndbcluster *h, NdbTransaction *trans, - bool force_release) +int execute_no_commit_ie(ha_ndbcluster *h, NdbTransaction *trans) { - h->release_completed_operations(trans, force_release); + h->release_completed_operations(trans); return trans->execute(NdbTransaction::NoCommit, NdbOperation::AO_IgnoreError, h->m_force_send); @@ -1792,7 +1790,7 @@ int ha_ndbcluster::pk_read(const uchar * ERR_RETURN(trans->getNdbError()); } - if ((res = execute_no_commit_ie(this,trans,FALSE)) != 0 || + if ((res = execute_no_commit_ie(this,trans)) != 0 || op->getNdbError().code) { table->status= STATUS_NOT_FOUND; @@ -1858,7 +1856,7 @@ int ha_ndbcluster::complemented_read(con } } - if (execute_no_commit(this,trans,FALSE) != 0) + if (execute_no_commit(this,trans) != 0) { table->status= STATUS_NOT_FOUND; DBUG_RETURN(ndb_err(trans)); @@ -2058,7 +2056,7 @@ int ha_ndbcluster::peek_indexed_rows(con } last= trans->getLastDefinedOperation(); if (first) - res= execute_no_commit_ie(this,trans,FALSE); + res= execute_no_commit_ie(this,trans); else { // Table has no keys @@ -2107,7 +2105,7 @@ int ha_ndbcluster::unique_index_read(con if ((res= define_read_attrs(buf, op))) DBUG_RETURN(res); - if (execute_no_commit_ie(this,trans,FALSE) != 0 || + if (execute_no_commit_ie(this,trans) != 0 || op->getNdbError().code) { int err= ndb_err(trans); @@ -2164,7 +2162,7 @@ inline int ha_ndbcluster::fetch_next(Ndb */ if (m_ops_pending && m_blobs_pending) { - if (execute_no_commit(this,trans,FALSE) != 0) + if (execute_no_commit(this,trans) != 0) DBUG_RETURN(ndb_err(trans)); m_ops_pending= 0; m_blobs_pending= FALSE; @@ -2196,7 +2194,7 @@ inline int ha_ndbcluster::fetch_next(Ndb { if (m_transaction_on) { - if (execute_no_commit(this,trans,FALSE) != 0) + if (execute_no_commit(this,trans) != 0) DBUG_RETURN(-1); } else @@ -2520,7 +2518,7 @@ int ha_ndbcluster::ordered_index_scan(co ERR_RETURN(trans->getNdbError()); } - if (execute_no_commit(this,trans,FALSE) != 0) + if (execute_no_commit(this,trans) != 0) DBUG_RETURN(ndb_err(trans)); DBUG_RETURN(next_result(buf)); @@ -2621,7 +2619,7 @@ int ha_ndbcluster::unique_index_scan(con if ((res= define_read_attrs(buf, op))) DBUG_RETURN(res); - if (execute_no_commit(this,trans,FALSE) != 0) + if (execute_no_commit(this,trans) != 0) DBUG_RETURN(ndb_err(trans)); DBUG_PRINT("exit", ("Scan started successfully")); DBUG_RETURN(next_result(buf)); @@ -2689,7 +2687,7 @@ int ha_ndbcluster::full_table_scan(uchar if ((res= define_read_attrs(buf, op))) DBUG_RETURN(res); - if (execute_no_commit(this,trans,FALSE) != 0) + if (execute_no_commit(this,trans) != 0) DBUG_RETURN(ndb_err(trans)); DBUG_PRINT("exit", ("Scan started successfully")); DBUG_RETURN(next_result(buf)); @@ -2903,7 +2901,7 @@ int ha_ndbcluster::write_row(uchar *reco m_bulk_insert_not_flushed= FALSE; if (m_transaction_on) { - if (execute_no_commit(this,trans,FALSE) != 0) + if (execute_no_commit(this,trans) != 0) { m_skip_auto_increment= TRUE; no_uncommitted_rows_execute_failure(); @@ -3197,7 +3195,7 @@ int ha_ndbcluster::update_row(const ucha */ if ((!cursor || m_update_cannot_batch) && - execute_no_commit(this,trans,false) != 0) { + execute_no_commit(this,trans) != 0) { no_uncommitted_rows_execute_failure(); DBUG_RETURN(ndb_err(trans)); } @@ -3314,7 +3312,7 @@ int ha_ndbcluster::delete_row(const ucha } // Execute delete operation - if (execute_no_commit(this,trans,FALSE) != 0) { + if (execute_no_commit(this,trans) != 0) { no_uncommitted_rows_execute_failure(); DBUG_RETURN(ndb_err(trans)); } @@ -3818,7 +3816,7 @@ int ha_ndbcluster::close_scan() deleteing/updating transaction before closing the scan */ DBUG_PRINT("info", ("ops_pending: %ld", (long) m_ops_pending)); - if (execute_no_commit(this,trans,FALSE) != 0) { + if (execute_no_commit(this,trans) != 0) { no_uncommitted_rows_execute_failure(); DBUG_RETURN(ndb_err(trans)); } @@ -4246,7 +4244,7 @@ int ha_ndbcluster::end_bulk_insert() m_bulk_insert_not_flushed= FALSE; if (m_transaction_on) { - if (execute_no_commit(this, trans,FALSE) != 0) + if (execute_no_commit(this, trans) != 0) { no_uncommitted_rows_execute_failure(); my_errno= error= ndb_err(trans); @@ -8642,8 +8640,7 @@ int ha_ndbcluster::write_ndb_file(const } void -ha_ndbcluster::release_completed_operations(NdbTransaction *trans, - bool force_release) +ha_ndbcluster::release_completed_operations(NdbTransaction *trans) { if (trans->hasBlobOperation()) { @@ -8652,16 +8649,7 @@ ha_ndbcluster::release_completed_operati */ return; } - if (!force_release) - { - if (get_thd_ndb(current_thd)->query_state & NDB_QUERY_MULTI_READ_RANGE) - { - /* We are batching reads and have not consumed all fetched - rows yet, releasing operation records is unsafe - */ - return; - } - } + trans->releaseCompletedOperations(); } @@ -8676,6 +8664,12 @@ ha_ndbcluster::null_value_index_search(K ulong reclength= table->s->reclength; uchar *curr= (uchar*)buffer->buffer; uchar *end_of_buffer= (uchar*)buffer->buffer_end; + + /* All passed ranges whose results could fit into the + * buffer are examined, although some may later be + * marked for skipping, wasting buffer space. + */ + assert(!(range->range_flag & SKIP_RANGE)); for (; rangerange_flag |= SKIP_RANGE; continue; @@ -8886,16 +8887,67 @@ ha_ndbcluster::read_multi_range_first(KE buffer->end_of_used_area= curr; } - /* - * Set first operation in multi range - */ - m_current_multi_operation= - lastOp ? lastOp->next() : m_active_trans->getFirstDefinedOperation(); - if (!(res= execute_no_commit_ie(this, m_active_trans,true))) + /* Get pointer to first range key operation (not scans) */ + const NdbOperation* rangeOp= lastOp ? lastOp->next() : + m_active_trans->getFirstDefinedOperation(); + + if (!(res= execute_no_commit_ie(this, m_active_trans))) { m_multi_range_defined= multi_range_curr; multi_range_curr= ranges; m_multi_range_result_ptr= (uchar*)buffer->buffer; + + /* We must unpack all of the results for any primary and unique key + * ranges now, as these operations may be invalidated by + * further execute+releaseOperations calls on this transaction by + * different handler objects. + */ + KEY_MULTI_RANGE* rangeInfo= multi_range_curr; + uchar* result_ptr= m_multi_range_result_ptr; + + for (;rangeInfo < m_multi_range_defined; rangeInfo++) + { + if (rangeInfo->range_flag & SKIP_RANGE) + { + /* Skip over buffer slot to next range */ + result_ptr += reclength; + continue; + } + + if (rangeInfo->range_flag & UNIQUE_RANGE) + { + if (rangeOp->getNdbError().code == 0) + { + /* Successful read, unpack results inplace + * in buffer now. + */ + DBUG_PRINT("info", ("Unique range op has result")); + setup_recattr(rangeOp->getFirstRecAttr()); + unpack_record(result_ptr); + } + else + { + NdbError err= rangeOp->getNdbError(); + + if (err.classification != + NdbError::NoDataFound) + ERR_RETURN(err); + + DBUG_PRINT("info", ("Unique range op has no result")); + /* Indicate to read_multi_range_next that this + * result is empty + */ + rangeInfo->range_flag |= EMPTY_RANGE; + } + + /* Move to next completed operation and buffer 'slot' */ + rangeOp= m_active_trans->getNextCompletedOperation(rangeOp); + result_ptr += reclength; + } + + /* For scan ranges, do nothing */ + } + DBUG_RETURN(read_multi_range_next(found_range_p)); } ERR_RETURN(m_active_trans->getNdbError()); @@ -8920,21 +8972,24 @@ ha_ndbcluster::read_multi_range_next(KEY int res; int range_no; ulong reclength= table_share->reclength; - const NdbOperation* op= m_current_multi_operation; for (;multi_range_curr < m_multi_range_defined; multi_range_curr++) { DBUG_MULTI_RANGE(12); if (multi_range_curr->range_flag & SKIP_RANGE) + { + /* Skip over buffer slot to next range */ + m_multi_range_result_ptr += reclength; continue; + } if (multi_range_curr->range_flag & UNIQUE_RANGE) { - if (op->getNdbError().code == 0) + /* If a row was found, return it */ + if (!(multi_range_curr->range_flag & EMPTY_RANGE)) { DBUG_MULTI_RANGE(13); goto found_next; } - op= m_active_trans->getNextCompletedOperation(op); m_multi_range_result_ptr += reclength; continue; } @@ -9052,12 +9107,9 @@ found_next: */ * multi_range_found_p= multi_range_curr; memcpy(table->record[0], m_multi_range_result_ptr, reclength); - setup_recattr(op->getFirstRecAttr()); - unpack_record(table->record[0]); table->status= 0; multi_range_curr++; - m_current_multi_operation= m_active_trans->getNextCompletedOperation(op); m_multi_range_result_ptr += reclength; DBUG_RETURN(0); } diff -Nrup a/sql/ha_ndbcluster.h b/sql/ha_ndbcluster.h --- a/sql/ha_ndbcluster.h 2008-02-04 14:40:00 +00:00 +++ b/sql/ha_ndbcluster.h 2008-03-19 14:10:33 +00:00 @@ -493,12 +493,12 @@ private: void no_uncommitted_rows_update(int); void no_uncommitted_rows_reset(THD *); - void release_completed_operations(NdbTransaction*, bool); + void release_completed_operations(NdbTransaction*); friend int execute_commit(ha_ndbcluster*, NdbTransaction*); friend int execute_no_commit_ignore_no_key(ha_ndbcluster*, NdbTransaction*); - friend int execute_no_commit(ha_ndbcluster*, NdbTransaction*, bool); - friend int execute_no_commit_ie(ha_ndbcluster*, NdbTransaction*, bool); + friend int execute_no_commit(ha_ndbcluster*, NdbTransaction*); + friend int execute_no_commit_ie(ha_ndbcluster*, NdbTransaction*); void transaction_checks(THD *thd); int start_statement(THD *thd, Thd_ndb *thd_ndb, Ndb* ndb); @@ -559,7 +559,6 @@ private: uchar *m_multi_range_result_ptr; KEY_MULTI_RANGE *m_multi_ranges; KEY_MULTI_RANGE *m_multi_range_defined; - const NdbOperation *m_current_multi_operation; NdbIndexScanOperation *m_multi_cursor; uchar *m_multi_range_cursor_result_ptr; int setup_recattr(const NdbRecAttr*);