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

LP:727667 - Wrong result with OR + NOT NULL in maria-5.3

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: None
    • Labels:

      Description

      Not reproducible in maria-5.2. The following query:

      SELECT * FROM t1 WHERE (f3 = 83) OR (f10 = 'q' AND f3 IS NULL);

      returns

      ----------+

      f3 f10

      ----------+

      NULL r

      ----------+

      which is obviously wrong since neither f3 = 83 nor f10 = 'q'

      test case:

      CREATE TABLE t1 (
      f3 int(11),
      f10 varchar(1),
      KEY (f3)
      );
      INSERT IGNORE INTO t1 VALUES ('9','k'),(NULL,'r');

      SELECT * FROM t1 WHERE (f3 = 83) OR (f10 = 'z' AND f3 IS NULL);

      bzr version-info:

      revision-id: <email address hidden>
      date: 2011-03-01 10:22:22 +0300
      build-date: 2011-03-02 11:45:01 +0200
      revno: 2928
      branch-nick: maria-5.3

        Gliffy Diagrams

          Attachments

            Activity

            Hide
            psergey Sergei Petrunia added a comment -

            Re: Wrong result with OR + NOT NULL in maria-5.3
            In 5.3:

            MariaDB [j28]> explain SELECT * FROM t1 WHERE (f3 = 83) OR (f10 = 'z' AND f3 IS NULL);
            ---------------------------------------------------------------------+

            id select_type table type possible_keys key key_len ref rows Extra

            ---------------------------------------------------------------------+

            1 SIMPLE t1 ref_or_null f3 f3 5 const 2  

            ---------------------------------------------------------------------+
            1 row in set (0.01 sec)

            Note the lack of "Using where". Debugging shows that

            • WHERE clause is indeed not checked.
            • it is not there because it was removed by the "remove parts of WHERE that are guaranteed to be true by use of ref-access" functionality inside make_cond_for_table().
            Show
            psergey Sergei Petrunia added a comment - Re: Wrong result with OR + NOT NULL in maria-5.3 In 5.3: MariaDB [j28] > explain SELECT * FROM t1 WHERE (f3 = 83) OR (f10 = 'z' AND f3 IS NULL); --- ----------- ----- ----------- ------------- ---- ------- ----- ---- ------+ id select_type table type possible_keys key key_len ref rows Extra --- ----------- ----- ----------- ------------- ---- ------- ----- ---- ------+ 1 SIMPLE t1 ref_or_null f3 f3 5 const 2   --- ----------- ----- ----------- ------------- ---- ------- ----- ---- ------+ 1 row in set (0.01 sec) Note the lack of "Using where". Debugging shows that WHERE clause is indeed not checked. it is not there because it was removed by the "remove parts of WHERE that are guaranteed to be true by use of ref-access" functionality inside make_cond_for_table().
            Hide
            psergey Sergei Petrunia added a comment -

            Re: Wrong result with OR + NOT NULL in maria-5.3
            This is 5.2, for comparison:
            MariaDB [j28]> explain SELECT * FROM t1 WHERE (f3 = 83) OR (f10 = 'z' AND f3 IS NULL);
            ---------------------------------------------------------------------------+

            id select_type table type possible_keys key key_len ref rows Extra

            ---------------------------------------------------------------------------+

            1 SIMPLE t1 ref_or_null f3 f3 5 const 2 Using where

            ---------------------------------------------------------------------------+
            1 row in set (0.00 sec)

            Show
            psergey Sergei Petrunia added a comment - Re: Wrong result with OR + NOT NULL in maria-5.3 This is 5.2, for comparison: MariaDB [j28] > explain SELECT * FROM t1 WHERE (f3 = 83) OR (f10 = 'z' AND f3 IS NULL); --- ----------- ----- ----------- ------------- ---- ------- ----- ---- ------------+ id select_type table type possible_keys key key_len ref rows Extra --- ----------- ----- ----------- ------------- ---- ------- ----- ---- ------------+ 1 SIMPLE t1 ref_or_null f3 f3 5 const 2 Using where --- ----------- ----- ----------- ------------- ---- ------- ----- ---- ------------+ 1 row in set (0.00 sec)
            Hide
            psergey Sergei Petrunia added a comment -

            Re: Wrong result with OR + NOT NULL in maria-5.3
            Exact location of the problem:

            test_if_ref()/part_of_refkey() will indicate to make_cond_for_table() that condition "f3 = 83" will be always true, which actually is not satisfied by the (NULL,'r'), which will be returned by ref-or-null access.

            when make_cond_for_table() considers a WHERE clause of:

            (f3 = 83) OR (f10 = 'z' AND f3 IS NULL)

            and it is told that (f3 = 83) is universally true, it (correctly) concludes that entire WHERE condition is universally true, so there is no point in checking it.

            Show
            psergey Sergei Petrunia added a comment - Re: Wrong result with OR + NOT NULL in maria-5.3 Exact location of the problem: test_if_ref()/part_of_refkey() will indicate to make_cond_for_table() that condition "f3 = 83" will be always true, which actually is not satisfied by the (NULL,'r'), which will be returned by ref-or-null access. when make_cond_for_table() considers a WHERE clause of: (f3 = 83) OR (f10 = 'z' AND f3 IS NULL) and it is told that (f3 = 83) is universally true, it (correctly) concludes that entire WHERE condition is universally true, so there is no point in checking it.
            Hide
            psergey Sergei Petrunia added a comment -

            Re: Wrong result with OR + NOT NULL in maria-5.3
            So, currently test_if_ref()/part_of_refkey() assume that "f3=83" is universally true. This bug was introduced when backporting DS-MRR/ICP/other-related-code into 5.3.

            How it worked before the backport
            -------------------------------------------------
            Before the backport, the part_of_refkey() looked like this:

            ...

            { KEY_PART_INFO *key_part= table->key_info[table->reginfo.join_tab->ref.key].key_part; for (uint part=0 ; part < ref_parts ; part++,key_part++) if (field->eq(key_part->field) && !(key_part->key_part_flag & (HA_PART_KEY_SEG | HA_NULL_PART))) return table->reginfo.join_tab->ref.items[part]; }

            The important part here: we check HA_NULL_PART bit. If it is present, equality will not be removed. It serves two purposes:
            P1. Handle the case where we get NULL value from ref.items[part], make a lookup in the table, and get a matching record with NULLs. The equality will be checked and will filter the record out.

            P2. Also cover ref_or_null access method. When ref_or_null is used, table access can return either the lookup value or a record with NULLs, so the equality is not universally guaranteed. ref_or_null is only employed for NULL-able columns (no point to look for NULLs in non-NULLable column), so if we keep the equality for NULLable columns, we cut off ref_or_null, too.

            Show
            psergey Sergei Petrunia added a comment - Re: Wrong result with OR + NOT NULL in maria-5.3 So, currently test_if_ref()/part_of_refkey() assume that "f3=83" is universally true. This bug was introduced when backporting DS-MRR/ICP/other-related-code into 5.3. How it worked before the backport ------------------------------------------------- Before the backport, the part_of_refkey() looked like this: ... { KEY_PART_INFO *key_part= table->key_info[table->reginfo.join_tab->ref.key].key_part; for (uint part=0 ; part < ref_parts ; part++,key_part++) if (field->eq(key_part->field) && !(key_part->key_part_flag & (HA_PART_KEY_SEG | HA_NULL_PART))) return table->reginfo.join_tab->ref.items[part]; } The important part here: we check HA_NULL_PART bit. If it is present, equality will not be removed. It serves two purposes: P1. Handle the case where we get NULL value from ref.items [part] , make a lookup in the table, and get a matching record with NULLs. The equality will be checked and will filter the record out. P2. Also cover ref_or_null access method. When ref_or_null is used, table access can return either the lookup value or a record with NULLs, so the equality is not universally guaranteed. ref_or_null is only employed for NULL-able columns (no point to look for NULLs in non-NULLable column), so if we keep the equality for NULLable columns, we cut off ref_or_null, too.
            Hide
            psergey Sergei Petrunia added a comment -

            Re: Wrong result with OR + NOT NULL in maria-5.3
            Why it got broken
            ---------------------------

            Somewhere along with DS-MRR/ICP works, we've added "Early/Late NULLs filtering", which would cut off the case of P1 form the previous post. That is, if we have a ref access on "t.key=col" and we get NULL for col, the new code will not search for NULL value of t.key, and so will not need the equality check to filter out NULL results. We forgot about case p2, though.

            The implementation in the code was odd. It worked as follows: the check in part_of_refkey() looked the same:

            !(key_part->key_part_flag & (HA_PART_KEY_SEG | HA_NULL_PART)))
            return table->reginfo.join_tab->ref.items[part];

            However, the part that sets the HA_PART_KEY_SEG flag was gone. It had been located in table.cc:open_binary_frm() and looked like this:

            ...
            to_be_deleted:

            /*
            If the field can be NULL, don't optimize away the test
            key_part_column = expression from the WHERE clause
            as we need to test for NULL = NULL.
            */
            if (field->real_maybe_null())
            key_part->key_part_flag|= HA_NULL_PART;
            ...

            so I went ahead and deleted that code. The effect was as desired: lots equalities weren't checked anymore, and lots of "Using where" were gone from ref access EXPLAINs.

            Show
            psergey Sergei Petrunia added a comment - Re: Wrong result with OR + NOT NULL in maria-5.3 Why it got broken --------------------------- Somewhere along with DS-MRR/ICP works, we've added "Early/Late NULLs filtering", which would cut off the case of P1 form the previous post. That is, if we have a ref access on "t.key=col" and we get NULL for col, the new code will not search for NULL value of t.key, and so will not need the equality check to filter out NULL results. We forgot about case p2, though. The implementation in the code was odd. It worked as follows: the check in part_of_refkey() looked the same: !(key_part->key_part_flag & (HA_PART_KEY_SEG | HA_NULL_PART))) return table->reginfo.join_tab->ref.items [part] ; However, the part that sets the HA_PART_KEY_SEG flag was gone. It had been located in table.cc:open_binary_frm() and looked like this: ... to_be_deleted: /* If the field can be NULL, don't optimize away the test key_part_column = expression from the WHERE clause as we need to test for NULL = NULL. */ if (field->real_maybe_null()) key_part->key_part_flag|= HA_NULL_PART; ... so I went ahead and deleted that code. The effect was as desired: lots equalities weren't checked anymore, and lots of "Using where" were gone from ref access EXPLAINs.
            Hide
            psergey Sergei Petrunia added a comment -

            Re: Wrong result with OR + NOT NULL in maria-5.3
            What is in the fix
            -------------------------

            When I've tried to remove HA_NULL_PART flag completely (so that we don't have checks for something we don't set)
            I discovered that the flag is used in MyISAM under some circumstances.

            So, the solution is:

            • Put the code that sets HA_NULL_PART flag back.
            • Modify the check in part_of_refkey() so that it
              1. removes the equality in case P1
              2. keeps the equality in case P2

            in other words:

            1. NULL-ability of the lookup column should not prevent the removal of ref-equality.
            2. but if ref_or_null access will alternate the column's value between the lookup value and NULL, then the equality should not be removed.

            Show
            psergey Sergei Petrunia added a comment - Re: Wrong result with OR + NOT NULL in maria-5.3 What is in the fix ------------------------- When I've tried to remove HA_NULL_PART flag completely (so that we don't have checks for something we don't set) I discovered that the flag is used in MyISAM under some circumstances. So, the solution is: Put the code that sets HA_NULL_PART flag back. Modify the check in part_of_refkey() so that it 1. removes the equality in case P1 2. keeps the equality in case P2 in other words: 1. NULL-ability of the lookup column should not prevent the removal of ref-equality. 2. but if ref_or_null access will alternate the column's value between the lookup value and NULL, then the equality should not be removed.
            Hide
            ratzpo Rasmus Johansson added a comment -

            Launchpad bug id: 727667

            Show
            ratzpo Rasmus Johansson added a comment - Launchpad bug id: 727667

              People

              • Assignee:
                psergey Sergei Petrunia
                Reporter:
                philipstoev Philip Stoev
              • Votes:
                0 Vote for this issue
                Watchers:
                0 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: