Skip to content

[CALCITE-5468] SqlToRelConverter throws if ORDER BY contains IN#3022

Merged
dssysolyatin merged 1 commit intoapache:mainfrom
dssysolyatin:CALCITE-5468
Feb 14, 2023
Merged

[CALCITE-5468] SqlToRelConverter throws if ORDER BY contains IN#3022
dssysolyatin merged 1 commit intoapache:mainfrom
dssysolyatin:CALCITE-5468

Conversation

@dssysolyatin
Copy link
Contributor

No description provided.

@dssysolyatin dssysolyatin force-pushed the CALCITE-5468 branch 3 times, most recently from cbf189d to 3333ddd Compare January 9, 2023 11:26
@dssysolyatin dssysolyatin changed the title CALCITE-5468: SqlToRelConverter should register sub-queries inside ORDER BY clause for queries without aggregation [CALCITE-5468]: SqlToRelConverter should register sub-queries inside ORDER BY clause for queries without aggregation Jan 9, 2023
@dssysolyatin dssysolyatin changed the title [CALCITE-5468]: SqlToRelConverter should register sub-queries inside ORDER BY clause for queries without aggregation [CALCITE-5468] SqlToRelConverter should register sub-queries inside ORDER BY clause for queries without aggregation Jan 9, 2023
@dssysolyatin dssysolyatin force-pushed the CALCITE-5468 branch 2 times, most recently from 3629b60 to 47ad8d1 Compare January 9, 2023 12:06
+ "ORDER BY\n"
+ "CASE WHEN empno IN (1,2) THEN 0 ELSE 1 END";
sql(sql).ok();
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't put stuff on the end. It causes merge conflicts. Put it in the most logical place.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This IN is not a subquery. Can you rename this test, and add a test for a subquery?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, IN is not subquery in terms of SQL. But inside codebase there are places with code like:

/**
 * Scalar expression that represents an IN, EXISTS or scalar sub-query.
 */
public class RexSubQuery extends RexCall

It is actually quite confusing.

I renamed test and added test for subquery.

@dssysolyatin dssysolyatin changed the title [CALCITE-5468] SqlToRelConverter should register sub-queries inside ORDER BY clause for queries without aggregation [CALCITE-5468] SqlToRelConverter throws if ORDER BY contains IN Jan 12, 2023
@dssysolyatin
Copy link
Contributor Author

@julianhyde Can I merge this PR ? Or there is any objections ?

@julianhyde
Copy link
Contributor

@dssysolyatin Looks good. Go ahead and merge to main.

@dssysolyatin dssysolyatin merged commit 79879f9 into apache:main Feb 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants