Skip to content

Commit

Permalink
[CALCITE-6221] JDBC adapter generates invalid query when the same tab…
Browse files Browse the repository at this point in the history 
…le is joined multiple times
  • Loading branch information
@kramerul
kramerul committed Feb 2, 2024
1 parent 75511b8 commit 755feca
Show file tree
Hide file tree
Showing 3 changed files with 194 additions and 2 deletions.
32 changes: 32 additions & 0 deletions core/src/main/java/org/apache/calcite/rel/rel2sql/RelToSqlConverter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -437,11 +437,43 @@ public Result visit(Filter e) {
final Result x = visitInput(e, 0, Clause.WHERE);
parseCorrelTable(e, x);
final Builder builder = x.builder(e);
if (input instanceof Join) {
final Context context = x.qualifiedContext();
final ImmutableList.Builder<SqlNode> selectList = ImmutableList.builder();
// Fieldnames are unique since they are created by SqlValidatorUtil.deriveJoinRowType()
final List<String> uniqueFieldNames = input.getRowType().getFieldNames();
boolean selectListRequired = false;
for (int i = 0; i < context.fieldCount; i++) {
final SqlNode field = context.field(i);
final String fieldName = uniqueFieldNames.get(i);
if (fieldWasRenamedByJoinDueToNameClash(field, fieldName)) {
selectListRequired = true;
}
selectList.add(
SqlStdOperatorTable.AS.createCall(POS, field,
new SqlIdentifier(fieldName, POS)));
}
if (selectListRequired) {
builder.setSelect(new SqlNodeList(selectList.build(), POS));
}
}
builder.setWhere(builder.context.toSql(null, e.getCondition()));
return builder.result();
}
}

private static boolean fieldWasRenamedByJoinDueToNameClash(SqlNode field, String fieldName) {
if (!(field instanceof SqlIdentifier)) {
return true;
}
SqlIdentifier identifier = (SqlIdentifier) field;
if (identifier.isStar()) {
return true;
}
List<String> names = identifier.names;
return !Util.last(names).equals(fieldName);
}

