===== mysql-test/suite/ndb/r/ndb_read_multi_range.result 1.17 vs edited ===== --- 1.17/mysql-test/suite/ndb/r/ndb_read_multi_range.result 2008-03-17 12:49:11 +00:00 +++ edited/mysql-test/suite/ndb/r/ndb_read_multi_range.result 2008-03-17 10:51:23 +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; ===== mysql-test/suite/ndb/t/ndb_read_multi_range.test 1.21 vs edited ===== --- 1.21/mysql-test/suite/ndb/t/ndb_read_multi_range.test 2008-03-17 12:49:11 +00:00 +++ edited/mysql-test/suite/ndb/t/ndb_read_multi_range.test 2008-03-17 10:50:02 +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; ===== sql/ha_ndbcluster.cc 1.476 vs edited ===== --- 1.476/sql/ha_ndbcluster.cc 2008-03-17 12:49:18 +00:00 +++ edited/sql/ha_ndbcluster.cc 2008-03-17 12:28:22 +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(); } @@ -8689,6 +8677,17 @@ ha_ndbcluster::null_value_index_search(K DBUG_RETURN(FALSE); } +/* Enum describing state of unique range operation + * result data in buffer. + * Indicates whether the range contained a row + * (or NoDataFound) + */ +enum unique_range_result_state +{ + enum_result_present, + enum_result_empty +}; + int ha_ndbcluster::read_multi_range_first(KEY_MULTI_RANGE **found_range_p, KEY_MULTI_RANGE *ranges, @@ -8700,7 +8699,13 @@ ha_ndbcluster::read_multi_range_first(KE int res; KEY* key_info= table->key_info + active_index; NDB_INDEX_TYPE cur_index_type= get_index_type(active_index); - ulong reclength= table_share->reclength; + const ulong reclength= table_share->reclength; + /* Buffer includes 1 byte for unique range result type info + * stored at the end of the buffer + */ + const ulong buffRecLength = ((reclength + 1 + (sizeof(void*) -1)) + / sizeof(void*)) * sizeof(void*); + const ulong ur_state_offset= buffRecLength - 1; NdbOperation* op; Thd_ndb *thd_ndb= get_thd_ndb(current_thd); DBUG_ENTER("ha_ndbcluster::read_multi_range_first"); @@ -8759,7 +8764,7 @@ ha_ndbcluster::read_multi_range_first(KE const NDBINDEX *idx= m_index[active_index].index; const NdbOperation* lastOp= m_active_trans->getLastDefinedOperation(); NdbIndexScanOperation* scanOp= 0; - for (; multi_range_currrange_flag |= SKIP_RANGE; continue; } @@ -8800,7 +8804,7 @@ ha_ndbcluster::read_multi_range_first(KE !define_read_attrs(curr, op) && (!m_use_partition_function || (op->setPartitionId(part_spec.start_part), TRUE))) - curr += reclength; + curr += buffRecLength; else ERR_RETURN(op ? op->getNdbError() : m_active_trans->getNdbError()); break; @@ -8820,7 +8824,7 @@ ha_ndbcluster::read_multi_range_first(KE !op->readTuple(lm) && !set_index_key(op, key_info, multi_range_curr->start_key.key) && !define_read_attrs(curr, op)) - curr += reclength; + curr += buffRecLength; else ERR_RETURN(op ? op->getNdbError() : m_active_trans->getNdbError()); break; @@ -8839,16 +8843,16 @@ ha_ndbcluster::read_multi_range_first(KE if (scanOp->reset_bounds(m_force_send)) DBUG_RETURN(ndb_err(m_active_trans)); - end_of_buffer -= reclength; + end_of_buffer -= buffRecLength; } else if ((scanOp= m_active_trans->getNdbIndexScanOperation(idx, tab)) &&!scanOp->readTuples(lm, 0, parallelism, sorted, FALSE, TRUE, need_pk, TRUE) &&!(m_cond && m_cond->generate_scan_filter(scanOp)) - &&!define_read_attrs(end_of_buffer-reclength, scanOp)) + &&!define_read_attrs(end_of_buffer-buffRecLength, scanOp)) { m_multi_cursor= scanOp; - m_multi_range_cursor_result_ptr= end_of_buffer-reclength; + m_multi_range_cursor_result_ptr= end_of_buffer-buffRecLength; } else { @@ -8886,16 +8890,65 @@ 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) + /* Nothing to do here */ + 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); + result_ptr[ur_state_offset] = enum_result_present; + } + 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 + */ + result_ptr[ur_state_offset]= enum_result_empty; + } + + /* Move to next completed operation and buffer 'slot' */ + rangeOp= m_active_trans->getNextCompletedOperation(rangeOp); + result_ptr += buffRecLength; + } + + /* For scan ranges, do nothing */ + } + DBUG_RETURN(read_multi_range_next(found_range_p)); } ERR_RETURN(m_active_trans->getNdbError()); @@ -8919,8 +8972,10 @@ ha_ndbcluster::read_multi_range_next(KEY int res; int range_no; - ulong reclength= table_share->reclength; - const NdbOperation* op= m_current_multi_operation; + const ulong reclength= table_share->reclength; + const ulong buffRecLength = ((reclength + 1 + (sizeof(void*) -1)) + / sizeof(void*)) * sizeof(void*); + const ulong ur_state_offset= buffRecLength - 1; for (;multi_range_curr < m_multi_range_defined; multi_range_curr++) { DBUG_MULTI_RANGE(12); @@ -8928,14 +8983,15 @@ ha_ndbcluster::read_multi_range_next(KEY continue; if (multi_range_curr->range_flag & UNIQUE_RANGE) { - if (op->getNdbError().code == 0) + /* If any row was returned, return it */ + if (m_multi_range_result_ptr[ur_state_offset] == + enum_result_present) { DBUG_MULTI_RANGE(13); goto found_next; } - op= m_active_trans->getNextCompletedOperation(op); - m_multi_range_result_ptr += reclength; + m_multi_range_result_ptr += buffRecLength; continue; } else if (m_multi_cursor && !multi_range_sorted) @@ -9052,13 +9108,10 @@ 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; + m_multi_range_result_ptr += buffRecLength; DBUG_RETURN(0); } ===== sql/ha_ndbcluster.h 1.190 vs edited ===== --- 1.190/sql/ha_ndbcluster.h 2008-03-17 12:49:18 +00:00 +++ edited/sql/ha_ndbcluster.h 2008-03-14 18:30:47 +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*);