Skip to content

Commit

Permalink
[CALCITE-6837] Invalid code generated for ROW_NUMBER function in Enum…
Browse files Browse the repository at this point in the history
…erable convention
  • Loading branch information
NobiGo authored and mihaibudiu committed Feb 26, 2025
1 parent 3242604 commit 4ee2e41
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import org.apache.calcite.adapter.java.JavaTypeFactory;
import org.apache.calcite.config.CalciteSystemProperty;
import org.apache.calcite.linq4j.tree.BlockBuilder;
import org.apache.calcite.linq4j.tree.BlockStatement;
import org.apache.calcite.linq4j.tree.ConstantExpression;
import org.apache.calcite.linq4j.tree.Expression;
import org.apache.calcite.linq4j.tree.Expressions;
Expand Down Expand Up @@ -133,7 +134,13 @@ protected JdbcToEnumerableConverter(
default:
calendar_ = null;
}
if (fieldCount == 1) {
if (fieldCount == 0) {
// return (Object) null;
final ParameterExpression value_ =
Expressions.parameter(Object.class, builder.newName("value"));
builder.add(Expressions.declare(Modifier.FINAL, value_, null));
builder.add(Expressions.return_(null, value_));
} else if (fieldCount == 1) {
final ParameterExpression value_ =
Expressions.parameter(Object.class, builder.newName("value"));
builder.add(Expressions.declare(Modifier.FINAL, value_, null));
Expand All @@ -155,22 +162,53 @@ protected JdbcToEnumerableConverter(
}
final ParameterExpression e_ =
Expressions.parameter(SQLException.class, builder.newName("e"));
BlockStatement valueBlock;
if (fieldCount == 0) {
// For some queries, we don't need to rely on a specific column value in the data source,
// we only need to know the number of rows in the table.
// For example: "select row_number() over () from dept"
// we can't push down the "row_number() over ()"
// So the generated code should be:
// public org.apache.calcite.linq4j.function.Function0 apply(
// final java.sql.ResultSet resultSet) {
// return new org.apache.calcite.linq4j.function.Function0() {
// public Object apply() {
// return (Object) null;
// }
// };
// }
valueBlock = builder.toBlock();
} else {
// public org.apache.calcite.linq4j.function.Function0 apply(
// final java.sql.ResultSet resultSet) {
// return new org.apache.calcite.linq4j.function.Function0() {
// public Object apply() {
// try {
// return resultSet.getString(1);
// } catch (java.sql.SQLException e) {
// throw new RuntimeException(
// e);
// }
// }
// };
// }
valueBlock =
Expressions.block(
Expressions.tryCatch(
builder.toBlock(),
Expressions.catch_(
e_,
Expressions.throw_(
Expressions.new_(
RuntimeException.class,
e_)))));
}
final Expression rowBuilderFactory_ =
builder0.append("rowBuilderFactory",
Expressions.lambda(
Expressions.block(
Expressions.return_(null,
Expressions.lambda(
Expressions.block(
Expressions.tryCatch(
builder.toBlock(),
Expressions.catch_(
e_,
Expressions.throw_(
Expressions.new_(
RuntimeException.class,
e_)))))))),
resultSet_));
Expressions.lambda(valueBlock))), resultSet_));

final Expression enumerable;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -525,7 +525,10 @@ public Result visit(Project e) {
}
addSelect(selectList, sqlExpr, e.getRowType());
}

// We generate "SELECT 1 FROM EMP" replace "SELECT FROM EMP"
if (selectList.isEmpty()) {
selectList.add(SqlLiteral.createExactNumeric("1", POS));
}
final SqlNodeList selectNodeList = new SqlNodeList(selectList, POS);
if (builder.select.getGroup() == null
&& builder.select.getHaving() == null
Expand Down
13 changes: 13 additions & 0 deletions core/src/test/java/org/apache/calcite/test/JdbcTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -4782,6 +4782,19 @@ private void startOfGroupStep3(String startOfGroup) {
"deptno=20; empid=200; commission=500; R=1; RCNF=1; RCNL=1; R=1; RD=1");
}

/** Test case for
* <a href="https://issues.apache.org/jira/browse/CALCITE-6837">[CALCITE-6837]
* Invalid code generated for ROW_NUMBER function in Enumerable convention</a>. */
@Test void testWinRowNumber1() {
CalciteAssert.that()
.with(CalciteAssert.Config.JDBC_SCOTT)
.query("select row_number() over () from dept")
.returnsUnordered("EXPR$0=1",
"EXPR$0=2",
"EXPR$0=3",
"EXPR$0=4");
}

/** Tests UNBOUNDED PRECEDING clause. */
@Test void testOverUnboundedPreceding() {
CalciteAssert.hr()
Expand Down

0 comments on commit 4ee2e41

Please sign in to comment.