Query Builder Class - orderBy more possibilities

[amigoscs]

I want to execute a query using Query Builder Class:

SELECT * FROM paintings ORDER BY year IS NOT NULL, year ASC;

or

SELECT * FROM paintings ORDER BY year IS NULL, year ASC;

but Query Builder Class does not support this:

$builder->orderBy('year IS NULL, year ASC');

Please add this feature in future releases.

[michalsn]

Cases like this are already supported:

$builder->orderBy('year IS NULL', '', false)->orderBy('year', 'asc');

[amigoscs]

Cases like this are already supported:

$builder->orderBy('year IS NULL', '', false)->orderBy('year', 'asc');

Yes, but if $escape = true then an invalid request will be generated:

ORDER BY 'year IS' 'NULL', 'year' ASC

Behavior is ambiguous...

[kenjis]

@michalsn Thank you. Exactly!

        $builder = new BaseBuilder('paintings', $this->db);
        $builder->orderBy('year IS NULL', '', false)->orderBy('year', 'asc');

SELECT * FROM "paintings" ORDER BY year IS NULL, "year" ASC

[kenjis]

@amigoscs
See https://codeigniter4.github.io/CodeIgniter4/database/query_builder.html#CodeIgniter\Database\BaseBuilder::orderBy
Where is ambiguous?

[amigoscs]

@kenjis

1. it works
   $builder->orderBy('year IS NULL', '', false)->orderBy('year', 'asc');

2. this does not work
   $builder->orderBy('year IS NULL')->orderBy('year', 'asc');

3. this does not work
   $builder->orderBy('year', 'IS NULL')->orderBy('year', 'asc');

4. this does not work
   $builder->orderBy('year', 'IS NULL', false)->orderBy('year', 'asc');

'IS NULL' is expected to have the same behavior as 'ASC' or 'DESC', but is produced differently.

If fix this line in the system/Database/BaseBuilder.php file:

$qbOrderBy[] = ($direction === '' && preg_match('/\s+(ASC|DESC)$/i', rtrim($field), $match, PREG_OFFSET_CAPTURE))

and

$direction = in_array($direction, ['ASC', 'DESC'], true) ? ' ' . $direction : '';

on those

$qbOrderBy[] = ($direction === '' && preg_match('/\s+(IS NULL|IS NOT NULL|ASC|DESC)$/i', rtrim($field), $match, PREG_OFFSET_CAPTURE))

and

$direction = in_array($direction, ['IS NULL', 'IS NOT NULL', 'ASC', 'DESC'], true) ? ' ' . $direction : '';

then everything starts working.

[michalsn]

@amigoscs

I don't think allowing the IS NULL and IS NOT NULL as valid parameters would be a good decision. Every DB engine supports the case you're dealing with differently.

If you change your DB engine to SQLSRV, your query will fail.

[amigoscs]

@michalsn

Well, maybe then add the modified orderBy() method to system\Database\MySQLi\Builder.php

[kenjis]

This syntax is not supported by QueryBuilder, because all the supported DBMS do not support it.
IS NULL is not a direction for ORDER BY.
I think 2, 3 and 4 should not work.

[michalsn]

@amigoscs

In general, we don't accept features that would have support in only one DB engine. And allowing different valid $direction values for each engine doesn't make sense either.

What would make sense is building some kind of abstraction like we did for the RANDOM value, but personally, I don't like this idea at all. Although if you sent a PR with a version that would support all DB engines, we would certainly consider such changes.

For cases like yours, we have a third parameter in orderBy(), and this is a proper way to handle this for now.

[sclubricants]

'IS NULL' is expected to have the same behavior as 'ASC' or 'DESC'

This is wrong. The way it works and is documented is:

ORDER BY [EXPRESSION] [DIRECTION]

I would do it like:

ORDER BY IF(year IS NULL, 0, 1) ASC

This should be closed.