From: Roy Lyseng Date: November 18 2010 9:47am Subject: bzr commit into mysql-trunk branch (roy.lyseng:3287) Bug#57525 List-Archive: http://lists.mysql.com/commits/124235 X-Bug: 57525 Message-Id: <20101118094747.0CDC11F2@tyr67.norway.sun.com> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="===============2089778045302035936==" --===============2089778045302035936== MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Content-Disposition: inline #At file:///home/rl136806/mysql/repo/mysql-work5/ based on revid:tor.didriksen@stripped 3287 Roy Lyseng 2010-11-18 Bug#57525: Semijoin transformed subquery with inner grouped subquery gives empty result. This is a followup to bug#31480, which attempted to fix resolved information in subqueries as part of semijoin transformation. The specific problem here is that the inner subquery (which is not transformed) contains an outer reference to the outer-most block in its HAVING clause. The HAVING clause is represented by Item_ref objects, and these are not re-resolved properly through fix_after_pullout(). The solution lies in realizing that Item_ref objects contain a complete set of resolution data structures (ie depended_from, name resolution context), and that it may point to an Item object with it's own set of resolution data structures. Hence, we implement fix_after_pullout() for Item_ref by first calling fix_after_pullout() on the referenced object and then for itself. The above fix revealed another problem: The offending query failed in prepared statement mode. The reason is that the depended_from field in Item_ref is reset in cleanup() and not restored at the next fix_fields() call. A hack that persists the depended_from field across muliple fix_fields() solves this problem, and I think it is safe in the general case. There is also another followup to bug#31480: A - t1 \ B - t2 \ C - t3 \ D - t4 where t1.x=t4.y The above figure describes an outer query expression (select_lex A) with 3 nested subqueries represented by select_lex B, C and D. The innermost subquery (D) contains a reference to a table t1 in the outermost block (A). The original resolver marked the subquery containing query expression B as using table t1, and the subqueries containing query expressions C and D with OUTER_REF_TABLE_BIT. The code in bug#31480 failed to preserve all outer references when calling fix_after_pullout(). mysql-test/r/optimizer_switch.result Updated with correct result and plan for query that exposed bug. mysql-test/t/optimizer_switch.test Warnings about wrong results removed. sql/item.cc A common implementation of fix_after_pullout() for all Item_ref classes has been made. The exception is Item_aggregate_ref which must have its own implementation and Item_outer_ref which we still have no test case for. Item_ref::fix_after_pullout() now calls fix_after_pullout() on the referenced item, and then it calls Item_ident::fix_after_pullout() on itself. Item_field::fix_after_pullout() is also moved to class Item_ident, which is parent class for both Item_field and Item_ref. A new function Item_ref::resolved_used_tables() was needed. sql/item.h Some adjustments to function prototypes, see sql/item.cc. sql/item_subselect.h One changed friend declaration. modified: mysql-test/r/optimizer_switch.result mysql-test/t/optimizer_switch.test sql/item.cc sql/item.h sql/item_subselect.h === modified file 'mysql-test/r/optimizer_switch.result' --- a/mysql-test/r/optimizer_switch.result 2010-11-08 14:51:09 +0000 +++ b/mysql-test/r/optimizer_switch.result 2010-11-18 09:47:04 +0000 @@ -293,14 +293,13 @@ a b 3 20 2 30 set @@optimizer_switch='materialization=off,semijoin=on'; -# The query result with semijoin is WRONG EXPLAIN SELECT * FROM t1 AS ta WHERE ta.a IN (SELECT c FROM t2 AS tb WHERE tb.d >= SOME(SELECT SUM(g) FROM t4 as tc GROUP BY f HAVING ta.a=tc.f)); id select_type table type possible_keys key key_len ref rows Extra -1 PRIMARY tb ALL NULL NULL NULL NULL 6 Using where; Start temporary +1 PRIMARY tb ALL NULL NULL NULL NULL 6 Start temporary 1 PRIMARY ta ALL NULL NULL NULL NULL 7 Using where; End temporary; Using join buffer (BNL, regular buffers) 3 DEPENDENT SUBQUERY tc ALL NULL NULL NULL NULL 6 Using temporary; Using filesort SELECT * FROM t1 AS ta @@ -309,6 +308,10 @@ WHERE tb.d >= SOME(SELECT SUM(g) FROM t4 GROUP BY f HAVING ta.a=tc.f)); a b +2 10 +2 20 +3 20 +2 30 # Subquery with ORDER BY and LIMIT set @@optimizer_switch='materialization=off,semijoin=off'; # NOTE: The ordered subquery should have a LIMIT clause to make sense @@ -332,7 +335,6 @@ a b 2 30 4 40 set @@optimizer_switch='materialization=off,semijoin=on'; -# The query result with semijoin is WRONG EXPLAIN SELECT * FROM t1 AS ta WHERE ta.a IN (SELECT c FROM t2 AS tb WHERE tb.d IN (SELECT g FROM t4 as tc === modified file 'mysql-test/t/optimizer_switch.test' --- a/mysql-test/t/optimizer_switch.test 2010-10-15 10:32:50 +0000 +++ b/mysql-test/t/optimizer_switch.test 2010-11-18 09:47:04 +0000 @@ -280,8 +280,6 @@ eval $query; set @@optimizer_switch='materialization=off,semijoin=on'; ---echo # The query result with semijoin is WRONG - eval EXPLAIN $query; eval $query; @@ -303,8 +301,6 @@ eval $query; set @@optimizer_switch='materialization=off,semijoin=on'; ---echo # The query result with semijoin is WRONG - eval EXPLAIN $query; eval $query; === modified file 'sql/item.cc' --- a/sql/item.cc 2010-11-14 18:09:32 +0000 +++ b/sql/item.cc 2010-11-18 09:47:04 +0000 @@ -1457,6 +1457,13 @@ public: const char *table_name_arg, const char *field_name_arg) :Item_ref(context_arg, item, table_name_arg, field_name_arg) {} + virtual void fix_after_pullout(st_select_lex *parent_select, + st_select_lex *removed_select, + Item **ref_arg) + { + (*ref)->fix_after_pullout(parent_select, removed_select, ref); + } + virtual inline void print (String *str, enum_query_type query_type) { if (ref) @@ -2319,7 +2326,7 @@ table_map Item_field::resolved_used_tabl return field->table->map; } -void Item_field::fix_after_pullout(st_select_lex *parent_select, +void Item_ident::fix_after_pullout(st_select_lex *parent_select, st_select_lex *removed_select, Item **ref) { @@ -2359,17 +2366,16 @@ void Item_field::fix_after_pullout(st_se */ st_select_lex *child_select= context->select_lex; - if (child_select->outer_select() != depended_from) + while (child_select->outer_select() != depended_from) { /* The subquery on this level is outer-correlated with respect to the field */ Item_subselect *subq_predicate= child_select->master_unit()->item; - subq_predicate->used_tables_cache|= OUTER_REF_TABLE_BIT; - } - while (child_select->outer_select() != depended_from) + subq_predicate->used_tables_cache|= OUTER_REF_TABLE_BIT; child_select= child_select->outer_select(); + } /* child_select is select_lex immediately inner to the depended_from level. @@ -6495,10 +6501,26 @@ void Item_ref::set_properties() } +table_map Item_ref::resolved_used_tables() const +{ + DBUG_ASSERT((*ref)->type() == FIELD_ITEM); + return ((Item_field*)(*ref))->resolved_used_tables(); +} + + void Item_ref::cleanup() { DBUG_ENTER("Item_ref::cleanup"); + /* + Save depended_from so that it can be restored after Item_ident::cleanup(). + Item_ref::depended_from is not recalculated in later fix_fields() calls, + so this hack is needed to have correct dependency information in prepared + statement execution. Dependencies should not change unless an involved + view is updated, and this currently has much more severe implications. + */ + st_select_lex *save_depended_from= depended_from; Item_ident::cleanup(); + depended_from= save_depended_from; result_field= 0; DBUG_VOID_RETURN; } @@ -6871,29 +6893,12 @@ void Item_ref::fix_after_pullout(st_sele st_select_lex *removed_select, Item **ref_arg) { - // @todo: Find an actual test case where depended_from == new_parent. - DBUG_ASSERT(depended_from != parent_select); - if (depended_from == parent_select) - depended_from= NULL; -} - -void Item_direct_view_ref::fix_after_pullout(st_select_lex *parent_select, - st_select_lex *removed_select, - Item **refptr) -{ - DBUG_EXECUTE("where", - print_where(*refptr, - "Item_direct_view_ref::fix_after_pullout", - QT_ORDINARY);); - (*ref)->fix_after_pullout(parent_select, removed_select, ref); - // @todo: Find an actual test case where depended_from == parent_select. - DBUG_ASSERT(depended_from != parent_select); - if (depended_from == parent_select) - depended_from= NULL; + Item_ident::fix_after_pullout(parent_select, removed_select, ref); } + /** Compare two view column references for equality. === modified file 'sql/item.h' --- a/sql/item.h 2010-10-15 10:32:50 +0000 +++ b/sql/item.h 2010-11-18 09:47:04 +0000 @@ -1675,7 +1675,13 @@ public: const char *field_name_arg); Item_ident(THD *thd, Item_ident *item); Item_ident(TABLE_LIST *view_arg, const char *field_name_arg); + /* + Return used table information for the level on which this table is resolved. + */ + virtual table_map resolved_used_tables() const= 0; const char *full_name() const; + virtual void fix_after_pullout(st_select_lex *parent_select, + st_select_lex *removed_select, Item **ref); void cleanup(); bool remove_dependence_processor(uchar * arg); virtual void print(String *str, enum_query_type query_type); @@ -1761,16 +1767,11 @@ public: bool send(Protocol *protocol, String *str_arg); void reset_field(Field *f); bool fix_fields(THD *, Item **); - void fix_after_pullout(st_select_lex *parent_select, - st_select_lex *removed_select, Item **ref); void make_field(Send_field *tmp_field); int save_in_field(Field *field,bool no_conversions); void save_org_in_field(Field *field); table_map used_tables() const; - /* - Return used table information for the level on which this table is resolved. - */ - table_map resolved_used_tables() const; + virtual table_map resolved_used_tables() const; enum Item_result result_type () const { return field->result_type(); @@ -2604,6 +2605,7 @@ public: if (!depended_from) (*ref)->update_used_tables(); } + virtual table_map resolved_used_tables() const; table_map not_null_tables() const { return (*ref)->not_null_tables(); } void set_result_field(Field *field) { result_field= field; } bool is_result_field() { return 1; } @@ -2726,8 +2728,6 @@ public: {} bool fix_fields(THD *, Item **); - void fix_after_pullout(st_select_lex *parent_select, - st_select_lex *removed_select, Item **ref); bool eq(const Item *item, bool binary_cmp) const; Item *get_tmp_table_item(THD *thd) { === modified file 'sql/item_subselect.h' --- a/sql/item_subselect.h 2010-10-15 10:32:50 +0000 +++ b/sql/item_subselect.h 2010-11-18 09:47:04 +0000 @@ -172,7 +172,7 @@ public: friend bool Item_field::fix_fields(THD *, Item **); friend int Item_field::fix_outer_field(THD *, Field **, Item **); friend bool Item_ref::fix_fields(THD *, Item **); - friend void Item_field::fix_after_pullout(st_select_lex *parent_select, + friend void Item_ident::fix_after_pullout(st_select_lex *parent_select, st_select_lex *removed_select, Item **ref); friend void mark_select_range_as_dependent(THD*, --===============2089778045302035936== MIME-Version: 1.0 Content-Type: text/bzr-bundle; charset="us-ascii"; name="bzr/roy.lyseng@stripped" Content-Transfer-Encoding: 7bit Content-Disposition: inline # Bazaar merge directive format 2 (Bazaar 0.90) # revision_id: roy.lyseng@stripped # target_branch: file:///home/rl136806/mysql/repo/mysql-work5/ # testament_sha1: ba0c8518c51707289acc0146769f3454dbfadc15 # timestamp: 2010-11-18 10:47:46 +0100 # base_revision_id: tor.didriksen@stripped\ # 86cs5woahaule77b # # Begin bundle IyBCYXphYXIgcmV2aXNpb24gYnVuZGxlIHY0CiMKQlpoOTFBWSZTWdXRLKQABlR/gHSwAAJ49/// f+ffpL////pgDwOrqrg+97OmtUO2rWKm9eLvbuTb1vb0dlYoKlVaatNIUowkkQMiVP2RpoGp5TKe VPCMUekaNDQ9TamE00PUeiDITQJhCalPU9qmm9SAPJAB6gAAAAGg1PUyamo0TEZNHqMEaMmgDQ0A AaADQAkREBDSbVPQCaT2gKBtIyMnqMg0NAAMhzCaA0Bo0YRoMRpiZMTQYRoGQDJgJIgQBDEAhoIZ Mk1Pap6QAyGnqaAAaUInZBdpY6vyYNd/poXzJDf43huxbJHmi3ExZsJMklMXKSdfD9MtTew+EJK0 fPudxLO8iTzdFky2QJPL3vnqdFvceB7pD+ZNnv2fVMNhXZPNMWXJxNtcX+TTjNLPFvC2AwMJdrBY y0mJd7KNG3xQBb/BldyM38cPNSHYaNc0pjE2aIuhDCaLo/1u8Mi17eTLIm0VXanm3YkBUwIoQAzI Cs6efRfhWnCXntS+n6vT6qnnR2I3IbY2NiG2w50vPgDnWbFMJXilm3MotwD4FyweJxJWIVbjW5EI LJumcWQ6slm7C2Va7qY4jxc0L7Fc7mWvCt40YylXwEF+LGZdOeTpXQi1bVqDV0zYvul7Ov3ISTQb IrTqdz1EOF1WaDxvIxb0xJRrca7bc9dBtW0uu6JyvpcSpEML5lVv1HiqaEX0+4RheiBRrF4hCkqC irq5M+VGRRQDmwK3WGseYqIbx8a3yJqH7rUxxiNabpN7z7p2CWsFisKJAU5KGEFxN6JDZKzXKsH0 eARBlw+LDHaQo9TaraKq65XOLmWkh1s4VRQZS5CTrvMZ7Yzi/UyBJtVeY8p8utHCFtR4m8R2eo1v pPYGnO0rVykeF4QabruJKwzuzfSpQrWW25n1bgiGrT50F5qipeMCaPo0J8a8a9xQmxGsaDtrRHE2 aW6POqHpVffNdXoebnjqx8xAy4t0ysPpcj0dXllz7wJmAWIU/HO4233ieJjhxyej7rRtVGwfVOGv slBzc0AfPsweh0LXB97P9ZVeBBVjqsxYMPg73c3lXIHy41g3WN1HEYmFYkdh4LsXqJY3sWLvhc2g vHWW4VGus9k1gXkRaVtO1/BUbtu0xRungiNLJY654IYkipywRAg2BCGwP/MG6rgb0rw7qmsMLb0R Ud/Dsvga/beCU2CTJRYUJxG/4TWonKH1hWciIdXtxHpNK8GSCaQu1o5jKpBBKVTESYW2FLsJCbYv pQwMklYUB8AmymsLC0VRcTuoxfSglkfjsPiMlkyMpaxUMkEEMrZMYagkPquPanEEnvQ0CAOX9Kre YMuGqpIJuMgN5XP5VX2GWFvss35/U3HUZFW6GeD8koLhBrWJaJ9fepmI9JMWMBkqYILBiFJUdB39 UwTXuKjDfsNpzCoWaKETUbEUXFxaFx+Al1rr0NWjWQLyEXjkX2re3WZAlzKyZMto0i0zkxcc50Y2 sB0LCeC6q7FVXfYMXDlGCtUq1RXCAiRMcxMDt6EcTMtVmRpLvxpVrhqze14wjTStbaiQVBpnDIvM X/urllgmr1RJzAuzzzE+D8hXSPHTuiwmrS2PNwYXPA7D42Nc/8MjncUiuzJLh12BLjM+BMlkNEkZ ZEjbFcCZUMEC0e9JMFZflrKpXNoKwm+8w5H0Ag8g0OlMbzwHhhl4Vy4csMCwJhLJnsKMFkZUK5b1 3alUo5lZcY0IylULHlKlycxLA1m013h90xOTFpzDzPuqsINgda5X3Zp4PzA+y1jQ1yvnCsuLs6wu L9pYtoxDbJ0QWrhOo6m2E0TGojXWThgZDb4J1AfcNoJHYxFjeQ7qF+V8aLls1jF0A2BNgoioyKB5 hx2d50qNOnKzvNgthjd8iNEuEEdz5whJtgL6E3shqMdfhDCAnySLapOkrzCklFeEpZ2Jkyyo5ylI mi8pUTH7lqwN9iLsMddzksxzIhPrsCYY1CRGk+TgO+ugbYRPljZQTh2SsAVVk7bBTm7hcMSb3e9m zJGoL1iQvj6YXROza2t3TGMpBi2Gx3Sum0+Dio04JNMjoEJiuAqratS+FxgOHQLTJ9vtmH8iMqGW dmbehyCjHZ50QwDet0Ackss4DDhvb/97hpDUDyQC4ELixe1zjaPgcojGZfroqfZZqGQ8XlTf7H5H bbL/jKthLMyyIyT1BolksWoHjU/KWZo3eCk3F2vDB412JttsNOYe2mI73/T/PXx14OlJbvLykKV1 iJN3aiFUTAhAwj1JKYRMwj1eualrGmNoCoSBEgvx1qRx8nfcCgsMO8Zi+4yOzf5ykoJ+s9Nfs/Ej H7yUSzqoiZ85akiXNSaJ5NEruKSegD/atC9Ed/2o227P4PcRExMVTw+x1WyFzw2DCBoNgOyb8Akb v5GvSo/OYwLHQ1iQ6BfCPhdRUOkcyZowwIQWuhO7GMxDppvPz9vePO45Tk9a2mlcXExRlDOpJUif F4DcFhYBCYqprnAwYSlw+QQvmwFkNtrrIYfwGNBy45N7DxNBiJnjgyWH3IsOmh9x4q7Rzy3a/KOl 4UYhmA+J+eOzjYzE4TRBJDxexJERmwXFXWrUJYHCUk1HTdPAXF0g1pGWS2iJIJY5hsTGwYNkPhQ1 zGZv5ZcJWU1FZElMiW2c5CKUpLvtsqSIiTmBYlpzmFpuNDhk1ZUxgCuFAvYPeMRo/eYHJYinPpYv P7zJyNSYXhAurY7EioQY4ESRwJZNiE0zQCsj5FL3h0/u8IxOFrlRHiemjwuMCYJdpvL++zxOlZqa dc++0iRXDhejQjSXMVMO0lCKgGCXjSRwv0646okR3ShufLoGESAVHqtOCKqddBtZUEXavErRe1Vi kIJwFCHCgmFkZkH93Q47m4vqEsUyNYwp8bul9bU/s5QDUkLeM22IbEJtkfHoRQ6+NXWKp9rW0wN4 LdwokE+Co7IZIcgPsUHQREerObMTDSp4KDTOjiBgISkrkedzLCqjkYiLLeGotm6fQVVhRYTvggJ6 nU0nvglYpP6s8HAjVERoKFQKjQudhkxzMLEVDmenm0kXFfSetcprOJrnQ8rNxebh9dRWDaMNXKbR bORR0/E2jV7USiAPGxjUR2MlG24a5JSKQvCpLxG8AVkqg3BzPi5yQQWtRMwDp3T9ZZ48NS1c0Iz9 S7DGbinMMMyeJkEj2griBlUslCg9YMbG2MbYzb5ICYwgwps6ixQ0G1Q/oXnyJHUJEk6Qxv5nA2+P mKG7tDh1oDWn9b7AKgsbTDKsUIz8x1tMSKS4FaRvSdgzZNrKuPQCMhsTMx1/CFphUnLsvDqunYag TaYD0721FQ0Jg2DagCyC1mYD9nm4Rx3HdyKJSZt220EXKxDRAb0PD5nkL2Hu7efLsF4HTgw64D4r xOncB9VDBZ5ndqdxZidggFhJKY0tcg8NYZcHO+6akxYyYVNoUSRBHwmnREIk0ChRoqiYW57NiCdC MbyztUyxl1MRSWA6fIvKkBeJMpEgyIPQA4c24OTEoHkIOuZsTbTGJDENDQLezB3pPx10ioWIv6cy TqcDBVExLyVs2r72kQrRbaK1WNGiYsotpSw6VX3uMHDcRg5UTHQ1DJdHDkkh0cMbKxZtIR4iwvlF t6iMdCKCxe+OENgOoDBQ36alN5JoFVAiyJMOIKD1sYNZsEnVYpjbtyorLB2JhgEsQQhg2EMCSAyX v7++c7E3vO7YLJYrZgszsnCSOsPA6ccC4JMkUFWJDA3za2AYtVxhNNI6rYLBELt7W229xoSzXuSQ wNV7l2c0FpOSa2i0Qx4OcrpUbtae5JXh4fF4lIqXkx6D7ICCXEPIFGtIxI7hsGR8Q7B6T+6EBedk KwW6jS3LkfkJdLEO3LS3MipFw0kwYtOV4E34JKBWVGisOXkYnPIFxEqjxsXmsxLqSyQ2Oxphyshp vZAR0dkHzPSNAqpJO7NTfwVUTGIaEYjXJdYSHQ7o1s4xREkKjJayJX5jQUo4HxtPdFEyxthT4IFd gdOrqJWvuB5FtJQMwVZJ9ayq2IjGqQyNjdmW06BMh3bnOozlmst4WjpXozJowKMZtgpXo6IrrgpZ R3WxV22TI6zEMr7oly6tNqIcTpKA2W1hFz8Cnq6OXRzR73FetObfWsnSpyBdwMl5QxG968kgtOba mks1EXGxGJL2iWq84gH02o8uQuR8hh361DEPTuRubbc61muyWQWWvhiGGW2hOcQpT4A9hhO88ph3 cAEkm+wNAgiLgSMtpf0fURhAsGljFptvlbSQ0oGuNSqpGKnZZKuFGErHRNhWvSAbajBG8Y/r4VIk r3RUgg4mStENEagbmVwpoj6sOfTxNYGfnddoVD19zn0VYhkIYHMOnpDeEIJJ17VJu04l0RD36WEo RA0vOsZ5cvCawOKzUmpiJZD9F2mpd2CUy4RYoiJqlJHKMjCTYwjHCRBKx4I3kjeavjfkbExtReC7 vO4ieuiF719jWxxfYq8UlyvQxu+RtIurd5Mgxks6BPR3d5gkjkC7BX+XbBYMOO1SNgwnxTvzc/N6 YokB5J+jFXM+Tq2hJT/Bz8TKzI3gTimqPI+pd3WdhSZwMEGhpV+j3xEElgMFp7jA8hKKExcXFfIP CLodsJc+6UpMTEDehAAtGMq4FpA98og4I3iWSqCSlsw1bBr//F3JFOFCQ1dEspA= --===============2089778045302035936==--