Skip to content
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,8 @@
import java.util.Objects;
import java.util.Set;
import java.util.TreeSet;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.function.BiFunction;
import java.util.function.Consumer;
import java.util.function.Supplier;
Expand Down Expand Up @@ -5599,6 +5601,11 @@
// First pass, ensure that aliases are unique. "*" and "TABLE.*" items
// are ignored.

// Rewrite scalar sub-queries whose single select item is an aggregate

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could this be done after validating all arguments?
Then you would not need to call deriveType early.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, and this is the root cause behind the first question you asked this time.
The sub-query's SELECT scope is already established during the registration phase (registerSubQueries for the SELECT list), so the early deriveType calls were redundant. I've removed them; getSelectScope(subQuery) works directly.

// over outer columns. The aggregate belongs to the outer query per SQL
// standard.
rewriteOuterAggregatesInSelectList(select);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

call this handleOuterAggregate, and make the function reject it if not in a suitable conformance mode.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the latest commit implements controls in this area.


// Validate SELECT list. Expand terms of the form "*" or "TABLE.*".
final SqlValidatorScope selectScope = getSelectScope(select);
final List<SqlNode> expandedSelectItems = new ArrayList<>();
Expand Down Expand Up @@ -5658,6 +5665,269 @@
return typeFactory.createStructType(fieldList);
}

/**
* Rewrites scalar sub-queries in the SELECT list whose single select item is
* an aggregate function whose arguments reference only outer columns. Per the
* SQL standard, such aggregates belong to the outer query.
*
* <p>The algorithm is:
* <ol>
* <li>For each item in the SELECT list, check whether it is a scalar
* sub-query, optionally wrapped in {@code AS} or {@code WITH}.
* <li>Inside the sub-query, require the SELECT list to contain exactly one
* item, and that item to be an aggregate function call.
* <li>Require every argument of the aggregate to reference only columns from
* the outer query (no inner columns), and to contain no nested sub-queries.
* <li>If all conditions hold, lift the aggregate out of the sub-query: keep
* the aggregate in the outer SELECT list, and replace the original sub-query
* with {@code (SELECT 1 FROM ... LIMIT 1)}. The result is equivalent because
* the scalar sub-query contributes a factor of one per outer row, while the
* aggregate is evaluated over the outer rows.
* <li>If the outer query becomes an aggregate query as a result, upgrade its
* SELECT clause scope from {@link SelectScope} to
* {@link AggregatingSelectScope}.
* </ol>
*
* <p>For example,
* <blockquote><pre>SELECT (SELECT sum(a) FROM t LIMIT 1) FROM aa</pre></blockquote>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We cannot read this example because we don't know where column a is.
Can you also add tests where one or both tables are empty or contain only nulls in the aggregated column?

@xuzifu666 xuzifu666 Jun 27, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, The Javadoc examples for rewriteOuterAggregatesInSelectList have been improved, explicitly stating that a comes from aa and x comes from t.
Boundary tests have been added to agg.iq: Inner table empty → 1 row NULL
Outer table empty → 1 row NULL (because the outer aggregation has no input rows)
and postgresql test also updated (result is also keep the same with agg.iq, empty results for VALUE are not currently supported, so I am using the SQL statement SELECT 1 FROM emp WHERE 1 = 0 as a substitute.) https://onecompiler.com/postgresql/44tgmwq6p

* is rewritten to
* <blockquote><pre>SELECT sum(a) * (SELECT 1 FROM t LIMIT 1) FROM aa</pre></blockquote>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From this example it's not obvious to me at all what the general rewrite rule is.
Can you describe the algorithm?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I’ve added the logic for the overall process; I hope that clarifies things.

*/
private void rewriteOuterAggregatesInSelectList(SqlSelect select) {
final SqlNodeList selectItems = select.getSelectList();
if (selectItems == null) {
return;
}
final SqlValidatorScope selectScope = getSelectScope(select);
final boolean wasAggregate = isAggregate(select);
for (int i = 0; i < selectItems.size(); i++) {
final SqlNode selectItem = selectItems.get(i);
final SqlNode rewrittenItem =
rewriteOuterAggregateItem(selectScope, selectItem);
if (rewrittenItem != selectItem) {
selectItems.set(i, rewrittenItem);
}
}
// The rewrite may have introduced an aggregate into a query that was not
// previously aggregate. Update the SELECT clause scope accordingly so that
// subsequent validation and conversion see an AggregatingSelectScope.
if (!wasAggregate && isAggregate(select)) {
SqlValidatorScope scope =
clauseScopes.get(IdPair.of(select, Clause.SELECT));
if (scope instanceof AggregatingSelectScope) {
scope = ((AggregatingSelectScope) scope).getParent();
}
clauseScopes.put(IdPair.of(select, Clause.SELECT),
new AggregatingSelectScope(
requireNonNull(scope, "scope"), select, false));
}
}

