You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by "mihaibudiu (via GitHub)" <gi...@apache.org> on 2025/04/04 23:54:51 UTC
[PR] [CALCITE-6939] Add support for Lateral Column Alias [calcite]
mihaibudiu opened a new pull request, #4289:
URL: https://github.com/apache/calcite/pull/4289
This is based on an implementation from https://github.com/apache/calcite/pull/2931 by @jaystarshot from 2022.
However, this implementation does only one of the many things from the original. Moreover, it uses a conformance feature flag to gate the effects of this change.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6939] Add support for Lateral Column Alias [calcite]
Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on code in PR #4289:
URL: https://github.com/apache/calcite/pull/4289#discussion_r2033554279
##########
core/src/main/java/org/apache/calcite/sql/validate/SqlConformance.java:
##########
@@ -102,6 +102,16 @@ public interface SqlConformance {
*/
boolean isGroupByAlias();
+ /**
+ * Whether to allow aliases from the {@code SELECT} clause to be used as
+ * column names in the same {@code SELECT} clause.
+ * E.g., SELECT 1 as X, X+1 as Y;
+ * Name lookup considers an identifier in the same SELECT only
+ * if other lookups failed.
+ * Supported by Spark, Snowflake, BigQuery.
Review Comment:
Our users are porting Spark programs that use this feature.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6939] Add support for Lateral Column Alias [calcite]
Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on code in PR #4289:
URL: https://github.com/apache/calcite/pull/4289#discussion_r2036120505
##########
core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java:
##########
@@ -6677,6 +6677,32 @@ public boolean isBangEqualAllowed() {
.withConformance(lenient).fails("Column 'E' not found in any table");
}
+ /** Test case for <a href="https://issues.apache.org/jira/browse/CALCITE-6939">
+ * [CALCITE-6939] Add support for Lateral Column Alias</a>. */
+ @Test void testAliasInSelect() {
Review Comment:
I have added some tests to show that it does now
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6939] Add support for Lateral Column Alias [calcite]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #4289:
URL: https://github.com/apache/calcite/pull/4289#issuecomment-2803434611
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4289) **Quality Gate passed**
Issues
 [11 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4289&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4289&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=4289&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [95.2% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4289&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4289&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4289)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6939] Add support for Lateral Column Alias [calcite]
