lp:~stewart/drizzle/bug720552

Branch merges

Propose for merging

No branches dependent on this one.

Merged into lp:drizzle/7.0 at revision 2183

Drizzle Developers: Pending requested 2011-02-17

Diff: 343 lines (+232/-1)

6 files modified

drizzled/optimizer/sum.cc (+10/-0)
plugin/storage_engine_api_tester/cursor_states.cc (+27/-0)
plugin/storage_engine_api_tester/engine_states.cc (+1/-0)
plugin/storage_engine_api_tester/storage_engine_api_tester.cc (+146/-1)
plugin/storage_engine_api_tester/tests/r/min_index_scan_error.result (+35/-0)
plugin/storage_engine_api_tester/tests/t/min_index_scan_error.test (+13/-0)

api

Related source package recipes

No recipes using this branch. (?)

Create packaging recipe

Related snap packages

Related bugs

Bug #720552: doStartIndexScan return code ignored in optimizer::sum_query() for MIN() optimization

Medium

Fix Released

Link a bug report

Related blueprints

2183. By Stewart Smith on 2011-02-17

in optimization for MIN() on indexed field, the result of Cursor::startIndexScan was not checked before calling other index functions on the Cursor.

The optimizer has the ability to replace certain MIN() queries with a constant.

e.g.
CREATE TABLE t1 (a int, index(a));
.... insert data ...
SELECT MIN(a) FROM t1;

Will execute a lookup on the first record of the index and replace that part of the query tree with a constant.

In the code in sum.cc which executes this optimization, although the return code from startIndexScan was being saved, the variable was immediately overwritten by a subsequent call to (e.g.) index_first. This meant that if a StorageEngine did not save the error code and return it on subsequent index calls, we could (at best) crash.

At some point in the past, the InnoDB devs figured this out, and on error in getting the index, set things appropriately so that anywhere we subsequently get a row_search_for_mysql() call, we'll get back an error code again. I have not audited all index code paths to ensure this is the case everywhere. If one of the index code paths didn't call row_search_for_mysql() or do appropriate checks, we'd probably crash. The scenarios where this would be possible would possibly be around a ongoing transaction accessing a newly created/deleted table or a table protobuf message not matching the table in innodb.

MyISAM and ARCHIVE just sets a local variable and does nothing in doStartIndexScan that could possibly fail.

It looks as though this could cause a crash in PBXT and HailDB.

A way to test this is with storage_engine_api_tester injecting an error into the codepath.

2182. By Stewart Smith on 2011-02-17

update storage_engine_api_tester min_index_scan_error test result to fix what should happen (prior to the fix for the bug, the serevr will crash, so was hard to produce a correct result file)

2181. By Stewart Smith on 2011-02-17

add SEAPITester Cursor state for error in dostartIndexScan()

2180. By Stewart Smith on 2011-02-17

add --error to expect the error we're getting from our error injection in storage_engine_api_tester.min_index_scan_error test

2179. By Stewart Smith on 2011-02-17

add error inject test in SEAPITester to test if doStartIndexScan error is being checked. The test case we add is for MIN()

2178. By Stewart Smith on 2011-02-17

remove ::extra() being logged - more trouble than currently worth. should just be a 'called from anywhere' command

2177. By Stewart Smith on 2011-02-17

move index_flags() to engine where it belongs in SEAPITester

2176. By Stewart Smith on 2011-02-17

fix up a few more index bits in SEAPITester Cursor impl

2175. By Stewart Smith on 2011-02-17

fill in index access methods in SEAPITester.

2174. By Stewart Smith on 2011-02-17

fix SEAPITester ::info() support by properly copying back errkey for HA_STATUS_ERRKEY and ha_statistics from the real cursor