Skip to content

Commit

Permalink
[CALCITE-6559] Query with measure that applies AVG to SMALLINT throws…
Browse files Browse the repository at this point in the history
… AssertionError "Cannot add expression of different type to set"

When a query has a measure that is AVG applied to a SMALLINT or TINYINT
column, we get the following assertion failure:

  java.lang.AssertionError: Cannot add expression of different type to set:
  set type is RecordType(VARCHAR(9) JOB, MEASURE<SMALLINT NOT NULL> NOT NULL $f1, SMALLINT $f0) NOT NULL
  expression type is RecordType(VARCHAR(9) JOB, MEASURE<SMALLINT NOT NULL> NOT NULL $f1, SMALLINT NOT NULL $f0) NOT NULL

The problem is that a cast is added internally to the result of
SUM / COUNT, but this cast prevents a cast to nullable for the
null-generating side of the outer join. (The outer join comes from
the expansion of the measure as a correlated scalar subquery.)

Add method RexBuilder.makeNullable, sharing implementation with
existing method makeNotNull.

Add tests to scalar.iq and lateral.iq for some tricky query patterns
that already work.

Close #3950
  • Loading branch information
julianhyde committed Sep 5, 2024
1 parent b2224a8 commit ded0651
Show file tree
Hide file tree
Showing 5 changed files with 169 additions and 8 deletions.
28 changes: 22 additions & 6 deletions core/src/main/java/org/apache/calcite/rex/RexBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -960,17 +960,33 @@ public RexNode makeReinterpretCast(
}

/**
* Makes a cast of a value to NOT NULL;
* no-op if the type already has NOT NULL.
* Makes a cast of an expression to nullable;
* returns the expression unchanged if its type is already nullable.
*/
public RexNode makeNullable(RexNode exp) {
return makeNullable(exp, true);
}

/**
* Makes a cast of an expression to NOT NULL;
* returns the expression unchanged if its type already has NOT NULL.
*/
public RexNode makeNotNull(RexNode exp) {
return makeNullable(exp, false);
}