/**
* Rewrites a single SELECT list item if it is a scalar sub-query containing
* an aggregate over outer columns.
*
* @return the rewritten expression, or {@code selectItem} if no rewrite applies
*/
private SqlNode rewriteOuterAggregateItem(SqlValidatorScope parentScope,
SqlNode selectItem) {
// Unwrap AS.
SqlNode expr = selectItem;
@Nullable SqlIdentifier alias = null;
if (SqlUtil.isCallTo(selectItem, SqlStdOperatorTable.AS)) {
SqlCall asCall = (SqlCall) selectItem;
expr = asCall.operand(0);
alias = asCall.operand(1);
}
if (!SqlUtil.isCallTo(expr, SqlStdOperatorTable.SCALAR_QUERY)) {
return selectItem;
}
final SqlBasicCall scalarSubQuery = (SqlBasicCall) expr;
final SqlNode query = scalarSubQuery.operand(0);
final SqlSelect subQuery = query instanceof SqlWith
? (SqlSelect) ((SqlWith) query).body
: query instanceof SqlSelect
? (SqlSelect) query
: null;

Check warning on line 5751 in core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Extract this nested ternary operation into an independent statement.

See more on https://sonarcloud.io/project/issues?id=apache_calcite&issues=AZ8E5Gg8VhkxC5OQBjm6&open=AZ8E5Gg8VhkxC5OQBjm6&pullRequest=5052
if (subQuery == null) {
return selectItem;
}
// Validate the sub-query first so that its scope is established.
deriveType(parentScope, scalarSubQuery);
final SqlNode rewrittenSubQuery =
rewriteOuterAggregate(query, subQuery, parentScope);
if (rewrittenSubQuery == query) {
return selectItem;
}
SqlNode result = rewrittenSubQuery;
if (alias != null) {
result =
SqlStdOperatorTable.AS.createCall(selectItem.getParserPosition(),
rewrittenSubQuery, alias);
}
return result;
}

