Adds projection alias support to ORDER BY clause

[johnedquinn]

Related Issues

• Closes Add support for using aliases created in select list in GROUP BY and ORDER BY #571

Description

• A PR that adds support for projection aliases in ORDER BY -- it does this by rewriting the query in accordance with PartiQL Spec Section 12.5
• Revisions:
  • 1: Initially created a PR that would essentially compile the projection list twice to evaluate the ORDER BY RHS expression, but after discussing with Yingtao and Robert, I decided to make revision Annotations can be dropped in some circumstances #2
  • 2: Make ORDER BY go AFTER projection. However -- this was wrong according to PartiQL Spec Sections 3.3 and 12.5.
    • A breakdown of the situation can be summarized by this PartiQL Specification discussion where Almann chimed in.
  • 3: I rewrite the query using a Visitor Transform and introduce a Meta to help maintain idempotency. Based on the PartiQL Spec sections mentioned above.

Good Examples

The following queries are supposed to work, and now they do.

SELECT b AS "C"
FROM (
    SELECT a AS b, (-1 * a) AS z
    FROM [ { 'a': 2, 'z': 4 }, { 'a': 1, 'z': 2 }, { 'a': -1, 'z': -2 } ]
    ORDER BY b
)
ORDER BY "C"
====
[
  {
    'C': -1
  },
  {
    'C': 1
  },
  {
    'C': 2
  }
]
--- 

SELECT (a * -1) AS b FROM << { 'a': 1 }, { 'a': 2 } >> ORDER BY b;
--- RESULT:
[
  {
    'b': -2
  },
  {
    'b': -1
  }
]

SELECT sum(a) AS c, (a * -1) AS b FROM << { 'a': 1 }, { 'a': 2 } >> GROUP BY a ORDER BY b LIMIT 1;
--- RESULT:
[
  {
    'c': 2,
    'b': -2
  }
]

And, here's a very interesting one that shows what it means to modify the environment's bindings.

SELECT (a * -1) AS a FROM << { 'a': 1 }, { 'a': 2 } >> ORDER BY -a;
-- RESULT:
[
  {
    'a': -1
  },
  {
    'a': -2
  }
]

-- We ran !add_to_global_env { 'c': -1 } before the following
SELECT (a * -1) AS a FROM << { 'a': 1 }, { 'a': 2 } >> ORDER BY a * c;
-- RESULT:
[
  {
    'a': -1
  },
  {
    'a': -2
  }
]

Bad Examples

The following queries should NOT work, and they do NOT.

SELECT b AS "C"
FROM (
    SELECT a AS b, (-1 * a) AS z
    FROM [ { 'a': 2, 'z': 4 }, { 'a': 1, 'z': 2 }, { 'a': -1, 'z': -2 } ]
    ORDER BY b
)
ORDER BY "c"

This failure shows that the a binding cannot be used in ORDER BY.

SELECT (a * -1) AS b FROM << { 'a': 1 }, { 'a': 2 } >> ORDER BY a;

More Information

• Updated CHANGELOG
• No backward incompatible changes
• No external dependencies

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[codecov-commenter]

Codecov Report

Base: 81.21% // Head: 81.53% // Increases project coverage by +0.31% 🎉

Coverage data is based on head (4f71b9a) compared to base (74b6cf6).
Patch coverage: 90.00% of modified lines in pull request are covered.

Additional details and impacted files

@@             Coverage Diff              @@
##               main     #740      +/-   ##
============================================
+ Coverage     81.21%   81.53%   +0.31%     
- Complexity     2615     2623       +8     
============================================
  Files           257      259       +2     
  Lines         21288    21715     +427     
  Branches       3861     4045     +184     
============================================
+ Hits          17290    17705     +415     
- Misses         2776     2779       +3     
- Partials       1222     1231       +9     

Flag	Coverage Δ
CLI	24.08% <ø> (ø)
EXAMPLES	77.19% <ø> (ø)
EXTENSIONS	66.66% <ø> (ø)
LANG	83.41% <90.00%> (+0.30%)	⬆️
PTS	∅ <ø> (∅)
TEST_SCRIPT	80.90% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...ng/src/org/partiql/lang/eval/EvaluatingCompiler.kt	80.27% <ø> (ø)
...g/eval/visitors/OrderBySortSpecVisitorTransform.kt	86.66% <86.66%> (ø)
.../partiql/lang/ast/IsTransformedOrderByAliasMeta.kt	100.00% <100.00%> (ø)
...rg/partiql/lang/eval/visitors/VisitorTransforms.kt	90.90% <100.00%> (+0.90%)	⬆️
...al/visitors/StaticTypeInferenceVisitorTransform.kt	88.55% <0.00%> (+4.63%)	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[yliuuuu]

The core logic here seems to be evaluating the project thunk and therefore creating a new environment for ORDER BY so that the alias is bind to a value. Perhaps it is possible to rewrite the query before the compilation? Just so there is no need to create a new environment for ORDER BY.
For example:
SELECT a + 1 as b FROM tbl ORDER BY b can be rewritten to SELECT a+1 as b FROM tbl ORDER BY a+1

