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

Prepared statement return wrong result (missing row)

Details

    Description

      In the example below, we have found that the query doesn't always return the correct count of rows when NULL is passed in as the prepared statement parameters. If NULL is used for the first call, then the results are always correct. (This is the second case). If a non NULL parameter is used, then a future call with NULL will return the wrong result set.

      This form of SQL is used in our software for reports, in which a parameter could be set to NULL. The 2 parameters are always the same value for a particular execution.

      We have additionally found that using the null safe comparator <=> on the string comparison, stops the problem from occurring.

      This problem was originally noted using the Maria JDBC connector. With 1.3.6 the code works, but was broken in 1.3.7. I am currently trying to determine the difference here, as to my knowledge both Connector/J versions should use server Prepared Statements.

      CREATE TABLE a (a_id INT AUTO_INCREMENT PRIMARY KEY, a_text VARCHAR(20));
      CREATE TABLE b (b_id INT AUTO_INCREMENT PRIMARY KEY, b_a_id INT);
       
      INSERT INTO a VALUES (NULL, 'word1');
      INSERT INTO b VALUES (NULL, 1);
      INSERT INTO b VALUES (NULL, NULL);
       
      PREPARE q FROM 'SELECT * FROM b
       LEFT JOIN a
        ON (a.a_id = b.b_a_id)
       
      WHERE ((? IS NULL) OR (a.a_text = ?))';
      SET @var = 'word1';
      EXECUTE q USING @var, @var;
      /* expect row count 1, actual row count 1 */
      EXECUTE q USING @nul, @nul;
      /* expect row count = 2, actual row count 1 */
       
      PREPARE q2 FROM 'SELECT * FROM b
       LEFT JOIN a
        ON (a.a_id = b.b_a_id)
       
      WHERE ((? IS NULL) OR (a.a_text = ?))';
      SET @var = 'word1';
      EXECUTE q2 USING @nul, @nul;
      /* expect row count 2, actual row count 2 */
      EXECUTE q2 USING @var, @var;
      /* expect row count 1, actual row count 1 */
       
      DROP TABLE b, a;
      

      Attachments

        Activity

          Note, that I guessed that the issue was relating to the optimiser, and hence put the component on. Please remove if not the case.

          brendon Brendon Abbott added a comment - Note, that I guessed that the issue was relating to the optimiser, and hence put the component on. Please remove if not the case.

          Thanks for the report and the test case.

          Apparently the problem appeared on 5.5 tree with the following revision:

          commit 04684b7709f55a5a9de9226e834bcfbed05ee5c0 6fb1786584bc85db84e7ad184d6162d3208068c0
          Author: Sergey Petrunya <psergey@askmonty.org>
          Date:   Wed Jul 31 17:24:52 2013 +0400
           
              MDEV-4817: Optimizer fails to optimize expression of the form 'FOO' IS NULL
              - Modify the way Item_cond::fix_fields() and Item_cond::eval_not_null_tables()
                calculate bitmap for Item_cond_or::not_null_tables():
                if they see a "... OR inexpensive_const_false_item OR ..." then the item can
                be ignored.
              - Updated test results. There can be more warnings produced since parts of WHERE
                are evaluated more times.
          

          elenst Elena Stepanova added a comment - Thanks for the report and the test case. Apparently the problem appeared on 5.5 tree with the following revision: commit 04684b7709f55a5a9de9226e834bcfbed05ee5c0 6fb1786584bc85db84e7ad184d6162d3208068c0 Author: Sergey Petrunya <psergey@askmonty.org> Date: Wed Jul 31 17:24:52 2013 +0400   MDEV-4817: Optimizer fails to optimize expression of the form 'FOO' IS NULL - Modify the way Item_cond::fix_fields() and Item_cond::eval_not_null_tables() calculate bitmap for Item_cond_or::not_null_tables(): if they see a "... OR inexpensive_const_false_item OR ..." then the item can be ignored. - Updated test results. There can be more warnings produced since parts of WHERE are evaluated more times.
          alice Alice Sherepa added a comment -

          repeatable on the current 10.3 (4e9206736c403206915c09d)-10.10

          alice Alice Sherepa added a comment - repeatable on the current 10.3 (4e9206736c403206915c09d)-10.10
          monty Michael Widenius added a comment - - edited

          The problem is that the first execution of the prepared statement makes
          a permanent optimisation of converting the LEFT JOIN to an INNER JOIN.

          This is based on the assumption that all the user parameters are
          always constants and that parameters to Item_cond() will not change value
          from true and false between different executions.

          (The example was using IS NULL, which will change value if parameter depending
          on if the parameter is NULL or not).

          The fix is to change Item_cond::fix_fields() and
          Item_cond::eval_not_null_tables() to not threat user parameters as
          constants. This will ensure that we don't do the LEFT_JOIN -> INNER
          JOIN conversion that causes problems.

          There is also some things that needs to be improved regarding
          calculations of not_null_tables_cache as we get a different value for
          WHERE 1 or t1.a=1
          compared to
          WHERE t1.a= or 1

          We are doing the fix in 10.6 as the changes in 10.4 would be much bigger than in 10.6 and a lot of the 10.4 code would not be usable for 10.6.

          monty Michael Widenius added a comment - - edited The problem is that the first execution of the prepared statement makes a permanent optimisation of converting the LEFT JOIN to an INNER JOIN. This is based on the assumption that all the user parameters are always constants and that parameters to Item_cond() will not change value from true and false between different executions. (The example was using IS NULL, which will change value if parameter depending on if the parameter is NULL or not). The fix is to change Item_cond::fix_fields() and Item_cond::eval_not_null_tables() to not threat user parameters as constants. This will ensure that we don't do the LEFT_JOIN -> INNER JOIN conversion that causes problems. There is also some things that needs to be improved regarding calculations of not_null_tables_cache as we get a different value for WHERE 1 or t1.a=1 compared to WHERE t1.a= or 1 We are doing the fix in 10.6 as the changes in 10.4 would be much bigger than in 10.6 and a lot of the 10.4 code would not be usable for 10.6.

          Waiting for final review from Sergei Petrunia

          monty Michael Widenius added a comment - Waiting for final review from Sergei Petrunia

          Review done. Some input provided on Slack.

          psergei Sergei Petrunia added a comment - Review done. Some input provided on Slack.

          Pushed to 10.6

          monty Michael Widenius added a comment - Pushed to 10.6

          People

            monty Michael Widenius
            brendon Brendon Abbott
            Votes:
            0 Vote for this issue
            Watchers:
            7 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.