Add wpdb fallback for wp db query and wp db import when mysql/mariadb binary is unavailable - #320
Add wpdb fallback for wp db query and wp db import when mysql/mariadb binary is unavailable#320swissspidy with Copilot wants to merge 35 commits into
wp db query and wp db import when mysql/mariadb binary is unavailable#320Conversation
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
…lable Agent-Logs-Url: https://github.com/wp-cli/db-command/sessions/8996fc65-2840-4792-8cf4-ddc700a7c040 Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com>
Agent-Logs-Url: https://github.com/wp-cli/db-command/sessions/8996fc65-2840-4792-8cf4-ddc700a7c040 Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com>
wp db query when mysql/mariadb binary is unavailable
This comment was marked as resolved.
This comment was marked as resolved.
Agent-Logs-Url: https://github.com/wp-cli/db-command/sessions/2fe9988f-095f-4c61-b162-5bf4f8956cca Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com>
This comment was marked as resolved.
This comment was marked as resolved.
wp db query when mysql/mariadb binary is unavailablewp db query, wp db import, wp db create, wp db drop, and wp db reset when mysql/mariadb binary is unavailable
This comment was marked as resolved.
This comment was marked as resolved.
Agent-Logs-Url: https://github.com/wp-cli/db-command/sessions/fa41acc5-de2a-4dc3-9f7a-f5b282ed670a Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com>
This comment was marked as resolved.
This comment was marked as resolved.
wp db query, wp db import, wp db create, wp db drop, and wp db reset when mysql/mariadb binary is unavailablewp db query, wp db import, wp db drop, and wp db reset when mysql/mariadb binary is unavailable
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthrough
ChangesMySQL CLI fallback
Estimated code review effort: 4 (Complex) | ~45 minutes Suggested labels: Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant WPCLI
participant DB_Command
participant wpdb
participant Database
WPCLI->>DB_Command: run db query or db import
DB_Command->>DB_Command: detect missing MySQL/MariaDB binary
DB_Command->>wpdb: execute query or split import statements
wpdb->>Database: send SQL
Database-->>wpdb: return rows, status, or errors
wpdb-->>DB_Command: return execution results
DB_Command-->>WPCLI: print output and fallback warning
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
src/DB_Command.php (1)
1977-1997: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winUses a fresh connection per DDL call — fine for single calls, costly in
clean()'s loop.
run_query()now routes everyrun_query()caller through this method when the binary is unavailable, includingclean(), which callsrun_query()once per table (Lines 235-244). Each call opens and closes a brand-new mysqli connection just to run oneDROP TABLE. For large installs with many tables this multiplies connection overhead unnecessarily.♻️ Suggested direction
Accept an optional existing
mysqliconnection (or expose a connection-reuse variant) so callers that issue many DDL statements in a loop (likeclean()) can reuse a single connection instead of opening one per statement.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/DB_Command.php` around lines 1977 - 1997, Update run_query_via_mysqli() to accept an optional existing mysqli connection, creating and closing its own connection only when one is not provided. Modify clean() to obtain one reusable connection before its table-drop loop and pass it to each run_query_via_mysqli() call, then close it after the loop while preserving existing error handling.features/db-query.feature (1)
158-183: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winScenario correctly matches the fallback's debug message; consider adding SQL-mode-compat coverage.
This scenario is well-formed and the previously requested
env PATH=...fix is already applied. Given the critical SQL-mode-compatibility gap identified inwpdb_query()/wpdb_import()(src/DB_Command.php), consider adding a fallback scenario analogous to "wp db queryadapts the SQL mode by default without a separate mode probe" (Lines 100-113) but using the fake-bin technique, so a fix to that gap is actually exercised by CI.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@features/db-query.feature` around lines 158 - 183, Add a fake-bin fallback scenario in features/db-query.feature that verifies wp db query adapts SQL mode by default without a separate mode probe, using the same PATH override and unavailable mysql/mariadb binaries as the current scenario. Ensure the assertions exercise the compatibility behavior implemented by DB_Command::wpdb_query() or DB_Command::wpdb_import(), including the expected query result and debug output.features/db-import.feature (1)
72-99: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winScenario structure is correct but doesn't reproduce the known regression.
This test imports a plain export on what is likely a lenient-SQL-mode CI database, so it wouldn't fail even with the missing SQL-mode-compat statement in
wpdb_import()(see src/DB_Command.php). Consider mirroring "wp db importloads a dump containing legacy zero-date values" (Lines 323-344) but through the fake-bin/mysqli fallback path, since that's the exact scenario that previously surfaced "Invalid default value for 'comment_date'" per the PR history.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@features/db-import.feature` around lines 72 - 99, The fallback scenario should import a dump containing legacy zero-date values so it exercises the SQL-mode compatibility logic in DB_Command::wpdb_import(), rather than importing a plain export. Adapt the existing fake-bin/mysql and fake-bin/mariadb unavailable setup to use the zero-date dump content and retain assertions for successful import and the fallback warning.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/DB_Command.php`:
- Around line 2593-2629: Apply get_sql_mode_compat_statement( $assoc_args ) to
the fallback connection before executing SQL in both src/DB_Command.php lines
2593-2629 within wpdb_import() and lines 2535-2583 within wpdb_query(); each
site requires the same compatibility statement before the split statements or
query runs.
- Around line 2008-2013: Update the run_query() fallback around
is_mysql_binary_available() so database creation does not route through
run_query_via_mysqli() without a selected database. Exclude the create()
operation from this fallback, preserving the existing mysqli behavior for other
queries, or explicitly align the command documentation and design with
supporting create() through that path.
- Around line 2481-2524: Update get_db_connection() to use the command’s
effective CLI database username and password values, including --dbuser and
--dbpass overrides, instead of always reading DB_USER and DB_PASSWORD. Handle
mysqli connection exceptions so failures reach the existing WP_CLI::error()
path, and ensure fallback query operations similarly tolerate or catch mysqli
exceptions before relying on connect_errno/query checks.
---
Nitpick comments:
In `@features/db-import.feature`:
- Around line 72-99: The fallback scenario should import a dump containing
legacy zero-date values so it exercises the SQL-mode compatibility logic in
DB_Command::wpdb_import(), rather than importing a plain export. Adapt the
existing fake-bin/mysql and fake-bin/mariadb unavailable setup to use the
zero-date dump content and retain assertions for successful import and the
fallback warning.
In `@features/db-query.feature`:
- Around line 158-183: Add a fake-bin fallback scenario in
features/db-query.feature that verifies wp db query adapts SQL mode by default
without a separate mode probe, using the same PATH override and unavailable
mysql/mariadb binaries as the current scenario. Ensure the assertions exercise
the compatibility behavior implemented by DB_Command::wpdb_query() or
DB_Command::wpdb_import(), including the expected query result and debug output.
In `@src/DB_Command.php`:
- Around line 1977-1997: Update run_query_via_mysqli() to accept an optional
existing mysqli connection, creating and closing its own connection only when
one is not provided. Modify clean() to obtain one reusable connection before its
table-drop loop and pass it to each run_query_via_mysqli() call, then close it
after the loop while preserving existing error handling.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 147dfdef-c9e6-4d69-9dda-8223e5fcbc7f
📒 Files selected for processing (4)
features/db-import.featurefeatures/db-query.featurefeatures/db.featuresrc/DB_Command.php
swissspidy
changed the title
wp db query, wp db import, wp db drop, and wp db reset when mysql/mariadb binary is unavailablewp db query and wp db import when mysql/mariadb binary is unavailable
a7665f9 to
30dfd4f
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/DB_Command.php (1)
602-619: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winDocument the wpdb-fallback behavior and its limitations in the
query/importdocblocks. Both commands gained a significant new code path (automatic fallback to$wpdbwhen the mysql/mariadb binary is unavailable) with real behavioral differences from the mysql-client path, but neither docblock mentions it.
src/DB_Command.php#L602-L619: add a note toquery()'s docblock that when the mysql/mariadb binary is unavailable, the query runs through$wpdbinstead,--dbuser/--dbpass/other mysql-specific options are not honored in that mode, and multi-statement input (e.g.wp db query < file.sql) is not currently supported via this fallback.src/DB_Command.php#L955-L977: add a note toimport()'s docblock that the wpdb fallback ignores--dbuser/--dbpass, and that dumps relying on customDELIMITERblocks (stored routines/triggers) or server-level DDL are not supported by the fallback.As per path instructions,
**/*.php: "Update relevant inline code documentation when changes affect user-facing functionality."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/DB_Command.php` around lines 602 - 619, Update the query() docblock at src/DB_Command.php lines 602-619 to document that unavailable mysql/mariadb binaries trigger execution through $wpdb, mysql-specific options including --dbuser and --dbpass are ignored, and multi-statement input is unsupported. Update the import() docblock at src/DB_Command.php lines 955-977 to document that the fallback ignores --dbuser/--dbpass and does not support custom DELIMITER blocks or server-level DDL.Source: Coding guidelines
♻️ Duplicate comments (2)
src/DB_Command.php (2)
2501-2507: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
--dbuser/--dbpass(and other connection overrides) are silently ignored by the wpdb fallback.
maybe_load_wpdb()always instantiateswpdbwith the rawDB_USER/DB_PASSWORD/DB_HOSTconstants, with no way to honor--dbuser/--dbpasspassed towp db query/wp db import. This is the same underlying gap flagged on the previous mysqli-based implementation (get_db_connection()readingDB_USER/DB_PASSWORDdirectly), just carried into the new wpdb-based fallback.🛠️ Suggested direction
- protected function maybe_load_wpdb() { + protected function maybe_load_wpdb( $assoc_args = [] ) { ... - $wpdb = new wpdb( DB_USER, DB_PASSWORD, DB_NAME, DB_HOST ); + $dbuser = Utils\get_flag_value( $assoc_args, 'dbuser', DB_USER ); + $dbpass = Utils\get_flag_value( $assoc_args, 'dbpass', DB_PASSWORD ); + $wpdb = new wpdb( $dbuser, $dbpass, DB_NAME, DB_HOST );and pass
$assoc_argsthrough fromwpdb_query()/wpdb_import().🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/DB_Command.php` around lines 2501 - 2507, Update maybe_load_wpdb() to accept the connection override arguments and use them when constructing the fallback wpdb instance instead of always using DB_USER, DB_PASSWORD, DB_NAME, and DB_HOST directly. Pass $assoc_args through from wpdb_query() and wpdb_import(), preserving the constants as defaults when overrides are absent.
2585-2606: 🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick winSQL-mode compatibility statement is missing from both wpdb fallback executors. Neither
wpdb_import()norwpdb_query()appliesget_sql_mode_compat_statement( $assoc_args )on the fallback connection, unlike the CLI path's--init-command. This is the same root cause previously flagged, and matches the PR history's reported persisting "Invalid default value for 'comment_date'" import failure across WordPress versions.
src/DB_Command.php#L2585-L2606: inwpdb_import(), run$this->get_sql_mode_compat_statement( $assoc_args )via$wpdb->query()before executing the split statements.src/DB_Command.php#L2519-L2575: inwpdb_query(), run the same compatibility statement before executing$query.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/DB_Command.php` around lines 2585 - 2606, Apply the SQL-mode compatibility statement to both fallback executors before running SQL: in src/DB_Command.php lines 2585-2606, update wpdb_import() to execute get_sql_mode_compat_statement( $assoc_args ) through $wpdb->query() before the split statements; likewise in src/DB_Command.php lines 2519-2575, update wpdb_query() to execute the same statement before $query.
🧹 Nitpick comments (1)
src/DB_Command.php (1)
2656-2751: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winStatement splitter doesn't handle custom
DELIMITERblocks (stored routines/triggers).
split_sql_statements()correctly tracks quotes/comments/conditional comments, but has no concept of a customDELIMITER(e.g.DELIMITER ;;used by dumps with--routines/--triggers). Any embedded;inside such a routine body would be split as a statement boundary, corrupting the routine definition. Given--skip-optimizationaside, this is a real gap for non-trivial dumps, though likely acceptable if this limitation is documented (see the docblock consolidation comment) rather than fixed outright, since the fallback already explicitly doesn't support server-level DDL.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/DB_Command.php` around lines 2656 - 2751, Update split_sql_statements() to recognize DELIMITER directives and use the configured delimiter instead of treating every semicolon inside stored routine or trigger definitions as a statement boundary. Preserve existing quote, comment, and conditional-comment handling, and ensure DELIMITER directives themselves are not included in returned SQL statements; if this scope is intentionally unsupported, document that limitation in the associated method docblock instead.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/DB_Command.php`:
- Around line 2519-2575: Update wpdb_query() to pass the input through the
existing split_sql_statements() helper and execute each resulting statement
individually, matching the iteration behavior used by wpdb_import(). Preserve
per-statement handling for modifying queries, errors, affected-row success
messages, result headers, and rows, while ensuring all statements from piped SQL
are processed.
- Around line 2519-2526: Update wpdb_query() to suppress wpdb errors during
query execution, mirroring the behavior in wpdb_import(). Capture the previous
suppression state, call $wpdb->suppress_errors(true) before executing the query,
and restore the captured state before every return path and when the method
completes.
- Around line 2633-2645: Update the transaction cleanup in the import flow
around the $wpdb query calls and its enclosing method so failures from COMMIT or
any settings-restore query are detected and propagated to the caller. Ensure
import() does not print its success message when cleanup fails, while preserving
normal success behavior when all queries complete successfully.
- Around line 2480-2493: Update the wpdb bootstrap required-files list near
$required_files to include WordPress's wp-includes/functions.php before loading
dependent files. Keep the existing require_once loop and file existence checks
unchanged so wpdb::print_error() and get_caller() can safely resolve
wp_debug_backtrace_summary().
---
Outside diff comments:
In `@src/DB_Command.php`:
- Around line 602-619: Update the query() docblock at src/DB_Command.php lines
602-619 to document that unavailable mysql/mariadb binaries trigger execution
through $wpdb, mysql-specific options including --dbuser and --dbpass are
ignored, and multi-statement input is unsupported. Update the import() docblock
at src/DB_Command.php lines 955-977 to document that the fallback ignores
--dbuser/--dbpass and does not support custom DELIMITER blocks or server-level
DDL.
---
Duplicate comments:
In `@src/DB_Command.php`:
- Around line 2501-2507: Update maybe_load_wpdb() to accept the connection
override arguments and use them when constructing the fallback wpdb instance
instead of always using DB_USER, DB_PASSWORD, DB_NAME, and DB_HOST directly.
Pass $assoc_args through from wpdb_query() and wpdb_import(), preserving the
constants as defaults when overrides are absent.
- Around line 2585-2606: Apply the SQL-mode compatibility statement to both
fallback executors before running SQL: in src/DB_Command.php lines 2585-2606,
update wpdb_import() to execute get_sql_mode_compat_statement( $assoc_args )
through $wpdb->query() before the split statements; likewise in
src/DB_Command.php lines 2519-2575, update wpdb_query() to execute the same
statement before $query.
---
Nitpick comments:
In `@src/DB_Command.php`:
- Around line 2656-2751: Update split_sql_statements() to recognize DELIMITER
directives and use the configured delimiter instead of treating every semicolon
inside stored routine or trigger definitions as a statement boundary. Preserve
existing quote, comment, and conditional-comment handling, and ensure DELIMITER
directives themselves are not included in returned SQL statements; if this scope
is intentionally unsupported, document that limitation in the associated method
docblock instead.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 5a2840be-db42-4924-83dc-02975a8da892
📒 Files selected for processing (3)
features/db-import.featurefeatures/db.featuresrc/DB_Command.php
💤 Files with no reviewable changes (1)
- features/db.feature
🚧 Files skipped from review as they are similar to previous changes (1)
- features/db-import.feature
|
Coming from #319, where I described our case: webserver image without the The overall shape looks right to me. Five things I would want fixed before this lands. CodeRabbit flagged most of them, I verified them against the current branch (
Two smaller notes:
And independent of the fallback: the commands that still need a binary ( @swissspidy happy to help move this along. I can test the branch against our setup, write the two missing Behat scenarios (zero-date dump through the fallback, multi-statement |
- Short-circuit is_mysql_binary_available() when binary path is empty. - Check binary availability before running commands without fallback (cli, export, check, optimize, repair) to avoid executing empty paths via /usr/bin/env. - Apply SQL mode compatibility statement before running queries and imports under wpdb fallback. - Pass custom dbuser, dbpass, dbname, and dbhost credentials to maybe_load_wpdb(). - Add support for multi-statement queries, -- space comment rules, # comments, and DELIMITER directives in split_sql_statements(). - Add error suppression to wpdb_query() and propagate COMMIT failures in wpdb_import(). - Update query and import docblocks to document wpdb fallback behavior.
…and fallback behavior
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@features/db-query.feature`:
- Around line 212-215: Update the “Database querying falls back to wpdb when
PATH contains no binaries” scenario so the wp command remains resolvable while
mysql and mariadb are hidden from PATH; use the runner’s absolute path or
construct PATH to retain wp, ensuring the test reaches the intended wpdb
fallback behavior.
In `@src/DB_Command.php`:
- Around line 2837-2846: Update the SQL parsing logic around the delimiter
handling in DB_Command to recognize DELIMITER directives when they occur at a
line boundary, before normal statement accumulation or semicolon processing.
Ensure newline-terminated directives update the active delimiter without
splitting procedure bodies, while preserving existing quoted-string handling and
statement collection. Add a procedure-based Behat acceptance scenario covering a
standard MySQL dump with a DELIMITER directive and verify the routine parses
correctly.
- Around line 289-296: Replace the unreachable empty-string guards with a shared
cached executable-validation helper that probes the selected binary using
--version and reports the existing client-tools error when unavailable. Apply
this validation before building the check command at src/DB_Command.php lines
289-296, before building the optimize command at lines 355-362, before building
the repair command at lines 421-428, and before probing options or exporting
with the dump binary at lines 798-803; reuse the helper across all four sites.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 994b2729-3667-4b32-b15c-20a6baeba4f2
📒 Files selected for processing (3)
features/db-import.featurefeatures/db-query.featuresrc/DB_Command.php
- Ensure is_mysql_binary_available() probes --version for non-empty binary paths so fake-bin test shims with non-zero exit codes correctly trigger wpdb fallback across all wp-cli dependency versions. - Recognize DELIMITER directives at line boundaries before normal statement accumulation in split_sql_statements(). - Add acceptance test for DELIMITER procedure imports under wpdb fallback.
Ensure wp-includes/functions.php is loaded during wpdb bootstrap so helper functions like wp_debug_backtrace_summary() are available if wpdb::print_error() or get_caller() are invoked.
…wpdb fallback details Document client binary requirements across create, drop, reset, check, optimize, repair, cli, and export subcommands, and clarify wpdb fallback behavior for query and import.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/DB_Command.php (1)
623-638: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winHonor or reject MySQL connection options consistently in fallback mode. The binary paths accept options such as
--host, while the wpdb fallback usesDB_HOSTunless undocumenteddbhostis supplied. Silent fallback can target the wrong database.
src/DB_Command.php#L623-L638: translate supported connection options before callingwpdb_query(), or error for unsupported options.src/DB_Command.php#L982-L1002: apply the same behavior before callingwpdb_import().As per coding guidelines, “Update relevant inline code documentation when changes affect user-facing functionality.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/DB_Command.php` around lines 623 - 638, Ensure MySQL connection options are handled consistently when binaries are unavailable: in the fallback query path around DB_Command’s wpdb_query call and the import path around wpdb_import, translate every supported option such as --host into the fallback connection configuration, or reject unsupported options with a clear WP_CLI error before execution. Update the relevant inline documentation to describe this user-facing fallback behavior; apply the change at both src/DB_Command.php ranges 623-638 and 982-1002.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/DB_Command.php`:
- Around line 2774-2778: Update the DELIMITER parsing logic around
$delimiter_line and $delimiter in the SQL processing loop to validate that the
extracted delimiter is non-empty before accepting it. Reject or report malformed
empty DELIMITER directives and ensure the loop cannot continue with a
zero-length delimiter that leaves $i unchanged.
---
Outside diff comments:
In `@src/DB_Command.php`:
- Around line 623-638: Ensure MySQL connection options are handled consistently
when binaries are unavailable: in the fallback query path around DB_Command’s
wpdb_query call and the import path around wpdb_import, translate every
supported option such as --host into the fallback connection configuration, or
reject unsupported options with a clear WP_CLI error before execution. Update
the relevant inline documentation to describe this user-facing fallback
behavior; apply the change at both src/DB_Command.php ranges 623-638 and
982-1002.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: d0d2d378-e606-40d7-be13-4f2109d55d36
📒 Files selected for processing (2)
features/db-import.featuresrc/DB_Command.php
🚧 Files skipped from review as they are similar to previous changes (1)
- features/db-import.feature
| $delimiter_line = trim( substr( $sql, $i, $line_end - $i ) ); | ||
| $delimiter = trim( substr( $delimiter_line, 10 ) ); | ||
| $current = ''; | ||
| $i = $line_end; | ||
| continue; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
Reject empty DELIMITER directives.
Line 2775 accepts an empty delimiter; the later zero-length match never advances $i, hanging wp db query or wp db import on malformed SQL.
Proposed fix
$delimiter_line = trim( substr( $sql, $i, $line_end - $i ) );
$delimiter = trim( substr( $delimiter_line, 10 ) );
+if ( '' === $delimiter ) {
+ WP_CLI::error( 'Invalid DELIMITER directive.' );
+}
$current = '';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| $delimiter_line = trim( substr( $sql, $i, $line_end - $i ) ); | |
| $delimiter = trim( substr( $delimiter_line, 10 ) ); | |
| $current = ''; | |
| $i = $line_end; | |
| continue; | |
| $delimiter_line = trim( substr( $sql, $i, $line_end - $i ) ); | |
| $delimiter = trim( substr( $delimiter_line, 10 ) ); | |
| if ( '' === $delimiter ) { | |
| WP_CLI::error( 'Invalid DELIMITER directive.' ); | |
| } | |
| $current = ''; | |
| $i = $line_end; | |
| continue; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/DB_Command.php` around lines 2774 - 2778, Update the DELIMITER parsing
logic around $delimiter_line and $delimiter in the SQL processing loop to
validate that the extracted delimiter is non-empty before accepting it. Reject
or report malformed empty DELIMITER directives and ensure the loop cannot
continue with a zero-length delimiter that leaves $i unchanged.
Ensure split_sql_statements() does not accept empty delimiter tokens on malformed DELIMITER lines, preventing zero-length delimiter matching and infinite loops.
…_load_wpdb() Ensure all standard MySQL connection flags (user, password/pass, database, host) as well as dbuser/dbpass/dbname/dbhost are mapped to the fallback wpdb instance.
wp db queryandwp db importcurrently require themysql/mariadbCLI binary, causing failures with drop-in database engines (HyperDB, custom drivers) or environments where the binary isn't installed. This PR adds automatic fallback to WordPress's$wpdbfor these commands when the binary is unavailable.Changes
src/DB_Command.phpis_mysql_binary_available()— probes for themysql/mariadbbinary viaProcess::create(); short-circuits tofalsewhenUtils\get_mysql_binary_path()returns an empty string without spawning a subprocess; probes--versionreturn code to reliably detect non-functional test shims across framework dependency versions.cli(),export(),check(),optimize(), andrepair()so missing binaries produce clean user-facing error messages instead of passing empty paths to shell commands (which caused/usr/bin/env: illegal option -- n).maybe_load_wpdb()— loads minimal WordPress files needed for$wpdb(load.php,compat.php,functions.php,plugin.php,class-wp-error.php), includes anywp-content/db.phpdrop-in (e.g. HyperDB), and creates a plainwpdbinstance with credentials (honouring--dbuser,--dbpass,--dbname,--dbhostoverrides).$this->get_sql_mode_compat_statement()to the$wpdbsession prior to query/import execution so strict zero-date modes (e.g.NO_ZERO_DATE,STRICT_TRANS_TABLES) are stripped for compatibility with WordPress schema defaults.query()&wpdb_query()— checks binary availability after SQLite handling; routes towpdb_query()when absent. Suppresses$wpdberrors during execution, splits multi-statement SQL strings viasplit_sql_statements(), formats SELECT results as tab-separated values, and reports affected rows for DML operations.import()&wpdb_import()— when the mysql binary is unavailable, reads SQL from file or STDIN and delegates towpdb_import(). Executes statements with transaction & constraint optimizations (autocommit=0,unique_checks=0,foreign_key_checks=0), checks forCOMMITexecution success, and ignores non-essential privilege errors onSETstatements.split_sql_statements()— state-machine SQL parser handling single/double-quoted strings, line comments (--with space requirement,#), block comments (/* */), MySQL conditional comments (/*!...*/), and line-boundaryDELIMITERdirectives.Tests & Documentation
features/db-query.featureandfeatures/db-import.featureasserting fallback to$wpdb, handling multi-statement queries, zero-date imports (0000-00-00 00:00:00), andDELIMITERprocedure routines.wp db queryandwp db importto document$wpdbfallback behavior.Summary by CodeRabbit
New Features
wp db queryandwp db importwhen MySQL/MariaDB client tools are unavailable.Bug Fixes