/**
* Rewrites a scalar sub-query whose single select item is an aggregate
* function whose arguments reference only outer columns. The aggregate is
* pulled out to the enclosing query, and the sub-query's select item is
* replaced with the constant 1.
*
* @return the rewritten sub-query expression, or {@code subQuery} if no rewrite applies
*/
private SqlNode rewriteOuterAggregate(SqlNode originalQuery,
SqlSelect subQuery, SqlValidatorScope parentScope) {
final SqlNodeList subSelectItems = SqlNonNullableAccessors.getSelectList(subQuery);
if (subSelectItems.size() != 1) {
return originalQuery;
}
if (subQuery.getGroup() != null && !subQuery.getGroup().isEmpty()) {
return originalQuery;
}
SqlNode subSelectItem = subSelectItems.get(0);
if (SqlUtil.isCallTo(subSelectItem, SqlStdOperatorTable.AS)) {
subSelectItem = ((SqlCall) subSelectItem).operand(0);
}
if (!(subSelectItem instanceof SqlCall)) {
return originalQuery;
}
final SqlCall aggCall = (SqlCall) subSelectItem;
if (!aggCall.getOperator().isAggregator()) {
return originalQuery;
}
final SqlValidatorScope subScope = getSelectScope(subQuery);
if (subScope == null) {
return originalQuery;
}
final SqlValidatorScope operandScope;
if (subScope instanceof AggregatingSelectScope) {
operandScope = ((AggregatingSelectScope) subScope).parent;
} else {
operandScope = subScope;
}
if (operandScope == null) {
return originalQuery;
}
for (SqlNode operand : aggCall.getOperandList()) {
if (!referencesOnlyOuterColumns(operand, parentScope, operandScope)) {
return originalQuery;
}
}
// Rewrite: replace aggregate with 1 in a new sub-query, and multiply
// by the aggregate in the outer query.
final SqlLiteral one = SqlLiteral.createExactNumeric("1", SqlParserPos.ZERO);
final SqlNodeList newSubSelectItems =
new SqlNodeList(ImmutableList.of(one), subSelectItems.getParserPosition());
final SqlNodeList keywordList = (SqlNodeList) subQuery.getOperandList().get(0);
final SqlSelect newSubQuery =
new SqlSelect(subQuery.getParserPosition(),
keywordList,
newSubSelectItems,
subQuery.getFrom(),
subQuery.getWhere(),
subQuery.getGroup(),
subQuery.getHaving(),
subQuery.getWindowList(),
subQuery.getQualify(),
subQuery.getOrderList(),
subQuery.getOffset(),
subQuery.getFetch(),
subQuery.getHints(),
subQuery.getDistinctOn());
// Ensure the rewritten sub-query still returns at most one row, because
// removing the aggregate may otherwise produce multiple rows.
if (newSubQuery.getFetch() == null) {
newSubQuery.setFetch(
SqlLiteral.createExactNumeric("1", SqlParserPos.ZERO));
}
final SqlNode newQuery = originalQuery instanceof SqlWith
? new SqlWith(originalQuery.getParserPosition(),
((SqlWith) originalQuery).withList, newSubQuery)
: newSubQuery;
registerQuery(parentScope, null, newQuery, newQuery, null, false);
validateQuery(newQuery, parentScope, unknownType);
final SqlNode scalarSubQuery =
SqlStdOperatorTable.SCALAR_QUERY.createCall(newQuery.getParserPosition(), newQuery);
return SqlStdOperatorTable.MULTIPLY.createCall(
aggCall.getParserPosition(), aggCall, scalarSubQuery);
}

/**
* Returns whether an expression contains only references to columns that are
* outside the current select scope, and can be resolved in the parent scope.
*/
private boolean referencesOnlyOuterColumns(SqlNode node,
SqlValidatorScope parentScope, SqlValidatorScope currentScope) {
// Do not rewrite if the aggregate argument contains a sub-query; the
// sub-query may reference inner columns, and pulling the aggregate out
// would change the semantics.
if (containsSubQuery(node)) {
return false;
}
final AtomicBoolean ok = new AtomicBoolean(true);
final AtomicInteger outerColumnCount = new AtomicInteger(0);
node.accept(new SqlBasicVisitor<Void>() {
@Override public Void visit(SqlIdentifier id) {
if (!isOuterReference(currentScope, id)) {
ok.set(false);
return null;
}
// The identifier must be resolvable in the parent scope; otherwise it
// references an intermediate scope, not the immediate enclosing query.
try {
parentScope.fullyQualify(id);
outerColumnCount.incrementAndGet();
} catch (Exception e) {
ok.set(false);
}
return null;
}
});
return ok.get() && outerColumnCount.get() > 0;
}

