--dry-run is not fully working

[orderbynull]

Running phinx migrate --dry-run will execute queries if they're written via queryBuilder. For example here's code from docs:

<?php
$builder = $this->getQueryBuilder();
$builder
->update('users')
->set('fname', 'Snow')
->where(['fname' => 'Jon'])
->execute()

So phinx migrate --dry-run will not work with that code.

[lorenzo]

No, it will not. I think they should be added to the docs. For many reasons it is not easy and some times not doable to implement this for raw queries.

Would you like to add this to the docs yourself?

[orderbynull]

@lorenzo Yep, i'll make PR

[lorenzo]

Thanks!

[ghost]

Would this mean that --dry-run would also work for seeds?

[falkenhawk]

Is it only that dry-run won't work now for queries created with QueryBuilder or maybe other ones as well? (e.g. create table, insert data, batch insert?)

[falkenhawk]

It looks like the new querybuilder part was implemented to use CakePHP\Database (and maybe other parts have been migrated to it as well?) but other parts are still using Phinx\Db. So currently two db adapters are used concurrently, whereas only Phinx\Db can handle dry-run and CakePHP\Database does not have this option implemented at all. @lorenzo I suppose the plan for Phinx is to migrate fully to CakePHP\Database but does it mean ditching dry-run option completely or is it planned to implement this option in CakePHP\Database at some point?

[lorenzo]

There are no plans to ditch --dry-run, everything but the queries executed by the QueryBuilder understand this flag. I'm hesitant of implementing this for QueryBuilder (at least for SELECT queries) since the results may be used later. It could be added to the other type of queries, though.

Maybe that is something you want to help with?

[falkenhawk]

Yes I'd love to. I am reading into CakePHP\Database classes to see where I might wire the dry-run option

[falkenhawk]

Okay, so I tried first with setting the 'log' => $this->isDryRunEnabled(), option in getDecoratedConnection() method, which would then wrap statements with LoggingStatement which should then try to use QueryLogger, and eventually Log::write('debug', $query, ['queriesLog']); (had to composer require cakephp/log to make it not throw a PHP Fatal error: Uncaught Error: Class 'Cake\Log\Log' not found).

But... nothing has been logged anywhere. Okay, so the logger needs writers to be configured. It'd need a console output writer to make it look kind of the same to the other queries when dry-run option is enabled...... but it seems to be configured globally as the logger is accessed statically.... ??? I don't like that so I guess using the sorts-of "built-in" database logger is eventually not an option.

[lorenzo]

We can write our own DatabaseLogger, no need to use the one from cakephp, the logger could get the output object from the symphony console component and simply output the stuff. Does that sound right to you?

[vlad-datacat]

+1 For this. Query Builder has to be able to output raw SQL that interprets and sends to server. We've had a major problem where something got interpreted in an unintended way and entire database of suppliers got updated instead of a few intended records. Showing what was the interpreted SQL to be sent to server would have helped at code review stage. The issue we had was someone writing ->where(['id >', 1]) instead of ->where(['id >' => 1]) - that quietly passed for WHERE id SQL dropping the second bit, no error, and entire table updated. We need to see whats been run against the server.

[lorenzo]

@borisnetweb Do you want to see what queries are being executed or do you want to dump the SQL without executing it? The problem with not executing queries in the query builder is that most likely you are using the query builder to get results, and you will get none or an error. That does not seem very helpful.

I agree that being able to log the queries done by the query builder is important. Does that not work with the verbose output? (cannot remember if it does right now)

[vlad-datacat]

@lorenzo ideally both - view sql and execute it. we run as part of bitbucket pipelines so this way we will see and have a record of what happened. at this stage verbose does nothing for query builder.

[falkenhawk]

it should not execute anything on dry-run (by default), but if possible the execution could be configurable e.g. turn on for select queries... 🙈

[lorenzo]

That's another good suggestion, to only make destructive queries part of the dry-run, and actually run the selects.

[lorenzo]

Is anyone interested in implementing the feature? I can guide anyone who's up for it. I currently don't have the time to do it myself.

[vlad-datacat]

@lorenzo I am keen to contribute for QueryBuilder part. I have plenty of Laravel & other php experience, none with cakephp whatsoever - should be a perfect fit :) Looks like we are in very different timezones - I am in Brisbane Australia.

[bubach]

Kludge-solutions Inc.

    /**
     * Execute queries build using the CakePHP QueryBuilder but respect Phinx --dry-run option
     *
     * @param  Query $queryBuilder
     */
    public function executeFromQueryBuilder(Query $queryBuilder)
    {
        $sql = $queryBuilder->sql();
        $bindings = $queryBuilder->getValueBinder()->bindings();

        if ($this->getInput()->getOption('dry-run')) {
            if (!empty($bindings)) {
                $values = array_column($bindings, 'value');

                foreach ($values as &$value) {
                    $value = is_string($value) ? "'" . $value . "'" : $value;
                }

                $sql = str_replace(array_keys($bindings), $values, $sql);
            }

            $this->getAdapter()->getOutput()->writeln($sql);
        } else {
            $queryBuilder->execute();
        }
    }

[dereuromark]

Someone wants to make a PR?