Uploaded image for project: 'MariaDB Server'
  1. MariaDB Server
  2. MDEV-4355

Wrong result (extra row) with AND and OR conditions

Details

    Description

      It is most likely a regression introduced (or made visible) by the following revision:

      revno: 3628
      revision-id: igor@askmonty.org-20130225031611-jk8lyhhjazov66qc
      committer: Igor Babaev <igor@askmonty.org>
      message:
        Fixed bug mdev-4177

      I cannot verify it with 100% certainty because the provided test case causes a crash on revisions 3628..3634; but it produces a correct result on maria/5.3 up to revno 3627 and a wrong result starting from revno 3635 and up to (and including) the current 3646.
      Also reproducible on current maria/5.5.

      Test case:

      CREATE TABLE t1 (a INT, b INT) ENGINE=MyISAM;
      INSERT INTO t1 VALUES (1,101),(2,102),(3,103),(4,104),(5,NULL);
       
      SELECT * FROM t1
      WHERE ( NULL OR a = 5 ) AND ( b != 1 OR a = 1 );

      Actual result:

      a	b
      5	NULL

      Expected result - empty set:

      a	b

      Inconsistency of the actual result can be confirmed by executing the second part of AND separately:

      SELECT * FROM t1
      WHERE b != 1 OR a = 1;
      a	b
      1	101
      2	102
      3	103
      4	104

      It is a correct result, and it doesn't include the row with a=5, so the result set for the bigger query cannot include it either (but currently it does).

      Reproducible with the default optimizer_switch as well as with all OFF values.

      EXPLAIN:

      EXPLAIN EXTENDED
      SELECT * FROM t1
      WHERE ( NULL OR a = 5 ) AND ( b != 1 OR a = 1 );
      id	select_type	table	type	possible_keys	key	key_len	ref	rows	filtered	Extra
      1	SIMPLE	t1	ALL	NULL	NULL	NULL	NULL	5	100.00	Using where
      Warnings:
      Note	1003	select `test`.`t1`.`a` AS `a`,`test`.`t1`.`b` AS `b` from `test`.`t1` where ((`test`.`t1`.`a` = 5) and ((`test`.`t1`.`b` <> 1) or 1))

      Attachments

        Issue Links

          Activity

            I came up with a fix, not sure if it solves the real problem...

            === modified file 'sql/sql_select.cc'
            — sql/sql_select.cc 2013-06-05 20:53:35 +0000
            +++ sql/sql_select.cc 2013-07-06 17:33:40 +0000
            @@ -12134,8 +12134,11 @@
            TABLE_LIST *native_sjm= embedding_sjm(item_equal->context_field);
            if (item_const && upper->get_const())

            { - /* Upper item also has "field_item=const". Don't produce equality here */ - item= 0; + if (!item_const->eq(upper->get_const(), false)) + return new Item_int((longlong)0, 1); + else + /* Upper item also has "field_item=const". Don't produce equality here */ + item= 0; }

            else
            {

            pomyk Patryk Pomykalski added a comment - I came up with a fix, not sure if it solves the real problem... === modified file 'sql/sql_select.cc' — sql/sql_select.cc 2013-06-05 20:53:35 +0000 +++ sql/sql_select.cc 2013-07-06 17:33:40 +0000 @@ -12134,8 +12134,11 @@ TABLE_LIST *native_sjm= embedding_sjm(item_equal->context_field); if (item_const && upper->get_const()) { - /* Upper item also has "field_item=const". Don't produce equality here */ - item= 0; + if (!item_const->eq(upper->get_const(), false)) + return new Item_int((longlong)0, 1); + else + /* Upper item also has "field_item=const". Don't produce equality here */ + item= 0; } else {

            paste from the IRC:

            <serg> can you explain what the problem is?
            <igor> the problem arises in the following situation:
            you have f1=f2/const OR FALSE (or always FALSE disjunct)
            you also have other AND equalities involving f1.
            the tower of equalities (in AND OR branches) is built before we
            simplify conditions (in particular remove always FALSE disjuncts)
            as a result after simplification we must have new sets of
            equalities and they must be consistent in any respect.
            now they are not. this might cause any kind of problems when we
            process these conditions.
            the proper solution would be to propagate new AND equalities to
            the upper levels.
            btw on april 30 I tried to fix mdev-4355, spent more than a day
            and realized that it would take me much more.
            in a general case the procedure of equality propagation from a
            degenerated OR branches is recursive, because new equality can
            form new always FALSE disjuct oon upper levels of AND-OR formula

            serg Sergei Golubchik added a comment - paste from the IRC: <serg> can you explain what the problem is? <igor> the problem arises in the following situation: you have f1=f2/const OR FALSE (or always FALSE disjunct) you also have other AND equalities involving f1. the tower of equalities (in AND OR branches) is built before we simplify conditions (in particular remove always FALSE disjuncts) as a result after simplification we must have new sets of equalities and they must be consistent in any respect. now they are not. this might cause any kind of problems when we process these conditions. the proper solution would be to propagate new AND equalities to the upper levels. btw on april 30 I tried to fix mdev-4355, spent more than a day and realized that it would take me much more. in a general case the procedure of equality propagation from a degenerated OR branches is recursive, because new equality can form new always FALSE disjuct oon upper levels of AND-OR formula

            Maybe mdev-4177 should be reverted until it's fixed properly?

            pomyk Patryk Pomykalski added a comment - Maybe mdev-4177 should be reverted until it's fixed properly?

            Patryk,
            We are working on the problem.
            Do you have any real life query that has problems due to this bug?
            So far we had complains of this bug only from RQG.
            We can't just revert the patch for mdev-4177, because it fixes some other problems.

            igor Igor Babaev (Inactive) added a comment - Patryk, We are working on the problem. Do you have any real life query that has problems due to this bug? So far we had complains of this bug only from RQG. We can't just revert the patch for mdev-4177, because it fixes some other problems.

            We haven't upgraded to MariaDB yet, just testing it in dev environment. No problems yet, but it's hard to test everything with ~20 applications and a lot of generated queries. We hope to migrate to 5.5.32, thanks for the good work

            pomyk Patryk Pomykalski added a comment - We haven't upgraded to MariaDB yet, just testing it in dev environment. No problems yet, but it's hard to test everything with ~20 applications and a lot of generated queries. We hope to migrate to 5.5.32, thanks for the good work

            The fix for this bug was pushed into the 5.3 tree and later merged into the 5.5 tree.

            igor Igor Babaev (Inactive) added a comment - The fix for this bug was pushed into the 5.3 tree and later merged into the 5.5 tree.

            People

              igor Igor Babaev (Inactive)
              elenst Elena Stepanova
              Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:

                Git Integration

                  Error rendering 'com.xiplink.jira.git.jira_git_plugin:git-issue-webpanel'. Please contact your Jira administrators.