feat: add support for excluding table data during export#298
feat: add support for excluding table data during export#298astappiev wants to merge 5 commits intowp-cli:mainfrom
Conversation
Introduces the `--exclude_tables_data` option to the `wp db export` command, allowing users to export only the structure of specified tables while excluding their data. This enhances flexibility in database management and export processes.
This comment was marked as resolved.
This comment was marked as resolved.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
This comment was marked as resolved.
This comment was marked as resolved.
There was a problem hiding this comment.
Code Review
The pull request successfully introduces the --exclude_tables_data option to the wp db export command, which is a valuable addition for users needing to export table structures without their data. The implementation follows the existing patterns in the codebase. I have suggested a few improvements to make the logic more robust, specifically regarding input sanitization (handling spaces in the comma-separated list) and providing a clearer error message when the option is used on unsupported database systems (MySQL), as noted in the PR description.
| $exclude_tables_data = Utils\get_flag_value( $assoc_args, 'exclude_tables_data' ); | ||
| if ( isset( $exclude_tables_data ) ) { | ||
| $tables = explode( ',', trim( $assoc_args['exclude_tables_data'], ',' ) ); | ||
| unset( $assoc_args['exclude_tables_data'] ); | ||
| foreach ( $tables as $table ) { | ||
| $command .= ' --ignore-table-data'; | ||
| $command .= ' %s'; | ||
| $command_esc_args[] = trim( DB_NAME . '.' . $table ); | ||
| } | ||
| } |
There was a problem hiding this comment.
The current implementation has a few robustness issues:
- Database Compatibility: As noted in the PR description, this flag is MariaDB-specific. Adding an explicit check for the database type allows us to provide a helpful WP-CLI error message instead of a cryptic shell error from
mysqldump. - Input Sanitization: The
trim()on line 745 is applied to the concatenatedDB_NAME . '.' . $tablestring. If the user provides a list liketable1, table2(with a space), the resulting argument becomesdbname. table2, which will causemysqldumpto fail. Thetrim()should be applied to the$tablevariable individually. - Type Safety: If the flag is passed without a value (e.g.,
--exclude_tables_data),get_flag_valuereturnstrue. The subsequenttrim()andexplode()calls on a boolean can lead to unexpected behavior (e.g., trying to ignore a table nameddbname.1).
$exclude_tables_data = Utils\get_flag_value( $assoc_args, 'exclude_tables_data' );
if ( isset( $exclude_tables_data ) ) {
unset( $assoc_args['exclude_tables_data'] );
if ( is_string( $exclude_tables_data ) && '' !== trim( $exclude_tables_data ) ) {
if ( 'mariadb' !== Utils\get_db_type() ) {
WP_CLI::error( 'The --exclude_tables_data option is only supported by MariaDB.' );
}
$tables = explode( ',', trim( $exclude_tables_data, ',' ) );
foreach ( $tables as $table ) {
$table = trim( $table );
if ( '' === $table ) {
continue;
}
$command .= ' --ignore-table-data';
$command .= ' %s';
$command_esc_args[] = DB_NAME . '.' . $table;
}
}
}
Introduces the
--exclude_tables_dataoption to thewp db exportcommand, allowing users to export only the structure of specified tables while excluding their data.Warning: only supported by MariaDB 10.1 and later. As far as I know, not supported by MySQL.
Resolves #289