sqlite-database-integration icon indicating copy to clipboard operation
sqlite-database-integration copied to clipboard

SQL syntax error in get_keys() breaks ON DUPLICATE KEY translation

Open morganwdavis opened this issue 1 year ago • 2 comments

There are critical errors in class-wp-sqlite-translator.php that prevent it from running correctly and subsequently cause a ton of error debug messages to dump into php.log on every page request. Here is the fix for sites running relatively newer versions of PHP/Sqlite3 which are sensitive to this (apparently, older versions are immune which is why this bug seems to evade some developers).

Two SELECT statements in get_keys() need to have their quotes reversed (I reported this in another issue but I have since closed it for clarity here). Enclosing a table name in double quotes is not valid in SQLite:

INCORRECT: 'SELECT * FROM pragma_index_list("' . $table_name . '") as l;'

CORRECT: "SELECT * FROM pragma_index_list('" . $table_name . "') as l;"

Because of this error, the code for translating ON DUPLICATE KEY in translate_on_duplicate_key() never gets a chance to execute when it calls $this->get_keys().

Also, a minor optimization in that function. These three skip calls aren't needed since the while loop does the same job.

        if ( ! $conflict_columns ) {
            // Drop the consumed "ON".
            $this->rewriter->drop_last();
            // Skip over "DUPLICATE", "KEY", and "UPDATE".
            // $this->rewriter->skip();
            // $this->rewriter->skip();
            // $this->rewriter->skip();
            while ( $this->rewriter->skip() ) {
                // Skip over the rest of the query.
            }
            return;
        }

Once the above fixes are made, php.log no longer spews ton of lines of debug on every page request. It also allowed some plugins to update their settings properly (e.g., WP Armour) which previously was breaking.

Works on:

  • PHP 8.2.19
  • SQLite3 3.46.0

morganwdavis avatar Jun 06 '24 20:06 morganwdavis

I only did a quick test for this and it seems to be fixing the issue, but before making the change, I'd appreciate some feedback from @adamziel on this one 👍

aristath avatar Jun 10 '24 07:06 aristath

There are more of these lurking. The fix I suggested above knocks out a ton of errors, but I'm still getting them. Here's an example. The debug message even hints at possible quotation issues around the table names:

no such column: "wp_options" - should this be a string literal in single-quotes?

Complete error below:

[21-Jun-2024 05:32:45 UTC] WordPress database error <div style="clear:both">&nbsp;</div>
<div class="queries" style="clear:both;margin-bottom:2px;border:red dotted thin;">
<p>MySQL query:</p>
<p>DELETE a, b FROM wp_options a, wp_options b
                                WHERE a.option_name LIKE '_site_transient_%'
                                AND a.option_name NOT LIKE '_site_transient_timeout_%'
                                AND b.option_name = CONCAT( '_site_transient_timeout_', SUBSTRING( a.option_name, 17 ) )
                                AND b.option_value < 1718947965</p>
<p>Queries made or created this session were:</p>
<ol>
<li>Executing: BEGIN | (no parameters)</li>
<li>Executing: SELECT l.name FROM pragma_table_info(&quot;wp_options&quot;) as l WHERE l.pk = 1; | (no parameters)</li>
<li>Executing: ROLLBACK | (no parameters)</li>
</ol>
</div>
<div style="clear:both;margin-bottom:2px;border:red dotted thin;" class="error_message" style="border-bottom:dotted blue thin;">
Error occurred at line 3777 in Function <code>handle_error</code>. Error message was: SQLSTATE[HY000]: General error: 1 no such column: "wp_options" - should this be a string literal in single-quotes?.
</div>
<p>Backtrace:</p>
<pre>#0 /path/to/blog/wp-content/plugins/sqlite-database-integration/wp-includes/sqlite/class-wp-sqlite-db.php(287): WP_SQLite_Translator->get_error_message()
#1 /path/to/blog/wp-includes/option.php(1410): WP_SQLite_DB->query('DELETE a, b FRO...')
#2 /path/to/blog/wp-includes/class-wp-hook.php(324): delete_expired_transients()
#3 /path/to/blog/wp-includes/class-wp-hook.php(348): WP_Hook->apply_filters('', Array)
#4 /path/to/blog/wp-includes/plugin.php(565): WP_Hook->do_action(Array)
#5 /path/to/blog/wp-cron.php(191): do_action_ref_array('delete_expired_...', Array)
#6 {main}</pre>
 for query DELETE a, b FROM wp_options a, wp_options b
                                WHERE a.option_name LIKE '_site_transient_%'
                                AND a.option_name NOT LIKE '_site_transient_timeout_%'
                                AND b.option_name = CONCAT( '_site_transient_timeout_', SUBSTRING( a.option_name, 17 ) )
                                AND b.option_value < 1718947965 made by do_action_ref_array('delete_expired_transients'), WP_Hook->do_action, WP_Hook->apply_filters, delete_expired_transients, WP_SQLite_DB->query, WP_SQLite_DB->print_error

Hopefully the WP core team can get more behind SQLite as a storage option and start to reengineer a lot of the query expressions that assume MySQL that require this translation layer. SQLite makes more sense for the vast majority of WP sites in my experience.

morganwdavis avatar Jun 21 '24 06:06 morganwdavis