/**
* Makes a cast of an expression to the required nullability; returns
* the expression unchanged if its type already has the desired nullability.
*/
public RexNode makeNullable(RexNode exp, boolean nullability) {
final RelDataType type = exp.getType();
if (!type.isNullable()) {
if (type.isNullable() == nullability) {
return exp;
}
final RelDataType notNullType =
typeFactory.createTypeWithNullability(type, false);
return makeAbstractCast(notNullType, exp, false);
final RelDataType type2 =
typeFactory.createTypeWithNullability(type, nullability);
return makeAbstractCast(type2, exp, false);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -412,7 +412,13 @@ protected RexNode removeCorrelationExpr(
RemoveCorrelationRexShuttle shuttle =
new RemoveCorrelationRexShuttle(relBuilder.getRexBuilder(),
projectPulledAboveLeftCorrelator, null, isCount);
return exp.accept(shuttle);
RexNode exp2 = exp.accept(shuttle);

// Fix the nullability.
if (projectPulledAboveLeftCorrelator) {
exp2 = relBuilder.getRexBuilder().makeNullable(exp2);
}
return exp2;
}

/** Fallback if none of the other {@code decorrelateRel} methods match. */
Expand Down Expand Up @@ -1432,7 +1438,6 @@ private RelNode projectJoinOutputWithNullability(
pair.left,
projectPulledAboveLeftCorrelator,
nullIndicator);

newProjExprs.add(newProjExpr, pair.right);
}

Expand Down
27 changes: 27 additions & 0 deletions core/src/test/resources/sql/lateral.iq
Original file line number Diff line number Diff line change
Expand Up @@ -219,4 +219,31 @@ from t,

!ok

# LEFT JOIN LATERAL
select ename, deptno, ename2
from emp as o
left join lateral (select ename as ename2
from emp
where deptno = o.deptno + 10) on true
where job = 'MANAGER';
+-------+--------+--------+
| ENAME | DEPTNO | ENAME2 |
+-------+--------+--------+
| BLAKE | 30 | |
| CLARK | 10 | ADAMS |
| CLARK | 10 | FORD |
| CLARK | 10 | JONES |
| CLARK | 10 | SCOTT |
| CLARK | 10 | SMITH |
| JONES | 20 | ALLEN |
| JONES | 20 | BLAKE |
| JONES | 20 | JAMES |
| JONES | 20 | MARTIN |
| JONES | 20 | TURNER |
| JONES | 20 | WARD |
+-------+--------+--------+
(12 rows)

!ok

# End lateral.iq
51 changes: 51 additions & 0 deletions core/src/test/resources/sql/measure.iq
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,57 @@ from empm;

!ok

# [CALCITE-6559] Query with measure that applies AVG to SMALLINT
# throws AssertionError "Cannot add expression of different type to set"
#
# Occurs when AVG measure is applied to SMALLINT or TINYINT column.
# A CAST is added internally to the result of SUM / COUNT, but this cast
# prevents a cast to nullable for the null-generating side of an outer join.
WITH empm AS (
SELECT *, AVG(empno) AS MEASURE avg_empno
FROM emp)
SELECT job, avg_empno
FROM empm
GROUP BY job;
+-----------+-----------+
| JOB | AVG_EMPNO |
+-----------+-----------+
| ANALYST | 7845 |
| CLERK | 7769 |
| MANAGER | 7682 |
| PRESIDENT | 7839 |
| SALESMAN | 7629 |
+-----------+-----------+
(5 rows)

!ok

!if (false) {
# [CALCITE-6564] Support queries with measures on top of a VALUES relation
#
# The following querey is the original test case for [CALCITE-6559]; that
# bug is fixed, but this query now fails during planning due to VALUES.
WITH tbl_with_null_dim AS (
SELECT e.deptno, e.grade, AVG(e.grade) AS MEASURE avg_grade
FROM (VALUES (1, 70),
(1, 50),
(NULL, 50),
(3, 82)) AS e (deptno, grade))
SELECT deptno, avg_grade
FROM tbl_with_null_dim
GROUP BY deptno;
+--------+-----------+
| DEPTNO | AVG_GRADE |
+--------+-----------+
| 1 | 60 |
| 3 | 82 |
| | 50 |
+--------+-----------+
(3 rows)

!ok
!}

# CTE with literal measure
with emp2 as (
select *, 2 as measure two
Expand Down
62 changes: 62 additions & 0 deletions core/src/test/resources/sql/scalar.iq
Original file line number Diff line number Diff line change
Expand Up @@ -239,4 +239,66 @@ select deptno, (select empno from "scott".emp order by empno limit 1) as x from

!ok

# Scalar subquery on VALUES; based on a query mentioned in
# [CALCITE-3244] RelDecorrelator unable to decorrelate expression with filter and aggregate on top
select
(select count(t2.id)
from (values (1), (2)) t2(id)
where t2.id = t1.id) as c
from (values (1), (2)) t1(id);
+---+
| C |
+---+
| 1 |
| 1 |
+---+
(2 rows)

!ok

select
(select count(t2.id)
from (values (1), (2), (2), (4)) t2(id)
where t2.id = t1.id) as c
from (values (1), (3)) t1(id);
+---+
| C |
+---+
| 1 |
| 0 |
+---+
(2 rows)

!ok

# Scalar subquery using EXISTS on VALUES
select exists (select count(t2.id)
from (values (1), (2), (2), (4)) t2(id)
where t2.id = t1.id) as e
from (values (1), (3)) t1(id);
+------+
| E |
+------+
| true |
| true |
+------+
(2 rows)

!ok

# Variation on previous query
select exists (select *
from (values (1), (2), (2), (4)) t2(id)
where t2.id = t1.id) as e
from (values (1), (3)) t1(id);
+-------+
| E |
+-------+
| false |
| true |
+-------+
(2 rows)

!ok

# End scalar.iq

0 comments on commit ded0651

Please sign in to comment.