From: Jorgen Loland Date: January 4 2012 9:52am Subject: bzr push into mysql-trunk branch (jorgen.loland:3505 to 3506) Bug#13350136 List-Archive: http://lists.mysql.com/commits/142280 X-Bug: 13350136 Message-Id: <20120104095200.0E8161C8@atum21.no.oracle.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit 3506 Jorgen Loland 2012-01-04 BUG#13350136 - TEMPORARY MYISAM TABLES DO NOT PACK LONG VARCHAR COLUMNS MyISAM has three record formats (fixed, packed and compressed) out of which fixed and packed are applicable for temporary tables. The difference is that with fixed length record format, VARCHAR trailing bytes are written whereas for packed record format the trailing bytes are not written. The packed record therefore saves space at the cost of some additional memcopies. The packed record format is currently not in use by temporary tables, although old commit messages suggests that this has not always been the case, e.g: revision: sp1r-monty@stripped Date: 2004-12-06 "Internal temporary files can now use fixed length tables if the used VARCHAR columns are short" This CS enables packed record format for temporary tables. The CS also includes a fix for a MyISAM bug that is only a problem when the packed format and a special unique constraint (that checksums wide columns) is used. This unique constraint only applies to internal temporary tables when uniqueness is required on columns with a total size bigger than normal indexes allow (1000 bytes). The problem: A column's value in MyISAM can be read from or written to the row buffer using MI_COLUMNDEF::offset ("normal" column definition) or HA_KEYSEG::start (key definition). The former is calculated in mi_open() and is the sum of the length of all preceding columns. The latter is calculated in mi_create(). The bug was that in mi_create(), HA_KEYSEG::start was set to the offset of the column for the on-disk format (reclength) whereas it should have been the offset for the in-memory format. Due to this bug, unique constraint values were written to one offset (HA_KEYSEG::start in mi_check_unique()) and read from another offset (MI_COLUMNDEF::offset in _mi_pack_record et al), resulting in duplicate rows in the result set because equally valued rows got seemingly different checksums. @ mysql-test/suite/opt_trace/r/temp_table.result Trace changes because packed record format is used for tmp table @ sql/sql_tmp_table.cc Enable packed record format for MyISAM temporary tables @ storage/myisam/mi_create.c HA_KEYSEG::start is supposed to point to the same offset as MI_COLUMNDEF::offset, which is the offset of a column when in memory. Instead it pointed to the offset for the on-disk format. Set in mi_create() modified: mysql-test/suite/opt_trace/r/temp_table.result sql/sql_tmp_table.cc storage/myisam/mi_create.c 3505 Guilhem Bichot 2012-01-03 Fix for Bug#11766384 - 59487: WRONG RESULT WITH STRAIGHT_JOIN AND RIGHT JOIN BUG#11752239 - 43368: STRAIGHT_JOIN DOESN'T WORK FOR NESTED JOINS BUG#11766858 - 60080: STRAIGHT_JOIN IS NOT ENFORCED WHEN RIGHT-HAND TABLE IS ITSELF A JOIN dependencies were not propagated when a nested join was flattened. @ mysql-test/r/innodb_icp.result This is the testcase for BUG 11766858 - 60080: STRAIGHT_JOIN IS NOT ENFORCED WHEN RIGHT-HAND TABLE IS ITSELF A JOIN Query has EXPLAIN SELECT table1.col_int_nokey FROM t1 AS table1 STRAIGHT_JOIN (t1 AS table2 INNER JOIN t1 AS table3 etc) and so table1 should be first. @ mysql-test/suite/opt_trace/include/general2.inc bug is fixed @ mysql-test/suite/opt_trace/r/general2_no_prot.result Query is: SELECT t1.f2 FROM t1 STRAIGHT_JOIN (t2 JOIN t3 etc) so t2 and t3 should be marked as dependent from t1 (due to straight_join); they weren't. Now we see they are (depends_on_map_bits says "0"). t3 used to be recognized as constant ("system" table); now that it is dependent on other tables, it's not (see the test for !s->dependent on which I added a comment in sql_select.cc: it makes the table not be recognized as constant). So t3 now participates in plan search (not necessarily a good thing, except that we can say that this is better observing STRAIGHT_JOIN: marking a table as constant moves it to the plan's front, producing an order which is not the one requested by the user...). So we explore plan t1-t3-t2 and t1-t2, which all respect STRAIGHT_JOIN. We don't anymore explore the plan starting with t2, as now-correct t2->dep_tables make it impossible (t2 depends on t1). The fact that t3 is not constant is also seen in WHERE conditions (no more constant propagation). @ mysql-test/t/join.test test for BUG#11752239 @ mysql-test/t/join_outer.test test for Bug#11766384 @ sql/sql_optimizer.cc Consider this scenario: t1 STRAIGHT_JOIN (t2 JOIN t3): - a nested join may depend on other tables (here (t2 JOIN t3) depends on t1, due to STRAIGHT_JOIN), this is recorded in the nested join's dep_tables variable - when flattening this nested join (eliminating it, eliminating parenthesis), the nested join goes away, so its dep_tables is lost: the query becomes t1 JOIN t2 JOIN t3. So STRAIGHT_JOIN is not honoured. The fix is: tables of the nested join must inherit dependencies of the to-be-deleted nested join. We even got an assert sometimes (testcase in join_outer.test), consider SELECT STRAIGHT_JOIN t2 RIGHT JOIN (t1 JOIN t3). The parser produces this list of TABLE_LIST objects: t2 - NJ(t1 - t3). The JOIN's join_list is reversed so that right join appears as left join: NJ(t1 - t3) - t2. The initial order of JOIN_TABs is always that of TABLE_LISTs: t2 - t1 - t3 (and this is why the assert happened only with a RIGHT JOIN, btw see WL 6059 "Make RIGHT JOIN be fully converted to LEFT JOIN"). In the testcase of join_outer.test, the initial order of JOIN_TABs had t2 before t1,t4,t5. Because dep_tables was wrong (too weak) for t2, join_tab_cmp_straight() didn't move t2 to the plan's tail; it moved some other tables to the head because they had a correct dep_tables; t2 ended in the middle. Then check_interleaving_with_nj() correctly recognized that this plan cannot be executed, because t2 was between t4 and t3 i.e. in the middle of (t3 JOIN t4) - you can't have a table in the middle of a nest which doesn't own it, this causes problems for NULL-complementing. modified: mysql-test/r/innodb_icp.result mysql-test/r/innodb_icp_all.result mysql-test/r/innodb_icp_none.result mysql-test/r/join.result mysql-test/r/join_outer.result mysql-test/r/join_outer_bka.result mysql-test/r/join_outer_bka_nixbnl.result mysql-test/r/myisam_icp.result mysql-test/r/myisam_icp_all.result mysql-test/r/myisam_icp_none.result mysql-test/suite/opt_trace/include/general2.inc mysql-test/suite/opt_trace/r/general2_no_prot.result mysql-test/suite/opt_trace/r/general2_ps_prot.result mysql-test/t/join.test mysql-test/t/join_outer.test sql/sql_optimizer.cc === modified file 'mysql-test/suite/opt_trace/r/temp_table.result' --- a/mysql-test/suite/opt_trace/r/temp_table.result 2011-11-21 08:51:57 +0000 +++ b/mysql-test/suite/opt_trace/r/temp_table.result 2012-01-04 09:51:39 +0000 @@ -133,7 +133,7 @@ SELECT uniq, col1 FROM t1 GROUP BY col2, "key_length": 1040, "unique_constraint": true, "location": "disk (MyISAM)", - "record_format": "fixed" + "record_format": "packed" } /* tmp_table_info */ } /* creating_tmp_table */ }, @@ -153,7 +153,7 @@ SELECT uniq, col1 FROM t1 GROUP BY col2, "filesort_priority_queue_optimization": { "limit": 3, "rows_estimate": 18, - "row_size": 1043, + "row_size": 1042, "memory_available": 262144, "chosen": true } /* filesort_priority_queue_optimization */, @@ -163,7 +163,7 @@ SELECT uniq, col1 FROM t1 GROUP BY col2, "rows": 4, "examined_rows": 8, "number_of_tmp_files": 0, - "sort_buffer_size": 4204, + "sort_buffer_size": 4200, "sort_mode": "" } /* filesort_summary */ } @@ -484,7 +484,7 @@ SELECT uniq, col1, col2 FROM t1 GROUP BY "key_length": 13, "unique_constraint": false, "location": "disk (MyISAM)", - "record_format": "fixed" + "record_format": "packed" } /* tmp_table_info */ } /* converting_tmp_table_to_myisam */ }, @@ -506,7 +506,7 @@ SELECT uniq, col1, col2 FROM t1 GROUP BY "rows": 16, "examined_rows": 16, "number_of_tmp_files": 0, - "sort_buffer_size": 676, + "sort_buffer_size": 650, "sort_mode": "" } /* filesort_summary */ } === modified file 'sql/sql_tmp_table.cc' --- a/sql/sql_tmp_table.cc 2011-12-15 15:15:37 +0000 +++ b/sql/sql_tmp_table.cc 2012-01-04 09:51:39 +0000 @@ -464,8 +464,8 @@ create_tmp_table(THD *thd,TMP_TABLE_PARA uint temp_pool_slot=MY_BIT_NONE; uint fieldnr= 0; ulong reclength, string_total_length; - bool using_unique_constraint= 0; - bool use_packed_rows= 0; + bool using_unique_constraint= false; + bool use_packed_rows= false; bool not_all_columns= !(select_options & TMP_TABLE_ALL_COLUMNS); char *tmpname,path[FN_REFLEN]; uchar *pos, *group_buff, *bitmaps; @@ -527,10 +527,10 @@ create_tmp_table(THD *thd,TMP_TABLE_PARA */ (*tmp->item)->marker= 4; if ((*tmp->item)->max_length >= CONVERT_IF_BIGGER_TO_BLOB) - using_unique_constraint=1; + using_unique_constraint= true; } if (param->group_length >= MAX_BLOB_WIDTH) - using_unique_constraint=1; + using_unique_constraint= true; if (group) distinct=0; // Can't use distinct } @@ -750,6 +750,14 @@ create_tmp_table(THD *thd,TMP_TABLE_PARA *blob_field++= fieldnr; blob_count++; } + + if (new_field->real_type() == MYSQL_TYPE_STRING || + new_field->real_type() == MYSQL_TYPE_VARCHAR) + { + string_count++; + string_total_length+= new_field->pack_length(); + } + if (item->marker == 4 && item->maybe_null) { group_null_items++; @@ -800,7 +808,7 @@ create_tmp_table(THD *thd,TMP_TABLE_PARA if (group && (param->group_parts > table->file->max_key_parts() || param->group_length > table->file->max_key_length())) - using_unique_constraint=1; + using_unique_constraint= true; } else { @@ -836,7 +844,7 @@ create_tmp_table(THD *thd,TMP_TABLE_PARA (string_total_length >= STRING_TOTAL_LENGTH_TO_PACK_ROWS && (reclength / string_total_length <= RATIO_TO_PACK_ROWS || string_total_length / string_count >= AVG_STRING_LENGTH_TO_PACK_ROWS))) - use_packed_rows= 1; + use_packed_rows= true; if (!use_packed_rows) share->db_create_options&= ~HA_OPTION_PACK_RECORD; @@ -956,6 +964,10 @@ create_tmp_table(THD *thd,TMP_TABLE_PARA field->real_type() == MYSQL_TYPE_STRING && length >= MIN_STRING_LENGTH_TO_PACK_ROWS) recinfo->type=FIELD_SKIP_ENDSPACE; + else if (use_packed_rows && + field->real_type() == MYSQL_TYPE_VARCHAR && + length >= MIN_STRING_LENGTH_TO_PACK_ROWS) + recinfo->type= FIELD_VARCHAR; else recinfo->type=FIELD_NORMAL; if (!--hidden_field_count) @@ -1732,7 +1744,10 @@ bool create_myisam_tmp_table(TABLE *tabl start_recinfo, share->uniques, &uniquedef, &create_info, - HA_CREATE_TMP_TABLE))) + HA_CREATE_TMP_TABLE | + ((share->db_create_options & HA_OPTION_PACK_RECORD) ? + HA_PACK_RECORD : 0) + ))) { table->file->print_error(error,MYF(0)); /* purecov: inspected */ table->db_stat=0; === modified file 'storage/myisam/mi_create.c' --- a/storage/myisam/mi_create.c 2011-08-29 12:08:58 +0000 +++ b/storage/myisam/mi_create.c 2012-01-04 09:51:39 +0000 @@ -462,7 +462,6 @@ int mi_create(const char *name,uint keys key_del[i]=HA_OFFSET_ERROR; unique_key_parts=0; - offset=reclength-uniques*MI_UNIQUE_HASH_LENGTH; for (i=0, uniquedef=uniquedefs ; i < uniques ; i++ , uniquedef++) { uniquedef->key=keys+i; @@ -727,7 +726,7 @@ int mi_create(const char *name,uint keys #endif } /* Create extra keys for unique definitions */ - offset=reclength-uniques*MI_UNIQUE_HASH_LENGTH; + offset= real_reclength - uniques * MI_UNIQUE_HASH_LENGTH; memset(&tmp_keydef, 0, sizeof(tmp_keydef)); memset(&tmp_keyseg, 0, sizeof(tmp_keyseg)); for (i=0; i < uniques ; i++) No bundle (reason: useless for push emails).