WordPress / sqlite-database-integration

Feature Plugin to add SQLite support to WordPress. Under Development.
GNU General Public License v2.0
202 stars 30 forks source link

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

Open morganwdavis opened 3 weeks ago

morganwdavis commented 3 weeks ago

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:

aristath commented 3 weeks ago

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 commented 1 week ago

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: "wpoptions" - 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.