/** Visits a Project; called by {@link #dispatch} via reflection. */
public Result visit(Project e) {
// If the input is a Sort, wrap SELECT is not required.
Expand Down
118 changes: 116 additions & 2 deletions core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3517,7 +3517,64 @@ private SqlDialect nonOrdinalDialect() {
+ " on p.\"product_class_id\" = pc.\"product_class_id\"\n"
+ "where c.\"city\" = 'San Francisco'\n"
+ "and pc.\"product_department\" = 'Snacks'\n";
final String expected = "SELECT *\n"
final String expected = "SELECT "
+ "\"sales_fact_1997\".\"product_id\" AS \"product_id\", "
+ "\"sales_fact_1997\".\"time_id\" AS \"time_id\", "
+ "\"sales_fact_1997\".\"customer_id\" AS \"customer_id\", "
+ "\"sales_fact_1997\".\"promotion_id\" AS \"promotion_id\", "
+ "\"sales_fact_1997\".\"store_id\" AS \"store_id\", "
+ "\"sales_fact_1997\".\"store_sales\" AS \"store_sales\", "
+ "\"sales_fact_1997\".\"store_cost\" AS \"store_cost\", "
+ "\"sales_fact_1997\".\"unit_sales\" AS \"unit_sales\", "
+ "\"customer\".\"customer_id\" AS \"customer_id0\", "
+ "\"customer\".\"account_num\" AS \"account_num\", "
+ "\"customer\".\"lname\" AS \"lname\", "
+ "\"customer\".\"fname\" AS \"fname\", "
+ "\"customer\".\"mi\" AS \"mi\", "
+ "\"customer\".\"address1\" AS \"address1\", "
+ "\"customer\".\"address2\" AS \"address2\", "
+ "\"customer\".\"address3\" AS \"address3\", "
+ "\"customer\".\"address4\" AS \"address4\", "
+ "\"customer\".\"city\" AS \"city\", "
+ "\"customer\".\"state_province\" AS \"state_province\", "
+ "\"customer\".\"postal_code\" AS \"postal_code\", "
+ "\"customer\".\"country\" AS \"country\", "
+ "\"customer\".\"customer_region_id\" AS \"customer_region_id\", "
+ "\"customer\".\"phone1\" AS \"phone1\", "
+ "\"customer\".\"phone2\" AS \"phone2\", "
+ "\"customer\".\"birthdate\" AS \"birthdate\", "
+ "\"customer\".\"marital_status\" AS \"marital_status\", "
+ "\"customer\".\"yearly_income\" AS \"yearly_income\", "
+ "\"customer\".\"gender\" AS \"gender\", "
+ "\"customer\".\"total_children\" AS \"total_children\", "
+ "\"customer\".\"num_children_at_home\" AS \"num_children_at_home\", "
+ "\"customer\".\"education\" AS \"education\", "
+ "\"customer\".\"date_accnt_opened\" AS \"date_accnt_opened\", "
+ "\"customer\".\"member_card\" AS \"member_card\", "
+ "\"customer\".\"occupation\" AS \"occupation\", "
+ "\"customer\".\"houseowner\" AS \"houseowner\", "
+ "\"customer\".\"num_cars_owned\" AS \"num_cars_owned\", "
+ "\"customer\".\"fullname\" AS \"fullname\", "
+ "\"product\".\"product_class_id\" AS \"product_class_id\", "
+ "\"product\".\"product_id\" AS \"product_id0\", "
+ "\"product\".\"brand_name\" AS \"brand_name\", "
+ "\"product\".\"product_name\" AS \"product_name\", "
+ "\"product\".\"SKU\" AS \"SKU\", "
+ "\"product\".\"SRP\" AS \"SRP\", "
+ "\"product\".\"gross_weight\" AS \"gross_weight\", "
+ "\"product\".\"net_weight\" AS \"net_weight\", "
+ "\"product\".\"recyclable_package\" AS \"recyclable_package\", "
+ "\"product\".\"low_fat\" AS \"low_fat\", "
+ "\"product\".\"units_per_case\" AS \"units_per_case\", "
+ "\"product\".\"cases_per_pallet\" AS \"cases_per_pallet\", "
+ "\"product\".\"shelf_width\" AS \"shelf_width\", "
+ "\"product\".\"shelf_height\" AS \"shelf_height\", "
+ "\"product\".\"shelf_depth\" AS \"shelf_depth\", "
+ "\"product_class\".\"product_class_id\" AS \"product_class_id0\", "
+ "\"product_class\".\"product_subcategory\" AS \"product_subcategory\", "
+ "\"product_class\".\"product_category\" AS \"product_category\", "
+ "\"product_class\".\"product_department\" AS \"product_department\", "
+ "\"product_class\".\"product_family\" AS \"product_family\"\n"
+ "FROM \"foodmart\".\"sales_fact_1997\"\n"
+ "INNER JOIN \"foodmart\".\"customer\" "
+ "ON \"sales_fact_1997\".\"customer_id\" = \"customer\""
Expand Down Expand Up @@ -5328,7 +5385,64 @@ private void checkLiteral2(String expression, String expected) {
+ " up as up.\"net_weight\" > prev(up.\"net_weight\")\n"
+ " ) mr order by MR.\"net_weight\"";
final String expected = "SELECT *\n"
+ "FROM (SELECT *\n"
+ "FROM (SELECT "
+ "\"sales_fact_1997\".\"product_id\" AS \"product_id\", "
+ "\"sales_fact_1997\".\"time_id\" AS \"time_id\", "
+ "\"sales_fact_1997\".\"customer_id\" AS \"customer_id\", "
+ "\"sales_fact_1997\".\"promotion_id\" AS \"promotion_id\", "
+ "\"sales_fact_1997\".\"store_id\" AS \"store_id\", "
+ "\"sales_fact_1997\".\"store_sales\" AS \"store_sales\", "
+ "\"sales_fact_1997\".\"store_cost\" AS \"store_cost\", "
+ "\"sales_fact_1997\".\"unit_sales\" AS \"unit_sales\", "
+ "\"customer\".\"customer_id\" AS \"customer_id0\", "
+ "\"customer\".\"account_num\" AS \"account_num\", "
+ "\"customer\".\"lname\" AS \"lname\", "
+ "\"customer\".\"fname\" AS \"fname\", "
+ "\"customer\".\"mi\" AS \"mi\", "
+ "\"customer\".\"address1\" AS \"address1\", "
+ "\"customer\".\"address2\" AS \"address2\", "
+ "\"customer\".\"address3\" AS \"address3\", "
+ "\"customer\".\"address4\" AS \"address4\", "
+ "\"customer\".\"city\" AS \"city\", "
+ "\"customer\".\"state_province\" AS \"state_province\", "
+ "\"customer\".\"postal_code\" AS \"postal_code\", "
+ "\"customer\".\"country\" AS \"country\", "
+ "\"customer\".\"customer_region_id\" AS \"customer_region_id\", "
+ "\"customer\".\"phone1\" AS \"phone1\", "
+ "\"customer\".\"phone2\" AS \"phone2\", "
+ "\"customer\".\"birthdate\" AS \"birthdate\", "
+ "\"customer\".\"marital_status\" AS \"marital_status\", "
+ "\"customer\".\"yearly_income\" AS \"yearly_income\", "
+ "\"customer\".\"gender\" AS \"gender\", "
+ "\"customer\".\"total_children\" AS \"total_children\", "
+ "\"customer\".\"num_children_at_home\" AS \"num_children_at_home\", "
+ "\"customer\".\"education\" AS \"education\", "
+ "\"customer\".\"date_accnt_opened\" AS \"date_accnt_opened\", "
+ "\"customer\".\"member_card\" AS \"member_card\", "
+ "\"customer\".\"occupation\" AS \"occupation\", "
+ "\"customer\".\"houseowner\" AS \"houseowner\", "
+ "\"customer\".\"num_cars_owned\" AS \"num_cars_owned\", "
+ "\"customer\".\"fullname\" AS \"fullname\", "
+ "\"product\".\"product_class_id\" AS \"product_class_id\", "
+ "\"product\".\"product_id\" AS \"product_id0\", "
+ "\"product\".\"brand_name\" AS \"brand_name\", "
+ "\"product\".\"product_name\" AS \"product_name\", "
+ "\"product\".\"SKU\" AS \"SKU\", "
+ "\"product\".\"SRP\" AS \"SRP\", "
+ "\"product\".\"gross_weight\" AS \"gross_weight\", "
+ "\"product\".\"net_weight\" AS \"net_weight\", "
+ "\"product\".\"recyclable_package\" AS \"recyclable_package\", "
+ "\"product\".\"low_fat\" AS \"low_fat\", "
+ "\"product\".\"units_per_case\" AS \"units_per_case\", "
+ "\"product\".\"cases_per_pallet\" AS \"cases_per_pallet\", "
+ "\"product\".\"shelf_width\" AS \"shelf_width\", "
+ "\"product\".\"shelf_height\" AS \"shelf_height\", "
+ "\"product\".\"shelf_depth\" AS \"shelf_depth\", "
+ "\"product_class\".\"product_class_id\" AS \"product_class_id0\", "
+ "\"product_class\".\"product_subcategory\" AS \"product_subcategory\", "
+ "\"product_class\".\"product_category\" AS \"product_category\", "
+ "\"product_class\".\"product_department\" AS \"product_department\", "
+ "\"product_class\".\"product_family\" AS \"product_family\"\n"
+ "FROM \"foodmart\".\"sales_fact_1997\"\n"
+ "INNER JOIN \"foodmart\".\"customer\" "
+ "ON \"sales_fact_1997\".\"customer_id\" = \"customer\".\"customer_id\"\n"
Expand Down
46 changes: 46 additions & 0 deletions core/src/test/java/org/apache/calcite/test/JdbcAdapterTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1147,6 +1147,52 @@ private LockWrapper exclusiveCleanDb(Connection c) throws SQLException {
});
}

/**
* Test case for
* <a href="https://issues.apache.org/jira/browse/CALCITE-6221">[CALCITE-6221]</a>.*/
@Test void testUnknownColumn() {
CalciteAssert.model(JdbcTest.SCOTT_MODEL)
.query("SELECT\n"
+ " \"content-format-owner\",\n"
+ " \"content-owner\"\n"
+ "FROM\n"
+ " (\n"
+ " SELECT\n"
+ " d1.dname AS \"content-format-owner\",\n"
+ " d2.dname || ' ' AS \"content-owner\"\n"
+ " FROM\n"
+ " scott.emp e1\n"
+ " left outer join scott.dept d1 on e1.deptno = d1.deptno\n"
+ " left outer join scott.dept d2 on e1.deptno = d2.deptno\n"
+ " left outer join scott.emp e2 on e1.deptno = e2.deptno\n"
+ " GROUP BY\n"
+ " d1.dname,\n"
+ " d2.dname\n"
+ " )\n"
+ "WHERE\n"
+ " \"content-owner\" IN (?)")
.planHasSql("SELECT "
+ "\"t2\".\"DNAME\" AS \"content-format-owner\", "
+ "\"t2\".\"DNAME0\" || ' ' AS \"content-owner\"\n"
+ "FROM (SELECT \"t\".\"DEPTNO\" AS \"DEPTNO\", "
+ "\"t0\".\"DEPTNO\" AS \"DEPTNO0\", "
+ "\"t0\".\"DNAME\" AS \"DNAME\", "
+ "\"t1\".\"DEPTNO\" AS \"DEPTNO1\", "
+ "\"t1\".\"DNAME\" AS \"DNAME0\"\n"
+ "FROM (SELECT \"DEPTNO\"\n"
+ "FROM \"SCOTT\".\"EMP\") AS \"t\"\n"
+ "LEFT JOIN (SELECT \"DEPTNO\", \"DNAME\"\n"
+ "FROM \"SCOTT\".\"DEPT\") AS \"t0\" ON \"t\".\"DEPTNO\" = \"t0\".\"DEPTNO\"\n"
+ "LEFT JOIN (SELECT \"DEPTNO\", \"DNAME\"\n"
+ "FROM \"SCOTT\".\"DEPT\") AS \"t1\" "
+ "ON \"t\".\"DEPTNO\" = \"t1\".\"DEPTNO\"\n"
+ "WHERE \"t1\".\"DNAME\" || ' ' = ?) AS \"t2\"\n"
+ "LEFT JOIN (SELECT \"DEPTNO\"\n"
+ "FROM \"SCOTT\".\"EMP\") AS \"t3\" "
+ "ON \"t2\".\"DEPTNO\" = \"t3\".\"DEPTNO\"\n"
+ "GROUP BY \"t2\".\"DNAME\", \"t2\".\"DNAME0\"")
.runs();
}
/** Acquires a lock, and releases it when closed. */
static class LockWrapper implements AutoCloseable {
private final Lock lock;
Expand Down

0 comments on commit 755feca

Please sign in to comment.