Posted by "julianhyde (via GitHub)" <gi...@apache.org>.
julianhyde commented on code in PR #4289:
URL: https://github.com/apache/calcite/pull/4289#discussion_r2039910905
##########
core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java:
##########
@@ -399,10 +400,11 @@ public SqlConformance getConformance() {
SqlSelect select, boolean includeSystemVars) {
final List<SqlNode> list = new ArrayList<>();
final PairList<String, RelDataType> types = PairList.of();
+ final Map<String, SqlNode> expansions = new HashMap<>();
for (final SqlNode selectItem : selectList) {
final RelDataType originalType = getValidatedNodeTypeIfKnown(selectItem);
expandSelectItem(selectItem, select, first(originalType, unknownType),
- list, catalogReader.nameMatcher().createSet(), types,
+ list, catalogReader.nameMatcher().createSet(), types, expansions,
Review Comment:
I can't immediately think of a better way of doing this, but we discovered a long time ago that mutation during validation was a bad idea.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6939] Add support for Lateral Column Alias [calcite]
Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on code in PR #4289:
URL: https://github.com/apache/calcite/pull/4289#discussion_r2036120904
##########
core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java:
##########
@@ -6677,6 +6677,32 @@ public boolean isBangEqualAllowed() {
.withConformance(lenient).fails("Column 'E' not found in any table");
}
+ /** Test case for <a href="https://issues.apache.org/jira/browse/CALCITE-6939">
+ * [CALCITE-6939] Add support for Lateral Column Alias</a>. */
+ @Test void testAliasInSelect() {
+ final SqlConformance withSelectAlias = new SqlAbstractConformance() {
+ @Override public boolean isSelectAlias() {
+ return true;
+ }
+ };
+
+ // Standard conformance
+ sql("select 1 AS x, ^x^ as Y")
Review Comment:
I have added some SqlOperatorTests to show that this works end-to-end.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6939] Add support for Lateral Column Alias [calcite]
Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on code in PR #4289:
URL: https://github.com/apache/calcite/pull/4289#discussion_r2033564889
##########
core/src/main/java/org/apache/calcite/sql/validate/SqlConformance.java:
##########
@@ -102,6 +102,16 @@ public interface SqlConformance {
*/
boolean isGroupByAlias();
+ /**
+ * Whether to allow aliases from the {@code SELECT} clause to be used as
+ * column names in the same {@code SELECT} clause.
+ * E.g., SELECT 1 as X, X+1 as Y;
+ * Name lookup considers an identifier in the same SELECT only
+ * if other lookups failed.
+ * Supported by Spark, Snowflake, BigQuery.
Review Comment:
I see. I have Dataricks' Spark here. I can help you verify it tomorrow.
##########
core/src/main/java/org/apache/calcite/sql/validate/SqlConformance.java:
##########
@@ -102,6 +102,16 @@ public interface SqlConformance {
*/
boolean isGroupByAlias();
+ /**
+ * Whether to allow aliases from the {@code SELECT} clause to be used as
+ * column names in the same {@code SELECT} clause.
+ * E.g., SELECT 1 as X, X+1 as Y;
+ * Name lookup considers an identifier in the same SELECT only
+ * if other lookups failed.
+ * Supported by Spark, Snowflake, BigQuery.
Review Comment:
I see. I have Dataricks' Spark here. I can help you verify it tomorrow.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6939] Add support for Lateral Column Alias [calcite]
Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on code in PR #4289:
URL: https://github.com/apache/calcite/pull/4289#discussion_r2030184519
##########
core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java:
##########
@@ -7393,20 +7393,65 @@ private SqlNode nthSelectItem(int ordinal, final SqlParserPos pos) {
*/
static class SelectExpander extends Expander {
final SqlSelect select;
+ @Nullable SqlNode root = null;
SelectExpander(SqlValidatorImpl validator, SelectScope scope,
SqlSelect select) {
super(validator, scope);
this.select = select;
}
+ @Override public SqlNode go(SqlNode root) {
+ this.root = root;
+ return super.go(root);
+ }
+
@Override public @Nullable SqlNode visit(SqlIdentifier id) {
final SqlNode node =
expandCommonColumn(select, id, (SelectScope) getScope(), validator);
if (node != id) {
return node;
} else {
- return super.visit(id);
+ try {
+ return super.visit(id);
+ } catch (Exception ex) {
+ if (!(ex instanceof CalciteContextException)) {
+ throw ex;
+ }
+ if (validator.getConformance().isSelectAlias() && id.isSimple()) {
+ // Try to lookup the item in the select list itself
+ String name = id.getSimple();
+ @Nullable SqlNode expr = null;
+ final SqlNameMatcher nameMatcher =
+ validator.catalogReader.nameMatcher();
+ int n = 0;
Review Comment:
I feel that the variable name `n` is a bit vague. Is there a more appropriate name?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6939] Add support for Lateral Column Alias [calcite]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #4289:
URL: https://github.com/apache/calcite/pull/4289#issuecomment-2779946843
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4289) **Quality Gate passed**
Issues
 [9 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4289&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4289&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=4289&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [83.3% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4289&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4289&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4289)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6939] Add support for Lateral Column Alias [calcite]
Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on code in PR #4289:
URL: https://github.com/apache/calcite/pull/4289#discussion_r2030183849
##########
core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java:
##########
@@ -6677,6 +6677,32 @@ public boolean isBangEqualAllowed() {
.withConformance(lenient).fails("Column 'E' not found in any table");
}
+ /** Test case for <a href="https://issues.apache.org/jira/browse/CALCITE-6939">
+ * [CALCITE-6939] Add support for Lateral Column Alias</a>. */
+ @Test void testAliasInSelect() {
Review Comment:
Does it support multi-layer nesting?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6939] Add support for Lateral Column Alias [calcite]
Posted by "julianhyde (via GitHub)" <gi...@apache.org>.
julianhyde commented on code in PR #4289:
URL: https://github.com/apache/calcite/pull/4289#discussion_r2039907728
##########
core/src/main/java/org/apache/calcite/sql/validate/SqlConformance.java:
##########
@@ -102,6 +102,28 @@ public interface SqlConformance {
*/
boolean isGroupByAlias();
+ /**
+ * Value describing how to perform lookup for values defined in a SELECT statement.
Review Comment:
'lookup for values' - these are not values, these are aliases. It's not a SELECT statement, it's the SELECT list.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6939] Add support for Lateral Column Alias [calcite]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #4289:
URL: https://github.com/apache/calcite/pull/4289#issuecomment-2828393867
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4289) **Quality Gate passed**
Issues
 [11 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4289&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4289&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=4289&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [95.2% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4289&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4289&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4289)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6939] Add support for Lateral Column Alias [calcite]
Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on code in PR #4289:
URL: https://github.com/apache/calcite/pull/4289#discussion_r2041092030
##########
core/src/main/java/org/apache/calcite/sql/validate/SqlConformance.java:
##########
@@ -102,6 +102,16 @@ public interface SqlConformance {
*/
boolean isGroupByAlias();
+ /**
+ * Whether to allow aliases from the {@code SELECT} clause to be used as
+ * column names in the same {@code SELECT} clause.
+ * E.g., SELECT 1 as X, X+1 as Y;
+ * Name lookup considers an identifier in the same SELECT only
+ * if other lookups failed.
+ * Supported by Spark, Snowflake, BigQuery.
Review Comment:
dataricks test success
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6939] Add support for Lateral Column Alias [calcite]
Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on PR #4289:
URL: https://github.com/apache/calcite/pull/4289#issuecomment-2797402372
If you have comments on this topic please weigh in either here or in the jira.
Otherwise I plan to merge this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6939] Add support for Lateral Column Alias [calcite]
Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu merged PR #4289:
URL: https://github.com/apache/calcite/pull/4289
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6939] Add support for Lateral Column Alias [calcite]
Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on code in PR #4289:
URL: https://github.com/apache/calcite/pull/4289#discussion_r2033499434
##########
core/src/main/java/org/apache/calcite/sql/validate/SqlConformance.java:
##########
@@ -102,6 +102,16 @@ public interface SqlConformance {
*/
boolean isGroupByAlias();
+ /**
+ * Whether to allow aliases from the {@code SELECT} clause to be used as
+ * column names in the same {@code SELECT} clause.
+ * E.g., SELECT 1 as X, X+1 as Y;
+ * Name lookup considers an identifier in the same SELECT only
+ * if other lookups failed.
+ * Supported by Spark, Snowflake, BigQuery.
Review Comment:
https://www.databricks.com/blog/introducing-support-lateral-column-alias
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6939] Add support for Lateral Column Alias [calcite]
Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on code in PR #4289:
URL: https://github.com/apache/calcite/pull/4289#discussion_r2033568451
##########
core/src/main/java/org/apache/calcite/sql/validate/SqlConformance.java:
##########
@@ -102,6 +102,16 @@ public interface SqlConformance {
*/
boolean isGroupByAlias();
+ /**
+ * Whether to allow aliases from the {@code SELECT} clause to be used as
+ * column names in the same {@code SELECT} clause.
+ * E.g., SELECT 1 as X, X+1 as Y;
+ * Name lookup considers an identifier in the same SELECT only
+ * if other lookups failed.
+ * Supported by Spark, Snowflake, BigQuery.
Review Comment:
I see. @NobiGo Which version of Spark did you verify this on? Can you tell me? I can verify this on DataRisks later.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6939] Add support for Lateral Column Alias [calcite]
Posted by "NobiGo (via GitHub)" <gi...@apache.org>.
NobiGo commented on code in PR #4289:
URL: https://github.com/apache/calcite/pull/4289#discussion_r2038526107
##########
core/src/main/java/org/apache/calcite/sql/validate/SqlConformance.java:
##########
@@ -102,6 +102,28 @@ public interface SqlConformance {
*/
boolean isGroupByAlias();
+ /**
+ * Value describing how to perform lookup for values defined in a SELECT statement.
+ */
+ enum SelectAliasLookup {
+ /** Values defined in a SELECT statement are not visible within the statement. */
+ UNSUPPORTED,
+ /** Values defined in a SELECT statement are visible to the right of their definition. */
+ LEFT_TO_RIGHT,
Review Comment:
```
What if the table t already has a column called “a” ?
Should “b” be 42 + 1, or should it be the column “a” + 1 ?
```
In Spark:
```
select 2 as x, x + 2 from table;
```
When the table has column x. and value 1. The SQL will return `2 3`.
I'm not sure if any other databases return `2 4` means the priority of column aliases is higher than the definition of the table.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6939] Add support for Lateral Column Alias [calcite]
Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on PR #4289:
URL: https://github.com/apache/calcite/pull/4289#issuecomment-2845422092
Since this feature is gated by a conformance flag, I think it cannot disrupt any standard SQL programs, so it's relatively "safe".
One thing that is missing is documentation about the new name lookup rules; I could not find any name lookup rules in the Calcite documentation, so I don't know where to add such documentation (besides the JavaDoc, which has them). So I will merge this.
If there are problems with the implementation, we will fix them when we find them.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6939] Add support for Lateral Column Alias [calcite]
Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on code in PR #4289:
URL: https://github.com/apache/calcite/pull/4289#discussion_r2033521405
##########
core/src/main/java/org/apache/calcite/sql/validate/SqlConformance.java:
##########
@@ -102,6 +102,16 @@ public interface SqlConformance {
*/
boolean isGroupByAlias();
+ /**
+ * Whether to allow aliases from the {@code SELECT} clause to be used as
+ * column names in the same {@code SELECT} clause.
+ * E.g., SELECT 1 as X, X+1 as Y;
+ * Name lookup considers an identifier in the same SELECT only
+ * if other lookups failed.
+ * Supported by Spark, Snowflake, BigQuery.
Review Comment:
I remember that the documentation of Dataricks and Spark is sometimes different. I am not sure if this is also the case.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6939] Add support for Lateral Column Alias [calcite]
Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on code in PR #4289:
URL: https://github.com/apache/calcite/pull/4289#discussion_r2033500528
##########
core/src/main/java/org/apache/calcite/sql/validate/SqlConformance.java:
##########
@@ -102,6 +102,16 @@ public interface SqlConformance {
*/
boolean isGroupByAlias();
+ /**
+ * Whether to allow aliases from the {@code SELECT} clause to be used as
+ * column names in the same {@code SELECT} clause.
+ * E.g., SELECT 1 as X, X+1 as Y;
+ * Name lookup considers an identifier in the same SELECT only
+ * if other lookups failed.
+ * Supported by Spark, Snowflake, BigQuery.
Review Comment:
May be wrong about bigquery, though. See the issue on JIRA.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6939] Add support for Lateral Column Alias [calcite]
Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on code in PR #4289:
URL: https://github.com/apache/calcite/pull/4289#discussion_r2033568451
##########
core/src/main/java/org/apache/calcite/sql/validate/SqlConformance.java:
##########
@@ -102,6 +102,16 @@ public interface SqlConformance {
*/
boolean isGroupByAlias();
+ /**
+ * Whether to allow aliases from the {@code SELECT} clause to be used as
+ * column names in the same {@code SELECT} clause.
+ * E.g., SELECT 1 as X, X+1 as Y;
+ * Name lookup considers an identifier in the same SELECT only
+ * if other lookups failed.
+ * Supported by Spark, Snowflake, BigQuery.
Review Comment:
I see. @NobiGo Which version of Spark did you verify this on? Can you tell me?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6939] Add support for Lateral Column Alias [calcite]
Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on code in PR #4289:
URL: https://github.com/apache/calcite/pull/4289#discussion_r2033551348
##########
core/src/main/java/org/apache/calcite/sql/validate/SqlConformance.java:
##########
@@ -102,6 +102,16 @@ public interface SqlConformance {
*/
boolean isGroupByAlias();
+ /**
+ * Whether to allow aliases from the {@code SELECT} clause to be used as
+ * column names in the same {@code SELECT} clause.
+ * E.g., SELECT 1 as X, X+1 as Y;
+ * Name lookup considers an identifier in the same SELECT only
+ * if other lookups failed.
+ * Supported by Spark, Snowflake, BigQuery.
Review Comment:
The SQL engine inside Databricks has been rewritten in C++, which is different from the open source Spark. Maybe there are some changes in the rewriting process, which are difficult for us to verify. Maybe we should focus on the open source Spark version.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6939] Add support for Lateral Column Alias [calcite]
Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on code in PR #4289:
URL: https://github.com/apache/calcite/pull/4289#discussion_r2038528682
##########
core/src/main/java/org/apache/calcite/sql/validate/SqlConformance.java:
##########
@@ -102,6 +102,28 @@ public interface SqlConformance {
*/
boolean isGroupByAlias();
+ /**
+ * Value describing how to perform lookup for values defined in a SELECT statement.
+ */
+ enum SelectAliasLookup {
+ /** Values defined in a SELECT statement are not visible within the statement. */
+ UNSUPPORTED,
+ /** Values defined in a SELECT statement are visible to the right of their definition. */
+ LEFT_TO_RIGHT,
Review Comment:
If 'x' exists in the FROM, it is used from the FROM.
Only when lookup in the FROM fails it is made in the local SELECT.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6939] Add support for Lateral Column Alias [calcite]
Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on code in PR #4289:
URL: https://github.com/apache/calcite/pull/4289#discussion_r2033524354
##########
core/src/main/java/org/apache/calcite/sql/validate/SqlConformance.java:
##########
@@ -102,6 +102,16 @@ public interface SqlConformance {
*/
boolean isGroupByAlias();
+ /**
+ * Whether to allow aliases from the {@code SELECT} clause to be used as
+ * column names in the same {@code SELECT} clause.
+ * E.g., SELECT 1 as X, X+1 as Y;
+ * Name lookup considers an identifier in the same SELECT only
+ * if other lookups failed.
+ * Supported by Spark, Snowflake, BigQuery.
Review Comment:
What other SQL do they support besides Spark SQL?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6939] Add support for Lateral Column Alias [calcite]
Posted by "NobiGo (via GitHub)" <gi...@apache.org>.
NobiGo commented on code in PR #4289:
URL: https://github.com/apache/calcite/pull/4289#discussion_r2038494246
##########
core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java:
##########
@@ -6677,6 +6677,32 @@ public boolean isBangEqualAllowed() {
.withConformance(lenient).fails("Column 'E' not found in any table");
}
+ /** Test case for <a href="https://issues.apache.org/jira/browse/CALCITE-6939">
+ * [CALCITE-6939] Add support for Lateral Column Alias</a>. */
+ @Test void testAliasInSelect() {
+ final SqlConformance withSelectAlias = new SqlAbstractConformance() {
+ @Override public boolean isSelectAlias() {
+ return true;
+ }
+ };
+
+ // Standard conformance
+ sql("select 1 AS x, ^x^ as Y")
Review Comment:
I tested it again using 3.5.1 and it is supported. Previously, I used 3.1.2.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6939] Add support for Lateral Column Alias [calcite]
Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on code in PR #4289:
URL: https://github.com/apache/calcite/pull/4289#discussion_r2038528166
##########
core/src/main/java/org/apache/calcite/sql/validate/SqlConformance.java:
##########
@@ -102,6 +102,28 @@ public interface SqlConformance {
*/
boolean isGroupByAlias();
+ /**
+ * Value describing how to perform lookup for values defined in a SELECT statement.
+ */
+ enum SelectAliasLookup {
+ /** Values defined in a SELECT statement are not visible within the statement. */
+ UNSUPPORTED,
+ /** Values defined in a SELECT statement are visible to the right of their definition. */
+ LEFT_TO_RIGHT,
Review Comment:
As I have commented in the JIRA, if you would do that, you would break all existing queries that redefine columns that exist already. So I don't think any database can afford to do that.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6939] Add support for Lateral Column Alias [calcite]
Posted by "julianhyde (via GitHub)" <gi...@apache.org>.
julianhyde commented on code in PR #4289:
URL: https://github.com/apache/calcite/pull/4289#discussion_r2039911772
##########
core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java:
##########
@@ -7389,15 +7396,50 @@ private SqlNode nthSelectItem(int ordinal, final SqlParserPos pos) {
/**
* Converts an expression into canonical form by fully-qualifying any
* identifiers. For common columns in USING, it will be converted to
- * COALESCE(A.col, B.col) AS col.
+ * COALESCE(A.col, B.col) AS col. When using a conformance that
+ * allows isSelectAlias, it expands identifiers that refer to
+ * local select expressions into the expressions themselves.
*/
static class SelectExpander extends Expander {
final SqlSelect select;
-
- SelectExpander(SqlValidatorImpl validator, SelectScope scope,
- SqlSelect select) {
+ // Maps simple identifiers to their expansions.
+ final Map<String, SqlNode> expansions;
+ // List of identifiers that are currently beeing looked-up. Used to detect
Review Comment:
s/beeing/being/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6939] Add support for Lateral Column Alias [calcite]
Posted by "julianhyde (via GitHub)" <gi...@apache.org>.
julianhyde commented on code in PR #4289:
URL: https://github.com/apache/calcite/pull/4289#discussion_r2039908297
##########
core/src/main/java/org/apache/calcite/sql/validate/SqlConformance.java:
##########
@@ -102,6 +102,28 @@ public interface SqlConformance {
*/
boolean isGroupByAlias();
+ /**
+ * Value describing how to perform lookup for values defined in a SELECT statement.
+ */
+ enum SelectAliasLookup {
+ /** Values defined in a SELECT statement are not visible within the statement. */
+ UNSUPPORTED,
+ /** Values defined in a SELECT statement are visible to the right of their definition. */
Review Comment:
Mention 'lateral'
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6939] Add support for Lateral Column Alias [calcite]
Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on PR #4289:
URL: https://github.com/apache/calcite/pull/4289#issuecomment-2797473745
I will check to see how quidem tests can change conformance
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6939] Add support for Lateral Column Alias [calcite]
Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on PR #4289:
URL: https://github.com/apache/calcite/pull/4289#issuecomment-2798044925
> I am especially worried about matching of GROUP BY columns. I can "SELECT x + y" if I have "GROUP BY x + y" but what if "y" has a different meaning in the two expressions?
Some of this is already a consideration when using isGroupbyAlias.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6939] Add support for Lateral Column Alias [calcite]
Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on PR #4289:
URL: https://github.com/apache/calcite/pull/4289#issuecomment-2798252839
There is a second commit which attempted to address all the issues raised by @julianhyde
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6939] Add support for Lateral Column Alias [calcite]
Posted by "NobiGo (via GitHub)" <gi...@apache.org>.
NobiGo commented on code in PR #4289:
URL: https://github.com/apache/calcite/pull/4289#discussion_r2038508157
##########
core/src/main/java/org/apache/calcite/sql/validate/SqlConformance.java:
##########
@@ -102,6 +102,16 @@ public interface SqlConformance {
*/
boolean isGroupByAlias();
+ /**
+ * Whether to allow aliases from the {@code SELECT} clause to be used as
+ * column names in the same {@code SELECT} clause.
+ * E.g., SELECT 1 as X, X+1 as Y;
+ * Name lookup considers an identifier in the same SELECT only
+ * if other lookups failed.
+ * Supported by Spark, Snowflake, BigQuery.
Review Comment:
I retested with 3.5.1 and it is supported.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6939] Add support for Lateral Column Alias [calcite]
Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on PR #4289:
URL: https://github.com/apache/calcite/pull/4289#issuecomment-2798252620
I hope I have made it work with GROUP BY and HAVING. Indeed, there were some issues.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6939] Add support for Lateral Column Alias [calcite]
Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on PR #4289:
URL: https://github.com/apache/calcite/pull/4289#issuecomment-2779918304
To save you the trouble from reading the JIRA issue, this adds a conformance flag to allow such programs become legal:
`SELECT 1 as x, x+1 as y`, where `x` can be used immediately once it has been defined.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6939] Add support for Lateral Column Alias [calcite]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #4289:
URL: https://github.com/apache/calcite/pull/4289#issuecomment-2780120430
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4289) **Quality Gate passed**
Issues
 [9 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4289&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4289&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=4289&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [82.1% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4289&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4289&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4289)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6939] Add support for Lateral Column Alias [calcite]
Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on PR #4289:
URL: https://github.com/apache/calcite/pull/4289#issuecomment-2810338433
We are testing this in our production environment, and if it works fine and there are no other comments I plan to merge it in a few days.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6939] Add support for Lateral Column Alias [calcite]
Posted by "julianhyde (via GitHub)" <gi...@apache.org>.
julianhyde commented on code in PR #4289:
URL: https://github.com/apache/calcite/pull/4289#discussion_r2039903313
##########
core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java:
##########
@@ -7389,15 +7396,50 @@ private SqlNode nthSelectItem(int ordinal, final SqlParserPos pos) {
/**
* Converts an expression into canonical form by fully-qualifying any
* identifiers. For common columns in USING, it will be converted to
- * COALESCE(A.col, B.col) AS col.
+ * COALESCE(A.col, B.col) AS col. When using a conformance that
+ * allows isSelectAlias, it expands identifiers that refer to
+ * local select expressions into the expressions themselves.
*/
static class SelectExpander extends Expander {
final SqlSelect select;
-
- SelectExpander(SqlValidatorImpl validator, SelectScope scope,
- SqlSelect select) {
+ // Maps simple identifiers to their expansions.
+ final Map<String, SqlNode> expansions;
+ // List of identifiers that are currently beeing looked-up. Used to detect
+ // circular dependencies when SqlConformance#isSelectAlias is ANY.
+ final Set<String> lookingUp;
+ @Nullable SqlNode root = null;
+
+ private SelectExpander(SqlValidatorImpl validator, SelectScope scope,
+ SqlSelect select, Map<String, SqlNode> expansions, Set<String> lookingUp) {
super(validator, scope);
this.select = select;
+ this.expansions = expansions;
+ this.lookingUp = lookingUp;
+ }
+
+ /**
+ * Create an expander for the items in a SELECT list.
Review Comment:
s/Create/Creates/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6939] Add support for Lateral Column Alias [calcite]
Posted by "julianhyde (via GitHub)" <gi...@apache.org>.
julianhyde commented on code in PR #4289:
URL: https://github.com/apache/calcite/pull/4289#discussion_r2039912897
##########
core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java:
##########
@@ -7406,7 +7448,78 @@ static class SelectExpander extends Expander {
if (node != id) {
return node;
} else {
- return super.visit(id);
+ try {
+ return super.visit(id);
+ } catch (Exception ex) {
+ if (!(ex instanceof CalciteContextException)) {
Review Comment:
This is a lot of code for a `catch (Exception)` block.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6939] Add support for Lateral Column Alias [calcite]
Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on code in PR #4289:
URL: https://github.com/apache/calcite/pull/4289#discussion_r2033551348
##########
core/src/main/java/org/apache/calcite/sql/validate/SqlConformance.java:
##########
@@ -102,6 +102,16 @@ public interface SqlConformance {
*/
boolean isGroupByAlias();
+ /**
+ * Whether to allow aliases from the {@code SELECT} clause to be used as
+ * column names in the same {@code SELECT} clause.
+ * E.g., SELECT 1 as X, X+1 as Y;
+ * Name lookup considers an identifier in the same SELECT only
+ * if other lookups failed.
+ * Supported by Spark, Snowflake, BigQuery.
Review Comment:
The SQL engine in Dataricks has been rewritten using CPP, which is different from the open source Spark. Perhaps there are some changes in the rewriting process, or there may be problems with the Spark documentation, which is difficult for us to verify. Perhaps we should focus on the open source Spark version.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6939] Add support for Lateral Column Alias [calcite]
Posted by "NobiGo (via GitHub)" <gi...@apache.org>.
NobiGo commented on code in PR #4289:
URL: https://github.com/apache/calcite/pull/4289#discussion_r2032687930
##########
core/src/main/java/org/apache/calcite/sql/validate/SqlConformance.java:
##########
@@ -102,6 +102,16 @@ public interface SqlConformance {
*/
boolean isGroupByAlias();
+ /**
+ * Whether to allow aliases from the {@code SELECT} clause to be used as
+ * column names in the same {@code SELECT} clause.
+ * E.g., SELECT 1 as X, X+1 as Y;
+ * Name lookup considers an identifier in the same SELECT only
+ * if other lookups failed.
+ * Supported by Spark, Snowflake, BigQuery.
Review Comment:
Spark currently does not support.
##########
core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java:
##########
@@ -6677,6 +6677,32 @@ public boolean isBangEqualAllowed() {
.withConformance(lenient).fails("Column 'E' not found in any table");
}
+ /** Test case for <a href="https://issues.apache.org/jira/browse/CALCITE-6939">
+ * [CALCITE-6939] Add support for Lateral Column Alias</a>. */
+ @Test void testAliasInSelect() {
+ final SqlConformance withSelectAlias = new SqlAbstractConformance() {
+ @Override public boolean isSelectAlias() {
+ return true;
+ }
+ };
+
+ // Standard conformance
+ sql("select 1 AS x, ^x^ as Y")
Review Comment:
1: Does this currently support SQL execution? Or is it only supporting validate?
2: If the table includes x and y columns, what would the output be if it is renamed as x?
Some cases I test in Spark:
```
create table table(x int,y int);
insert into table values(1, 2);
select 2 as x, x + 2 from table; // output=> 2 3
select 2 as x1, x1 + 2; // Error in query: cannot resolve '`x1`' given input columns: []; line 1 pos **17;**
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org