pounard / goat-query Goto Github PK

Exemple where the usage created problems: ->expression('v.saison = ? or v.saison is null', [$saison]) yielded different results from ->expression('(v.saison = ? or v.saison is null)', [$saison]).

Write an independent Symfony:

• version bundle following goat-query itself,
• easy installation without makinacorpus/goat which is a monolith containing way too much stuff,
• make makinacorpus/goat more resilient to makinacorpus/goat-query evolution and changes.

Cf. title.

At least for transactions, we need to be able to do transaction retries easily.

When dialect changes, write unit tests for all backends.

I built a query looking like this:

$count = $this
    ->getRunner()
    ->getQueryBuilder()
    ->select('...')
    // ...
    ->condition((new Where(Where::OR)) // <- Not accepted.
        ->isNull('...')
        ->isNull('...')
        ->isNull('...')
    )
    ->getCountQuery()
    ->execute()
    ->fetchField()
;

Giving a new Where object to the condition method results in a QueryError:

Goat\Query\QueryError: column reference must be a string or an instance of Goat\Query\ExpressionColumn

The problem comes from ExpressionFactory::column() called at line 119 of Where::condition(). It only accepts string or Expression as argument.

Documentation is still missing some bits, and it needs to be published.

Question is: where ?

It's very slow.

Normalize expression processing, allow them to be callables, everywhere, examples:

$select->with('top_sales', function (SelectQuery $select) {
    // $select is a new SELECT query to be used for the WITH statement
    // ... write your own $select query
});

Or:

$select->condition(function (Where $where) {
    // ... do something with $where
});

Or:

$select->getWhere()->exists(function (SelectQuery $query) {
    // $select is a new SELECT query to be used for the WITH statement
    // ... write your own $select query
});

Or more generally, all expressions to be:

$select->expression(function (...$contextualParameters) {
    // ... return some string or Expression instance
});

Cf. title.

It converts the first JOINed table with USING, but it keeps all others as JOINs which seem to not be working.

• write tests
• fix it
• if there's no way to fix it, remove feature and document that nested SELECT or WITH statements are much better for the job

Documentation (and tests):

• exemple of DELETE .. WHERE foo IN (SELECT ...)
• exemple of WHERE foo AS (SELECT ..) DELETE FROM bar USING foo WHERE ...

ext-pgsql driver would be much, much faster than PDO at the cost of not having PDO support anymore for Symfony.

For easier override in bundles, which then won't have to always fix their own each time a new method is added.

PHP lacks this, having a simple Date type.

In 2.x only, force result to be freed when iteration reaches the end. This means that fetchColumn() and maybe a few other methods will not be callable anymore after that.

• whenever possible, use iterators or generators,
• for query builder and data temporary storage, use objects with explicit type,
• in query formatter, it might not always be possible to get rid of temporary arrays.

Some method signatures requires that you provide a Query object, using a RawQuery which contains arbitrary SQL from the API user could do the trick.

Before today, this was not necessary because we had only SQL writer implementation for each RDMS vendor, but now, we have 2 for MySQL, thanks to MERGE implemention. So here is what to be implemented:

• one PHPUnit test runner per version,
• read RDMS version on connection init, or force it by configuration (parameter such as &version=57 for example),
• spawn services depending upon version.

This was ugly, but possible in 1.0, this must be restored.

People have to know every variation, using a factory it would be much simpler for users with a good IDE providing auto-completion to work with it.

Add a "tutorial" section in documentation. Divide in sub-sections for specific backends or RDBMS.

Start with documentation on how to create a volatile stored procedure (for session lifetime) then how to execute it (it took me a while to figure it out, even though the answer was simple).

If the user writes an INSERT such as:

$builder
    ->insertQuery('foo')
    ->columns(['a', 'b'])
    ->query(
        $builder
            ->select('bar')
            ->columns(['c', 'd'])
            /// ...
    )
    ->perform()
;

Allow it to be written as such:

$builder
    ->insertQuery('foo')
    ->query(
        $builder
            ->select('bar')
            ->columns(['a' => 'c', 'b' => 'd'])
            /// ...
    )
    ->perform()
;

Where SELECT column aliases become the INSERT column list.

Of course, this can only work if:

• every SELECT column as a name (i.e. no expression),
• every SELECT column as an alias (i.e. expressions are accepted).

Need to get rid of HydratorMap which has nothing to do here.

• write a custom hydrator interface,
• write generated-hydrator direct usage implementation,
• write generated-hydrator-bundle direct usage implementation,
• write a stupid runtime fallback,
• inject it into runners.

Most commonly used aggregate (or not) expressions are the following as of now:

• ->columnExpression('true') and ->columnExpression('false')
• ->columnExpression('1')
• ->columnExpression("count(some_column)") and ->columnExpression("count(*)")
• ->columnExpression(ExpressionRaw::create('array_agg(some_column)'))
• ->columnExpression(ExpressionRaw::create('coalesce(col1, col2, ...)'), 'alias')

It's not clear the difference between insertValues and insertQuery in QueryuBuilder class. They have the same description, maybe it could be nice to have an example in function annotations.

    /**
     * Create an insert query builder
     *
     * @param string|ExpressionRelation $relation
     *   SQL from statement relation name
     */
    public function insertValues($relation): InsertValuesQuery;

    /**
     * Create an insert with query builder
     *
     * @param string|ExpressionRelation $relation
     *   SQL from statement relation name
     */
    public function insertQuery($relation): InsertQueryQuery;

And improve overall performances whenever it's possible

Some ideas:

• formatter is slow, remove sprintf() and temporary arrays usage,
• ensure bag merge to be faster,
• less temporary variables and additional function calls whenever possible (but keep maximum code readiblity),
• if everything seems fine, do nothing.

It generates wrong SQL for PgSQL, and it will be almost free to port the MySQL feature as well. Forget about MySQL, wait for 2.x, as of now supported MySQL version status is ambigous and porting no so obvious, whereas 2.x removes the ambiguities.

Cf. VALUES in pgsql https://www.postgresql.org/docs/9.5/queries-values.html

• check for other DBMS support,
• allow it everywhere select is allowed.

• add a ->count($expression = null, $alias = null) as a shorcut to SELECT COUNT(EXPR) FROM, where $expression per default will be '*'

Simplest solution would be to allow escaping using ??.

Here are my propositions:

• rename condition() to where() to be closer to the SQL terms;
• rename expression() to whereExpression() to be consistent with columnExpression() and havingExpression() methods (I firstly thought expression() was the method to use to do what columnExpression() does).

As well, I wonder if it would not be better to rename the Where class to Condition for a more generic term completely dissociated from the place the condition will finally have in the query.

ExpressionConstantTable::values() will already accept anything, but:

• it does not propagates arguments to its own bag,
• formatter does not handle those.

This was not possible in 1.x due to functional tests removal for various reasons. But 2.x brought them back !

Both MERGE and INSERT queries have two variants: one for using VALUES (...) and the other to use a nested SELECT query or arbitrary expression.

This could be solved by materialising a constant table query implementation, which would be a table expression, and be used in place of any other expression.

This way we could systematically have both ->query(Query) and ->values() methods on those queries, values accepting either a callable which gives you a ExpressionConstantTable or such parameter, which yields a ->values() method, or uses an ExpressionConstantTable internally to populate using given array.

We won't have to handle two different implementations anymore, and it would solve #11.

• ensure that all dates that are given to the database are UTC per default

I knew it that it will end up being significant one day, hopefully only in some edge cases, yet it could be improved.

RIght now, nothing was planned for this, but we need to be able to execute stored procedure. It seems that using ext-pgsql, calling ->perform() or ->execute() doesn't work, so I'd go for adding an ->executeProcedure() method on the runner.

I'm not sure they are actually.

This method is historically the oldest piece of this whole component. It has been rewrote or improved many times, but today, we cannot really make it faster anymore as it exists.

In some benchmarks, this method execution time takes up to 1/3rd of query formatting, prepare, execute, which is huge.

Some leads we can follow:

• make this a component which can be replaced,
• write default implementation to be the current one,
• write an optional parser not using preg and benchmark, whereas this could be a nice improvement for PHP8 + JIT (this is a purely arbitrary assumption) it may be much much slower than preg,
• write an external optional implementation using c, rust, I don't care the language, based upon a parsing framework such as nom (rust), or any other, and plug it using FFI,
• any other ideas?

For use cases such as:

$select->expression($columnName . '::date BETWEEN ?::date AND ?::date', [$from, $to]);

We need to be able to properly escape the column name yet keep the pgsql cast. It could be written using a proper standard SQL cast as well:

$select->expression('CAST(' . $columnName . ' AS date) BETWEEN ...', [$from, $to]);

I'm not sure how to solve this:

A solution could be using a placeholder for column names and handle it in the ExpressionRaw class such as:

$select->expression(ExpressionRaw::create('CAST([column] AS date) BETWEEN ...', [$from, $to], ['column' => $columnName]);

But it think it would create yet another non explicit language within the language to learn.

May be a ExpressionColumnRaw object, that would take the column name as an argument and use a placeholder to replace it, like ExpressionLike expression ?

Cf. title

Actual ->perform() does not fetch any results and ask the server for none when possible, yet it remains synchronous.

Add an asynchronous variant of perform, which can name the query, that would return an object able to query for running query state.

This seems possible (and easy) with ext-pgsql, not sure it will be possible with PDO.

If the server refuses the connexion, we get a notice, but execution continue, and subsequent calls will attempt to use a bool (returned by pg_connect()) as a resource.

We should also force a default timeout if none provided.

Make it optional.

The incriminated line is this one:

I think it should be only (%s).

Follows up #38

• per default allow nested transactions
• default behaviour on savepoints if the backend allows it
• optionnally provide a method to start a transaction and fail if already within one

Implement schema introspector.