mihaibudiu commented on code in PR #5240:
URL: https://github.com/apache/calcite/pull/5240#discussion_r3942656280
##########
core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java:
##########
@@ -638,6 +638,107 @@ private List<String> deriveNaturalJoinColumnList(SqlJoin
join) {
getNamespaceOrThrow(join.getRight()).getRowType());
}
+ /** Expands an identifier that refers to a common column of a NATURAL or
+ * USING join into a qualified reference or, for FULL JOIN, a COALESCE
+ * expression. */
+ SqlNode expandCommonColumn(SqlSelect sqlSelect, SqlNode selectItem,
+ SelectScope scope) {
+ return expandCommonColumn(sqlSelect, selectItem, scope, true);
+ }
+
+ /** As {@link #expandCommonColumn(SqlSelect, SqlNode, SelectScope)}, but
+ * allows control over whether the result is wrapped with an {@code AS}
+ * operator to preserve the column alias. */
+ SqlNode expandCommonColumn(SqlSelect sqlSelect, SqlNode selectItem,
Review Comment:
the github UI doesn't make it easy to compare with the old version
I will assume that only code touching wrapAlias has changed
##########
core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java:
##########
@@ -638,6 +638,107 @@ private List<String> deriveNaturalJoinColumnList(SqlJoin
join) {
getNamespaceOrThrow(join.getRight()).getRowType());
}
+ /** Expands an identifier that refers to a common column of a NATURAL or
+ * USING join into a qualified reference or, for FULL JOIN, a COALESCE
+ * expression. */
+ SqlNode expandCommonColumn(SqlSelect sqlSelect, SqlNode selectItem,
+ SelectScope scope) {
+ return expandCommonColumn(sqlSelect, selectItem, scope, true);
+ }
+
+ /** As {@link #expandCommonColumn(SqlSelect, SqlNode, SelectScope)}, but
+ * allows control over whether the result is wrapped with an {@code AS}
+ * operator to preserve the column alias. */
+ SqlNode expandCommonColumn(SqlSelect sqlSelect, SqlNode selectItem,
+ SelectScope scope, boolean wrapAlias) {
+ if (!(selectItem instanceof SqlIdentifier)) {
+ return selectItem;
+ }
+
+ final SqlNode from = sqlSelect.getFrom();
+ if (!(from instanceof SqlJoin)) {
+ return selectItem;
+ }
+
+ final SqlIdentifier identifier = (SqlIdentifier) selectItem;
+ if (!identifier.isSimple()) {
+ // Qualified identifiers (e.g. t1.col) are returned unchanged.
+ // Validation of qualified common columns is performed before expansion,
+ // in validateSelectCommonColumns, where the original user-written
+ // identifier (with its source position) is still available.
+ return selectItem;
+ }
+
+ return expandExprFromJoin((SqlJoin) from, identifier, scope, wrapAlias);
+ }
+
+ private SqlNode expandExprFromJoin(SqlJoin join, SqlIdentifier identifier,
+ SelectScope scope, boolean wrapAlias) {
+ final List<String> commonColumnNames;
+ // must be NATURAL or USING here, and cannot specify NATURAL keyword with
USING clause
Review Comment:
Should this comment be after this if block?
Because the "must" implies that the last else branch should throw
Alternatively, the comment is wrong
##########
core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java:
##########
@@ -5325,9 +5426,19 @@ protected void validateOrderList(SqlSelect select) {
final SqlValidatorScope orderScope = getOrderScope(select);
requireNonNull(orderScope, "orderScope");
+ // Expand unqualified common columns of NATURAL or USING joins before
+ // regular expansion, which would fail with an ambiguity error.
+ final SelectScope selectScope = getRawSelectScopeNonNull(select);
List<SqlNode> expandList = new ArrayList<>();
for (SqlNode orderItem : orderList) {
- SqlNode expandedOrderItem = expand(orderItem, orderScope);
+ SqlNode expandedCommon =
+ expandCommonColumnInOrderBy(orderItem, select, selectScope);
+ final SqlNode expandedOrderItem;
+ if (expandedCommon != orderItem) {
+ expandedOrderItem = expand(expandedCommon, orderScope);
Review Comment:
so columns are expanded twice, first by expandCommonColumnInOrderBy then by
expand?
##########
core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java:
##########
@@ -5325,9 +5426,19 @@ protected void validateOrderList(SqlSelect select) {
final SqlValidatorScope orderScope = getOrderScope(select);
requireNonNull(orderScope, "orderScope");
+ // Expand unqualified common columns of NATURAL or USING joins before
+ // regular expansion, which would fail with an ambiguity error.
+ final SelectScope selectScope = getRawSelectScopeNonNull(select);
List<SqlNode> expandList = new ArrayList<>();
for (SqlNode orderItem : orderList) {
- SqlNode expandedOrderItem = expand(orderItem, orderScope);
+ SqlNode expandedCommon =
+ expandCommonColumnInOrderBy(orderItem, select, selectScope);
+ final SqlNode expandedOrderItem;
+ if (expandedCommon != orderItem) {
Review Comment:
is this "if" necessary? Don't both branches do the same thing?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