[johnedquinn]

The core logic here seems to be evaluating the project thunk and therefore creating a new environment for ORDER BY so that the alias is bind to a value. Perhaps it is possible to rewrite the query before the compilation? Just so there is no need to create a new environment for ORDER BY. For example: SELECT a + 1 as b FROM tbl ORDER BY b can be rewritten to SELECT a+1 as b FROM tbl ORDER BY a+1

So, I was thinking about this, and it shouldn't work for specific scenarios. AKA, if we refer to the order of SQL clause evaluations (loosely):

1. FROM clause
2. WHERE clause
3. GROUP BY clause
4. HAVING clause
5. SELECT clause
6. ORDER BY clause

Then, we can assume that the aliases created during SELECT would be immediately usable as bindings for ORDER BY. So, consider the following query:

SELECT (a * -1) AS a FROM << { 'a': 1 }, { 'a': 2 } >> ORDER BY a;

From my interpretation, the above query should result in:

<<
  { a: -2 },
  { a: -1 }
>>

So, replacing the RHS of ORDER BY to be:

SELECT (a * -1) AS a FROM << { 'a': 1 }, { 'a': 2 } >> ORDER BY (a * -1);

SHOULD result in:

<<
  { a: -1 },
  { a: -2 }
>>

Because the a on the RHS of ORDER BY is now the new a from the projection (-1, -2), and the tuples will be ordered by their absolute values --> 1, 2.

But the current evaluating compiler has the evaluation order wrong, so your proposal DOES work with the existing compiler, but the compiler is wrong. So, regardless, I really like your comment, because it made me think about this in terms of evaluation logic and relational algebra, and I think I need to modify the order of the thunks rather than do either of our proposals.

FYI, MySQL takes the following query:

SELECT -id AS id1 FROM test ORDER BY -id1;
-- where test is a single-column table with values 1 and 2

and returns:

-1
-2

[yliuuuu]

I see what you mean here.
Based on the content you have written, I suppose that the last query really should be:

-- what your wrote
SELECT -id AS id1 FROM test ORDER BY -id1;
-- what your probably mean
SELECT -id AS id1 FROM test ORDER BY id1;

A similar analogy to consider is "Using SELECT alias in GROUP BY` in pure SQL context.
For example, consider:

SELECT a + 1 as group_key from test GROUP BY b

Here SQL faces the same problem, somehow it needs to know what is b even though GROUP BY get evaluated before SELECT.
Check this DB-Fiddle link to see the example query, notice how the two query plans generated by PostgreSQL are exactly the same.

Now back to the ORDER BY problem, I think the biggest confusion here is that:
Semantically, SQL's ORDER BY get evaluated after SELECT, but PARTIQL's ORDER BY get evaluated before SELECT. See Spec section 12.5 for more detail.

I suspect the spec defined it that way for semantic consistency, but I could be wrong.

If you main concern is the double evaluation (i.e., evaluate order by twice, once during projection and once during ordering), I suspect this problem can be addressed during the query planning.

Consider:

SELECT <expr> as a FROM tbl ORDER BY a
-- <expr> is some expression

I felt like the planer can essentially choose from

-- double evaluation
SELECT <expr> as a FROM tbl ORDER BY <expr>

or

-- double scan
SELECT a from (SELECT <expr> as a FROM tbl) ORDER BY a

Depending on which operation is cheaper.

check this DB-Fiddle Link for ORDER BY evaluation in PostgreSQL.

I could be totally wrong on this, but I am happy to brainstorming more on this topic with you. Hopefully it helps.

[johnedquinn]

Marking this as a DRAFT until I get some feedback on this PartiQL Spec Discussion

[johnedquinn]

After the feedback, I updated the PR to rewrite the query.

[yliuuuu]

Great work overall. Just two small nitpick comments.

[yliuuuu]

Would you please elaborate a little more on why do we need another class here?

[johnedquinn]

So, take the following query:

SELECT (a + b) AS b FROM << {'a': 0, 'b': 1}, {'a': 0, 'b': 1} >> ORDER BY b

Visitor transforms only allow specifying a single behavior per datatype. So, we use "support" classes to essentially deliver multiple types of behavior. AKA, in this scenario, we only want to replace the id in ORDER BY with whatever it is aliased to. Essentially, it helps with limiting the scope of rewriting. We might not necessarily want to rewrite ALL ids that have value b.

So, instead of replacing EVERY instance of id (b) with a lookup, we scope it to ORDER BY. That means the example from above gets rewritten to:

SELECT (a + b) AS b FROM << {'a': 0, 'b': 1}, {'a': 0, 'b': 1} >> ORDER BY (a + b)

and not:

SELECT (a + (a + b)) AS b FROM << {'a': 0, 'b': 1}, {'a': 0, 'b': 1} >> ORDER BY (a + b)

I'm not entirely sure if it would get rewritten exactly like this because off the top of my head IDK the order of evaluation of the visitor transforms, but you get the point. We don't want to rewrite every encounter of b -- we only want to when it is in the sortSpecs of Order By. See transformSortSpec_expr.

[yliuuuu]

LGTM.