This is an automated email from the ASF dual-hosted git repository.
zhenchen pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/calcite.git
The following commit(s) were added to refs/heads/main by this push:
new 252d6aa7d4 [CALCITE-7051] NATURAL JOIN and JOIN with USING does not
match the appropriate columns when caseSensitive is false
252d6aa7d4 is described below
commit 252d6aa7d4e922a1e5e233410c33b1dc1505007a
Author: Xiong Tenghui <[email protected]>
AuthorDate: Sun Jun 8 00:09:57 2025 +0800
[CALCITE-7051] NATURAL JOIN and JOIN with USING does not match the
appropriate columns when caseSensitive is false
---
.../calcite/sql/validate/SqlValidatorImpl.java | 154 ++++++++--------
.../org/apache/calcite/test/SqlValidatorTest.java | 194 +++++++++++++++++++++
2 files changed, 273 insertions(+), 75 deletions(-)
diff --git
a/core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java
b/core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java
index 69b2baf2d4..c3d93072b7 100644
--- a/core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java
+++ b/core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java
@@ -526,56 +526,6 @@ private boolean expandSelectItem(final SqlNode selectItem,
SqlSelect select,
return false;
}
- private static SqlNode expandExprFromJoin(SqlJoin join,
- SqlIdentifier identifier, SelectScope scope) {
- if (join.getConditionType() != JoinConditionType.USING) {
- return identifier;
- }
-
- final Map<String, String> fieldAliases = getFieldAliases(scope);
-
- for (String name
- : SqlIdentifier.simpleNames((SqlNodeList) getCondition(join))) {
- if (identifier.getSimple().equals(name)) {
- final List<SqlNode> qualifiedNode = new ArrayList<>();
- for (ScopeChild child : requireNonNull(scope, "scope").children) {
- if (child.namespace.getRowType().getFieldNames().contains(name)) {
- final SqlIdentifier exp =
- new SqlIdentifier(
- ImmutableList.of(child.name, name),
- identifier.getParserPosition());
- qualifiedNode.add(exp);
- }
- }
-
- assert qualifiedNode.size() == 2;
-
- // If there is an alias for the column, no need to wrap the coalesce
with an AS operator
- boolean haveAlias = fieldAliases.containsKey(name);
-
- final SqlCall coalesceCall =
- SqlStdOperatorTable.COALESCE.createCall(SqlParserPos.ZERO,
qualifiedNode.get(0),
- qualifiedNode.get(1));
-
- if (haveAlias) {
- return coalesceCall;
- } else {
- return SqlStdOperatorTable.AS.createCall(SqlParserPos.ZERO,
coalesceCall,
- new SqlIdentifier(name, SqlParserPos.ZERO));
- }
- }
- }
-
- // Only need to try to expand the expr from the left input of join
- // since it is always left-deep join.
- final SqlNode node = join.getLeft();
- if (node instanceof SqlJoin) {
- return expandExprFromJoin((SqlJoin) node, identifier, scope);
- } else {
- return identifier;
- }
- }
-
private static Map<String, String> getFieldAliases(final SelectScope scope) {
final ImmutableMap.Builder<String, String> fieldAliases = new
ImmutableMap.Builder<>();
@@ -623,28 +573,6 @@ private List<String> deriveNaturalJoinColumnList(SqlJoin
join) {
getNamespaceOrThrow(join.getRight()).getRowType());
}
- private static SqlNode expandCommonColumn(SqlSelect sqlSelect,
- SqlNode selectItem, SelectScope scope, SqlValidatorImpl validator) {
- 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()) {
- if (!validator.config().conformance().allowQualifyingCommonColumn()) {
- validateQualifiedCommonColumn((SqlJoin) from, identifier, scope,
validator);
- }
- return selectItem;
- }
-
- return expandExprFromJoin((SqlJoin) from, identifier, scope);
- }
-
private static void validateQualifiedCommonColumn(SqlJoin join,
SqlIdentifier identifier, SelectScope scope, SqlValidatorImpl validator)
{
List<String> names = validator.usingNames(join);
@@ -7312,6 +7240,83 @@ protected SqlNode expandDynamicStar(SqlIdentifier id,
SqlIdentifier fqId) {
}
return fqId;
}
+
+ protected SqlNode expandCommonColumn(SqlSelect sqlSelect, SqlNode
selectItem,
+ SelectScope scope) {
+ 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()) {
+ if (!validator.config().conformance().allowQualifyingCommonColumn()) {
+ validateQualifiedCommonColumn((SqlJoin) from, identifier, scope,
validator);
+ }
+ return selectItem;
+ }
+
+ return expandExprFromJoin((SqlJoin) from, identifier, scope);
+ }
+
+ private SqlNode expandExprFromJoin(SqlJoin join, SqlIdentifier identifier,
SelectScope scope) {
+ List<String> commonColumnNames;
+ // must be NATURAL or USING here, and cannot specify NATURAL keyword
with USING clause
+ if (join.isNatural()) {
+ commonColumnNames = validator.deriveNaturalJoinColumnList(join);
+ } else if (join.getConditionType() == JoinConditionType.USING) {
+ commonColumnNames = SqlIdentifier.simpleNames((SqlNodeList)
getCondition(join));
+ } else {
+ return identifier;
+ }
+
+ final SqlNameMatcher matcher =
validator.getCatalogReader().nameMatcher();
+ final Map<String, String> fieldAliases = getFieldAliases(scope);
+
+ for (String name : commonColumnNames) {
+ if (matcher.matches(identifier.getSimple(), name)) {
+ final List<SqlNode> qualifiedNode = new ArrayList<>();
+ for (ScopeChild child : requireNonNull(scope, "scope").children) {
+ if (matcher.indexOf(child.namespace.getRowType().getFieldNames(),
name) >= 0) {
+ final SqlIdentifier exp =
+ new SqlIdentifier(
+ ImmutableList.of(child.name, name),
+ identifier.getParserPosition());
+ qualifiedNode.add(exp);
+ }
+ }
+
+ assert qualifiedNode.size() == 2;
+
+ // If there is an alias for the column, no need to wrap the coalesce
with an AS operator
+ boolean haveAlias = fieldAliases.containsKey(name);
+
+ final SqlCall coalesceCall =
+ SqlStdOperatorTable.COALESCE.createCall(SqlParserPos.ZERO,
qualifiedNode.get(0),
+ qualifiedNode.get(1));
+
+ if (haveAlias) {
+ return coalesceCall;
+ } else {
+ return SqlStdOperatorTable.AS.createCall(SqlParserPos.ZERO,
coalesceCall,
+ new SqlIdentifier(identifier.getSimple(), SqlParserPos.ZERO));
+ }
+ }
+ }
+
+ // Only need to try to expand the expr from the left input of join
+ // since it is always left-deep join.
+ final SqlNode node = join.getLeft();
+ if (node instanceof SqlJoin) {
+ return expandExprFromJoin((SqlJoin) node, identifier, scope);
+ } else {
+ return identifier;
+ }
+ }
}
/**
@@ -7468,8 +7473,7 @@ private SelectExpander(SelectExpander expander) {
}
@Override public @Nullable SqlNode visit(SqlIdentifier id) {
- final SqlNode node =
- expandCommonColumn(select, id, (SelectScope) getScope(), validator);
+ final SqlNode node = expandCommonColumn(select, id, (SelectScope)
getScope());
if (node != id) {
return node;
} else {
@@ -7587,7 +7591,7 @@ static class ExtendedExpander extends Expander {
try {
// First try a standard expansion
if (clause == Clause.GROUP_BY) {
- SqlNode node = expandCommonColumn(select, id, selectScope,
validator);
+ SqlNode node = expandCommonColumn(select, id, selectScope);
if (node != id) {
return node;
}
diff --git a/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java
b/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java
index 958be01b8b..8470bb61d1 100644
--- a/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java
+++ b/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java
@@ -61,6 +61,7 @@
import org.apache.calcite.sql.validate.SqlValidatorImpl;
import org.apache.calcite.sql.validate.SqlValidatorUtil;
import org.apache.calcite.test.catalog.CountingFactory;
+import org.apache.calcite.test.catalog.MockCatalogReaderSimple;
import org.apache.calcite.test.catalog.MustFilterMockCatalogReader;
import org.apache.calcite.testlib.annotations.LocaleEnUs;
import org.apache.calcite.util.Bug;
@@ -68,8 +69,11 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Ordering;
+import com.google.common.collect.Sets;
+import org.checkerframework.checker.nullness.qual.NonNull;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
@@ -86,6 +90,7 @@
import java.util.Locale;
import java.util.Map;
import java.util.Objects;
+import java.util.Set;
import java.util.TreeSet;
import java.util.function.BiConsumer;
import java.util.function.Consumer;
@@ -5486,6 +5491,195 @@ private ImmutableList<ImmutableBitSet>
cube(ImmutableBitSet... sets) {
.ok();
}
+ /** Test case for
+ * <a
href="https://issues.apache.org/jira/browse/CALCITE-7051";>[CALCITE-7051]
+ * JOIN with USING does not match the appropriate columns when caseSensitive
is false</a>. */
+ @Test void testSelectJoinUsingCommonColumnCaseSensitive() {
+ ImmutableSet<String> columnNames = ImmutableSet.of("DEPTNO", "deptno",
"DeptNo", "dEptNO");
+ ImmutableList<String> empTableNames =
+ ImmutableList.of("S.EMP_UPPER", "S.emp_lower", "S.Emp_Mixed");
+ ImmutableList<String> deptTableNames =
+ ImmutableList.of("S.DEPT_UPPER", "S.dept_lower", "S.Dept_Mixed");
+ Set<List<String>> cartesianedSet =
+ Sets.cartesianProduct(
+ columnNames,
+ ImmutableSet.copyOf(empTableNames),
+ ImmutableSet.copyOf(deptTableNames),
+ columnNames);
+
+ for (List<String> list : cartesianedSet) {
+ String sql =
+ String.format(Locale.ROOT, "select %s from %s join %s using (%s)",
list.get(0),
+ list.get(1),
+ list.get(2),
+ list.get(3));
+ joinCommonColumnsFixture(false)
+ .withSql(sql)
+ .ok();
+ }
+
+ ImmutableList<String> columnNames2 = ImmutableList.of("DEPTNO", "deptno",
"DeptNo");
+ for (String columnName : columnNames2) {
+ String sql =
+ String.format(Locale.ROOT, "select %s from S.EMP_BOTH join
S.DEPT_BOTH using (%s)",
+ columnName,
+ columnName);
+ joinCommonColumnsFixture(true)
+ .withSql(sql)
+ .ok();
+ }
+
+ joinCommonColumnsFixture(true)
+ .withSql("select deptno,DeptNo,DEPTNO "
+ + "from S.EMP_BOTH join S.DEPT_BOTH using (deptno,DeptNo,DEPTNO)")
+ .ok();
+
+ joinCommonColumnsFixture(true)
+ .withSql("select ^dEptnO^ from S.EMP_BOTH join S.DEPT_BOTH using
(deptno,DeptNo,DEPTNO)")
+ .fails("Column 'dEptnO' not found in any table; did you mean 'DEPTNO',
'DEPTNO'\\?");
+
+ for (int i = 0; i < columnNames2.size(); i++) {
+ String sql =
+ String.format(Locale.ROOT, "select %s from %s join %s using (%s)",
columnNames2.get(i),
+ empTableNames.get(i),
+ deptTableNames.get(i),
+ columnNames2.get(i));
+ joinCommonColumnsFixture(true)
+ .withSql(sql)
+ .ok();
+ }
+
+
+ joinCommonColumnsFixture(true)
+ .withSql("select ^deptno^ from S.EMP_UPPER join S.DEPT_UPPER using
(DEPTNO)")
+ .fails("Column 'deptno' not found in any table; did you mean 'DEPTNO',
'DEPTNO'\\?");
+
+ joinCommonColumnsFixture(true)
+ .withSql("select DEPTNO from S.EMP_UPPER join S.DEPT_UPPER using
(^deptno^)")
+ .fails("Column 'deptno' not found in any table");
+
+ joinCommonColumnsFixture(true)
+ .withSql("select DEPTNO from S.EMP_UPPER join S.dept_lower using
(^DEPTNO^)")
+ .fails("Column 'DEPTNO' not found in any table");
+ }
+
+ /** Test case for
+ * <a
href="https://issues.apache.org/jira/browse/CALCITE-7051";>[CALCITE-7051]
+ * JOIN with USING does not match the appropriate columns when caseSensitive
is false</a>.
+ * select common columns in NATURAL JOIN*/
+ @Test void testNaturalJoinCommonColumn() {
+ ImmutableSet<String> columnNames = ImmutableSet.of("DEPTNO", "deptno",
"DeptNo", "dEptNO");
+ ImmutableList<String> empTableNames =
+ ImmutableList.of("S.EMP_UPPER", "S.emp_lower", "S.Emp_Mixed");
+ ImmutableList<String> deptTableNames =
+ ImmutableList.of("S.DEPT_UPPER", "S.dept_lower", "S.Dept_Mixed");
+ Set<List<String>> cartesianedSet =
+ Sets.cartesianProduct(
+ columnNames,
+ ImmutableSet.copyOf(empTableNames),
+ ImmutableSet.copyOf(deptTableNames),
+ columnNames);
+ for (List<String> list : cartesianedSet) {
+ String sql =
+ String.format(Locale.ROOT, "select %s from %s natural join %s",
list.get(0),
+ list.get(1),
+ list.get(2));
+ joinCommonColumnsFixture(false)
+ .withSql(sql)
+ .ok();
+ }
+
+ ImmutableList<String> columnNames2 = ImmutableList.of("DEPTNO", "deptno",
"DeptNo");
+ for (String columnName : columnNames2) {
+ String sql =
+ String.format(Locale.ROOT, "select %s from S.EMP_BOTH natural join
S.DEPT_BOTH",
+ columnName);
+ joinCommonColumnsFixture(true)
+ .withSql(sql)
+ .ok();
+ }
+ joinCommonColumnsFixture(true)
+ .withSql("select deptno,DeptNo,DEPTNO from S.EMP_BOTH natural join
S.DEPT_BOTH")
+ .ok();
+ joinCommonColumnsFixture(true)
+ .withSql("select ^dEptnO^ from S.EMP_BOTH natural join S.DEPT_BOTH")
+ .fails("Column 'dEptnO' not found in any table; did you mean 'DEPTNO',
'DEPTNO'\\?");
+
+ for (int i = 0; i < columnNames2.size(); i++) {
+ String sql =
+ String.format(Locale.ROOT, "select %s from %s natural join %s",
columnNames2.get(i),
+ empTableNames.get(i),
+ deptTableNames.get(i));
+ joinCommonColumnsFixture(true)
+ .withSql(sql)
+ .ok();
+ }
+ joinCommonColumnsFixture(true)
+ .withSql("select ^deptno^ from S.EMP_UPPER natural join S.DEPT_UPPER")
+ .fails("Column 'deptno' not found in any table; did you mean 'DEPTNO',
'DEPTNO'\\?");
+ // If case sensensitive, no common column in natural join
+ joinCommonColumnsFixture(true)
+ .withSql("select deptno,DEPTNO from S.EMP_UPPER natural join
S.dept_lower")
+ .ok();
+ }
+
+ /** Mock catalog reader for registering tables with different case. */
+ private static class JoinCommonColumnsTestCatalogReader
+ extends MockCatalogReaderSimple {
+ JoinCommonColumnsTestCatalogReader(RelDataTypeFactory typeFactory,
+ boolean caseSensitive) {
+ super(typeFactory, caseSensitive);
+ }
+
+ public static @NonNull JoinCommonColumnsTestCatalogReader create(
+ RelDataTypeFactory typeFactory, boolean caseSensitive) {
+ return new JoinCommonColumnsTestCatalogReader(typeFactory,
caseSensitive).init();
+ }
+
+ @Override public JoinCommonColumnsTestCatalogReader init() {
+ super.init();
+ MockSchema tSchema = new MockSchema("S");
+ registerSchema(tSchema);
+ registerDeptTable(tSchema, "DEPT_UPPER", "DEPTNO");
+ registerDeptTable(tSchema, "dept_lower", "deptno");
+ registerDeptTable(tSchema, "Dept_Mixed", "DeptNo");
+ registerDeptTable(tSchema, "DEPT_BOTH", "DEPTNO", "deptno", "DeptNo");
+ registerEmpTable(tSchema, "EMP_UPPER", "DEPTNO");
+ registerEmpTable(tSchema, "emp_lower", "deptno");
+ registerEmpTable(tSchema, "Emp_Mixed", "DeptNo");
+ registerEmpTable(tSchema, "EMP_BOTH", "DEPTNO", "deptno", "DeptNo");
+ return this;
+ }
+
+ private void registerDeptTable(MockSchema tSchema, String tableName,
String... deptnoColNames) {
+ MockTable t = MockTable.create(this, tSchema, tableName, false, 4);
+ for (String deptnoColName : deptnoColNames) {
+ t.addColumn(deptnoColName,
typeFactory.createSqlType(SqlTypeName.INTEGER));
+ }
+ t.addColumn("NAME", typeFactory.createSqlType(SqlTypeName.VARCHAR));
+ registerTable(t);
+ }
+
+ private void registerEmpTable(MockSchema tSchema, String tableName,
String... deptnoColNames) {
+ MockTable t = MockTable.create(this, tSchema, tableName, false, 4);
+ for (String deptnoColName : deptnoColNames) {
+ t.addColumn(deptnoColName,
typeFactory.createSqlType(SqlTypeName.INTEGER));
+ }
+ t.addColumn("EMPNO", typeFactory.createSqlType(SqlTypeName.VARCHAR));
+ t.addColumn("ENAME", typeFactory.createSqlType(SqlTypeName.INTEGER));
+ registerTable(t);
+ }
+ }
+
+ private SqlValidatorFixture joinCommonColumnsFixture(boolean caseSensitive) {
+ return fixture()
+ .withFactory(t ->
t.withCatalogReader(JoinCommonColumnsTestCatalogReader::create))
+ .withCaseSensitive(caseSensitive)
+ .withValidatorIdentifierExpansion(true)
+ .withQuotedCasing(Casing.UNCHANGED)
+ .withUnquotedCasing(Casing.UNCHANGED);
+ }
+
@Test void testCrossJoinOnFails() {
sql("select * from emp cross join dept\n"
+ " ^on^ emp.deptno = dept.deptno")
