Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
morganwdavis opened this issue Jun 6, 2024 · 2 comments
Open

Comments

@morganwdavis
Copy link

morganwdavis commented Jun 6, 2024

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
@aristath
Copy link
Member

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 👍

@morganwdavis
Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants