JetBrains / Exposed

Kotlin SQL Framework
http://jetbrains.github.io/Exposed/
Apache License 2.0
8.27k stars 690 forks source link

feat: EXPOSED-367 SubQuery inside OrderBy clause #2129

Closed obabichevjb closed 3 months ago

obabichevjb commented 3 months ago

Here is the feature with support of query like expressions.

It allows covering sql queries like (from tests):

SELECT 
    OrderByQueryBox.id, 
    (
        SELECT SUM(OrderByQueryCoin.cost) 
        FROM OrderByQueryCoin 
        WHERE OrderByQueryBox.id = OrderByQueryCoin.box_id
    ) cost_sum 
    FROM OrderByQueryBox
    ORDER BY cost_sum ASC
SELECT OrderByQueryBox.id
    FROM OrderByQueryBox
    ORDER BY (
        SELECT SUM(OrderByQueryCoin.cost) 
            FROM OrderByQueryCoin WHERE OrderByQueryBox.id = OrderByQueryCoin.box_id
        ) DESC

Implementation is quite simple, a new kind of Expression was added: QueryExpression<T>(query: AbstractQuery, columnType: IColumnType<T>) : ExpressionWithColumnType<T> that appends the whole query into the query builder.

Since it's an expression, it can be used with ExpressionAlias, so it can be accessed via alias from ResultRow.

But about the public part I'm not completely sure with this PR.

The first moment is requiring columnType as a parameter. The reason is type safety and getting result from the query. Expression like query must have only one column returned, and the type of the whole expression is the type of that column actually, but I don't see a way to infer this type from random query, so user has to specify it manually what leads to the pattern:

val columnExpression = table.id.count()
val queryExpression = table.select(columnExpression).expression(columnExpression.columnType)

The second moment is adding new method expression(type) to the query(AbstractQuery). I feel like one more "expression" may confuse users even if they already used them via count(), sum() and many others.

Now it's possible to create query alias via query.alias() and query expression alias query.expression().alias(), and it would be different aliases. I had some thoughts that Query/AbstractQuery may also implement Expression and be used interchangeably with other expressions.. One of variants to do that is convert Expression into interface (or create bore basic interface IExpression) and use this expression across the code. But it looks like it will cause many changes, I started to do that but stopped at the current moment, if you also see that it makes sence I can clean it and push in different branch.

obabichevjb commented 3 months ago

So, finally it looks like the whole feature is the asExpression function (that utilizes wrapAsExpression function)... We can merge for some documentation and tests... or we can close it since it does not bring something actually valuable...

obabichevjb commented 3 months ago

Yep, it makes sense, let's close the PR, it does not bring something new to the project)