wp-cli / db-command

Performs basic database operations using credentials stored in wp-config.php.
MIT License
71 stars 57 forks source link

Should self::run('/usr/bin/env mysql...') be replaced with mysqli functions ? #245

Closed lewart3 closed 12 months ago

lewart3 commented 12 months ago

src/DB_Command.php has instances of the following form:

        $command = '/usr/bin/env mysql...';
        self::run( $command, ... );

Instead of spawning an external command, wouldn't it be more efficient to use the built-in WordPress $wpdb->get_results() ?

If so, mysql and mysqlcheck could be replaced, but not mysqldump.

Thank you! Dan

swissspidy commented 12 months ago

Many commands in this package need to run even if WordPress is not installed or loaded and thus $wpdb is not available.

danielbachhuber commented 12 months ago

Instead of spawning an external command, wouldn't it be more efficient to use the built-in WordPress $wpdb->get_results() ?

@lewart3 Out of curiosity, what prompted your suggestion? Did you run into some real-world use case to use $wpdb->get_results(), or were you simply looking at the code?

lewart3 commented 12 months ago

@swissspidy, since that is the case, would using mysqlnd functions, when possible, be any better than spawning mysql commands?

lewart3 commented 12 months ago

@danielbachhuber, while optimizing the Depends: for the Debian build, I wondered if mysql-client was really necessary, since PHP has included MySQL Native Driver since version 5.3.0.

So I dpkg -i installed php-wpcli_2.9.0_all.deb without the mysql-client dependency. All the commands I tried worked perfectly, except for the wp db subcommands, which failed with: Error: Failed to get current SQL modes. Reason: /usr/bin/env: ‘mysql’: No such file or directory

get_current_sql_modes() executes SELECT @@SESSION.sql_mode by spawning a mysql command. Perhaps it (and other similar commands) could be done better with mysqli functions?

danielbachhuber commented 12 months ago

@lewart3 Ah, thanks for clarifying. That's helpful context.

Perhaps it (and other similar commands) could be done better with mysqli functions?

I could've sworn this had been explored before, but I can't find a GitHub issue about it.

In either case, I think this is pretty unlikely. It would take a lot of effort to refactor everything to work with mysqli. I'm not even sure it's possible.

Thanks for the suggestion, though!

lewart3 commented 12 months ago

@danielbachhuber, thank you for the Review!

I did a quick benchmark to compare mysqli functions vs mysql commands using the script below. The mysql commands were twice as fast as the mysqli functions (20s vs 40s), much to my surprise.

#!/bin/sh
N=100
sql="SELECT option_name, option_value FROM wp_options
WHERE option_name NOT LIKE '\_transient\_%' AND option_name NOT LIKE '\_site\_transient\_%'"
date
for i in $(seq 1 $N); do
    printf "%d " "$i"
    wp option list >> mysqli-functions.log
done
printf "\n"
date
for i in $(seq 1 $N); do
    printf "%d " "$i"
    wp db query "$sql" >> mysql-commands.log
done
printf "\n"
date