/**
* Returns whether an expression contains a sub-query.
*/
private static boolean containsSubQuery(SqlNode node) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there no other helper function doing this in the Calcite codebase?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's RexUtil.SubQueryFinder, but it operates on the Rex layer (RexSubQuery), whereas here we're still at the SqlNode layer, so it doesn't apply. I didn't find an existing SqlNode-level "contains sub-query" helper. That said, the hand-written switch over every sub-query SqlKind is fragile (easy to miss a new kind) — I had replaced it with the predefined SqlKind.QUERY set (SqlKind.QUERY.contains(call.getKind())) instead of enumerating each case.

final AtomicBoolean found = new AtomicBoolean(false);
node.accept(new SqlBasicVisitor<Void>() {
@Override public Void visit(SqlCall call) {
switch (call.getKind()) {
case SCALAR_QUERY:
case EXISTS:
case IN:
case NOT_IN:
case SOME:
case ALL:
case MULTISET_QUERY_CONSTRUCTOR:
case ARRAY_QUERY_CONSTRUCTOR:
case MAP_QUERY_CONSTRUCTOR:
case CURSOR:
found.set(true);
return null;
default:
return super.visit(call);
}
}
});
return found.get();
}

/**
* Returns whether an identifier refers to a scope outside the current select.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure exactly what "outside" means, but given this function definition I think that the javadoc could be "in a scope which is not the supplied one". There's certainly no "current select" in sight.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes sense; I’ve made the relevant changes to the comment.

*/
private boolean isOuterReference(SqlValidatorScope scope, SqlIdentifier id) {
final SqlQualified fqId = scope.fullyQualify(id);
if (fqId.prefixLength <= 0) {
return false;
}
final SqlValidatorScope.ResolvedImpl resolved = new SqlValidatorScope.ResolvedImpl();
scope.resolve(fqId.prefix(), catalogReader.nameMatcher(), false, resolved);
return resolved.count() == 1 && !resolved.only().scope.isWithin(scope);
}

