mihaibudiu commented on code in PR #3658:
URL: https://github.com/apache/calcite/pull/3658#discussion_r1470106502
##########
core/src/main/java/org/apache/calcite/rel/rel2sql/RelToSqlConverter.java:
##########
@@ -257,6 +257,48 @@ public Result visit(Join e) {
return result(join, leftResult, rightResult);
}
+ private Result maybeFixRenamedFields(Result rightResult, Join e) {
+ Frame last = stack.peekLast();
+ if (last != null && last.r instanceof TableModify) {
+ return rightResult;
+ }
+ List<String> rightFieldNames = e.getRight().getRowType().getFieldNames();
+ List<String> fieldNames = e.getRowType().getFieldNames();
+ int offset = e.getLeft().getRowType().getFieldCount();
+ boolean hasFieldNameCollision = false;
+ for (int i = 0; i < rightFieldNames.size(); i++) {
+ if (!rightFieldNames.get(i).equals(fieldNames.get(offset + i))) {
+ hasFieldNameCollision = true;
+ }
+ }
+ if (!hasFieldNameCollision) {
+ return rightResult;
+ }
+ Builder builder = rightResult.builder(e);
+ List<SqlNode> oldSelectList = new ArrayList<>();
+ if (builder.select.getSelectList() == SqlNodeList.SINGLETON_STAR) {
+ for (int i = 0; i < rightFieldNames.size(); i++) {
+ oldSelectList.add(new SqlIdentifier(rightFieldNames.get(i), POS));
+ }
+ } else {
+ for (SqlNode node : builder.select.getSelectList().getList()) {
+ oldSelectList.add(requireNonNull(node, "node"));
+ }
+ }
+ List<SqlNode> selectList = new ArrayList<>();
+ for (int i = 0; i < rightFieldNames.size(); i++) {
+ SqlNode column = oldSelectList.get(i);
+ if (!rightFieldNames.get(i).equals(fieldNames.get(offset + i))) {
+ column =
+ SqlStdOperatorTable.AS.createCall(POS, SqlUtil.stripAs(column),
Review Comment:
What guarantees that this name is unique?
This may be another column name that appears on the other side.
I think you need to create a Set with all "forbidden" names and make sure
that this name is not part of that set.
##########
core/src/main/java/org/apache/calcite/rel/rel2sql/RelToSqlConverter.java:
##########
@@ -257,6 +257,48 @@ public Result visit(Join e) {
return result(join, leftResult, rightResult);
}
+ private Result maybeFixRenamedFields(Result rightResult, Join e) {
Review Comment:
This doesn't seem to have any relation to JDBC.
Perhaps the issue should be renamed.
##########
core/src/main/java/org/apache/calcite/rel/rel2sql/RelToSqlConverter.java:
##########
@@ -225,7 +225,7 @@ public Result visit(Join e) {
break;
}
final Result leftResult = visitInput(e, 0).resetAlias();
- final Result rightResult = visitInput(e, 1).resetAlias();
+ Result rightResult = maybeFixRenamedFields(visitInput(e, 1).resetAlias(),
e);
Review Comment:
Can these two calls be on different lines? Helps with debugging.
Looks like you can keep the "final" too.
##########
core/src/main/java/org/apache/calcite/rel/rel2sql/RelToSqlConverter.java:
##########
@@ -257,6 +257,48 @@ public Result visit(Join e) {
return result(join, leftResult, rightResult);
}
+ private Result maybeFixRenamedFields(Result rightResult, Join e) {
Review Comment:
I suspect that this change may affect other open issues on JIRA, it looks
pretty basic.
Can you do a quick check?
##########
core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java:
##########
@@ -809,8 +809,8 @@ private static String toSql(RelNode root, SqlDialect
dialect,
// RelFieldTrimmer maybe build the RelNode.
relFn(fn).ok("SELECT \"t\".\"V\" AS \"l_v\"\n"
+ "FROM (VALUES (1, 2)) AS \"t\" (\"K\", \"V\")\n"
- + "INNER JOIN "
- + "(VALUES (1)) AS \"t0\" (\"K\") ON \"t\".\"K\" = \"t0\".\"K\"");
+ + "INNER JOIN (SELECT \"K\" AS \"K0\"\n"
Review Comment:
It's not easy to review all these changes. I checked some of 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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