/**
* Validates an expression.
*
Expand Down
82 changes: 82 additions & 0 deletions core/src/test/resources/sql/agg.iq
Original file line number Diff line number Diff line change
Expand Up @@ -4301,4 +4301,86 @@ GROUP BY GROUPING SETS ((deptno), ());

!ok

# [CALCITE-6104] Aggregate function that references outer column should be evaluated in outer query

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a comment that these are validated using an independent database.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we also have a test with 2 outer aggregates?
Showing the plan could also help.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the reminder, I had added comment about it and the test case had been added.
But agg.iq to verify the correctness of the numerical results calculated after the Calcite fix within PostgreSQL, the original SQL must be rewritten into standard SQL supported by PostgreSQL.
Validated these sql in https://onecompiler.com/postgresql/44tgmwq6p and result is as expected.

# The expected results below have been verified in PostgreSQL by running

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am confused, I thought these queries are standard SQL, but, indeed, I cannot run them on postgres or other databases I tried. Isn't this PR supposed to implement a standard SQL feature? Why can't these queries use standard SQL then?

@xuzifu666 xuzifu666 Jun 27, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this is indeed a confusing point. These correlated-aggregate queries are not standard SQL, and CALCITE-6104 should not implement a SQL standard feature. Instead, it makes Calcite support a non-standard extension that some databases already implement.

The SQL standard does not allow aggregate functions to reference outer columns
In standard SQL, an aggregate function like sum(a) can only reference columns from the SELECT level where it appears. So a query like:

SELECT (SELECT sum(a) FROM xx LIMIT 1) FROM aa;

will fail on PostgreSQL, Oracle, and MySQL/MariaDB because a belongs to the outer table aa, not the sub-query xx.

But PostgreSQL and others support this extension.
These databases allow an aggregate to "climb up" to the nearest SELECT that contains its free variables. Calcite previously handled this pattern incorrectly (it aggregated inside the sub-query, grouped by the outer rows). The fix makes Calcite behave consistently with those databases.
This test is provided in Jira (julianhyde@3f4cab5) is supported—at least with the current PR applied; these are not standard SQL either.
Additionally, I tested converting these SQL statements to standard SQL; they passed even without this PR, which is why I hold the view mentioned above.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the clarification. Maybe this SQL should not be accepted by default? There are these conformance modes, I think only modes like LENIENT or BABEL should accept this construct.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You say that postgres accepts these queries, then why do you have to modify the queries to run them on postgres? I couldn't run them as written in Postgres.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, thanks for pointing this out. The current comment is incorrect, should state "These SQL queries are non-standard and their successful execution is only guaranteed within Calcite for now" makes perfect sense.

Regarding the limitation where correlated aggregate rewriting is only available under modes like LENIENT or BABEL, could log a separate jira to resolve this later(If ok I would create a new jira)? This way, we can track these changes independently and easily gather any additional feedback. From my perspective, this limitation bears little relevance to the current Jira ticket. I’d like to hear if you agree with my view on this.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this will require 3 lines of code and should be done as part of this PR.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I'll implement this restriction directly in this PR.

# equivalent standard-SQL rewrites of each query.
WITH aa (a) AS (VALUES 1, 2, 3),
xx (x) AS (VALUES 10, 20, 30)
SELECT (SELECT sum(a) FROM xx LIMIT 1) AS sa
FROM aa;
+----+
| SA |
+----+
| 6 |
+----+
(1 row)

!ok

# Aggregate with no column references aggregates at the innermost level
WITH aa (a) AS (VALUES 1, 2, 3),
xx (x) AS (VALUES 10, 20, 30)
SELECT (SELECT sum(1) FROM xx LIMIT 1) AS s
FROM aa;
+---+
| S |
+---+
| 3 |
| 3 |
| 3 |
+---+
(3 rows)

!ok

# Aggregate referencing only inner column aggregates at the inner level
WITH aa (a) AS (VALUES 1, 2, 3),
xx (x) AS (VALUES 10, 20, 30)
SELECT (SELECT sum(x) FROM xx LIMIT 1) AS s
FROM aa;
+----+
| S |
+----+
| 60 |
| 60 |
| 60 |
+----+
(3 rows)

!ok

# Two scalar sub-queries with outer-only aggregates are both lifted to the outer query
WITH aa (a) AS (VALUES 1, 2, 3),
xx (x) AS (VALUES 10, 20, 30)
SELECT (SELECT sum(a) FROM xx LIMIT 1) AS sa,
(SELECT count(a) FROM xx LIMIT 1) AS ca
FROM aa;
+----+----+
| SA | CA |
+----+----+
| 6 | 3 |
+----+----+
(1 row)

!ok

EnumerableCalc(expr#0..2=[{inputs}], expr#3=[*($t0, $t2)], expr#4=[*($t1, $t2)], SA=[$t3], CA=[$t4])
EnumerableNestedLoopJoin(condition=[true], joinType=[left])
EnumerableAggregate(group=[{}], agg#0=[SUM($0)], agg#1=[COUNT()])
EnumerableValues(tuples=[[{ 1 }, { 2 }, { 3 }]])
EnumerableLimit(fetch=[1])
EnumerableValues(tuples=[[{ 1 }, { 1 }, { 1 }]])
!plan

SELECT (SELECT sum(sal) FROM dept) AS sum_sal

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The blog post quoted by @julianhyde has more examples, please add all of the relevant ones

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I review the blog overall and add new case for it. The correspondence is as follows:

SELECT (SELECT sum(1) FROM xx LIMIT 1) FROM aa

Aggregate with no column references aggregates at the innermost level.

SELECT (SELECT sum(a) FROM xx LIMIT 1) FROM aa

Aggregate function that references outer column should be evaluated in outer query.

SELECT (SELECT sum(x) FROM xx LIMIT 1) FROM aa

Aggregate referencing only inner column aggregates at the inner level.

FROM emp;
+----------+
| SUM_SAL |
+----------+
| 29025.00 |
+----------+
(1 row)

!ok

# End agg.iq
Loading