[CALCITE-6242] Enhance lambda closure by cjj2010 · Pull Request #4926 · apache/calcite

cjj2010 · 2026-05-08T08:32:42Z

jira: https://issues.apache.org/jira/browse/CALCITE-6242

xuzifu666 · 2026-05-08T12:41:54Z

        .type("RecordType(INTEGER NOT NULL EXPR$0) NOT NULL");
    s.withSql("select HIGHER_ORDER_FUNCTION2(1, () -> 0.1)")
        .type("RecordType(INTEGER NOT NULL EXPR$0) NOT NULL");
+    s.withSql("select HIGHER_ORDER_FUNCTION(1, (x, y) -> x + 1 + ^emp.deptno^) from emp")


Please add jira message in test.

xuzifu666 · 2026-05-08T12:42:22Z


 !ok
+
+select *


xuzifu666 · 2026-05-08T12:47:26Z


  @Override public R visitLambda(RexLambda lambda, P arg) {
-    return null;
+    if (!deep) {


I think you could add some comments here to explain the reason.

done, thanks

xuzifu666

LGTM

xuzifu666 · 2026-05-11T06:22:24Z

There are some new discussions in Jira that you should keep up with.

mihaibudiu

What happens to nested lambdas? Are they legal? Will they validate?
x -> (y -> x + y)
For the inner closure "x" should not be treated like a table column name.

cjj2010 · 2026-05-18T10:26:27Z

What happens to nested lambdas? Are they legal? Will they validate? x -> (y -> x + y) For the inner closure "x" should not be treated like a table column name.

Thank you for your suggestion. Currently, nested lambdas like x -> (y -> x + y) can be parsed normally. I have added more cases to verify @mihaibudiu

mihaibudiu

This is very nice, left a couple more requests for checking some tricky corner cases.

mihaibudiu · 2026-05-18T18:02:23Z

-    checkArgument(parameterTypes.containsKey(columnName),
-        "column %s not found", columnName);
-    return parameterTypes.get(columnName);
+    if (parameterTypes.containsKey(columnName)) {


Do you know what happens to case sensitivity?
I suspect parameter names are subject to the same rules as other identifiers, including quoting.
Can you please write some tests using uppercase/lowercase parameter names and also using quoted names?

Do you know what happens to case sensitivity? I suspect parameter names are subject to the same rules as other identifiers, including quoting. Can you please write some tests using uppercase/lowercase parameter names and also using quoted names?

Thank you for reminding me that the support for case sensitivity here is indeed not rigorous enough. I have made optimizations and added test cases. I am very grateful for your professional and meticulous review of this pull request (PR)

mihaibudiu · 2026-05-18T18:03:29Z

+    // the inner lambda can resolve references to outer lambda parameters.
+    // e.g., in x -> EXISTS(arr, y -> x + y = 4), the inner lambda's blackboard
+    // needs access to "X" from the outer lambda's nameToNodeMap.
+    if (bb.nameToNodeMap != null) {


speaking of this, I suspect we should reject lambdas with the same parameter used twice x -> x -> x + 1. Please write a test about that too.

speaking of this, I suspect we should reject lambdas with the same parameter used twice x -> x -> x + 1. Please write a test about that too.

I completely agree. I have added new test cases for this

dssysolyatin · 2026-05-19T03:54:55Z

+        // nested lambdas can resolve outer lambda parameters via the
+        // scope chain, rather than treating them as table column names.
+        registerOperandSubQueries(
+            i == 1 ? lambdaScope : parentScope, call, i);


Instead of the magic i == 1 inside the for loop, can we just write: (We anyway expect shape of SqlLambda)

registerOperandSubQueries(lambdaScope, call.getExpression(), i)

Also, I don't quite get why we need registerOperandSubQueries for the parameters (operand = 0)? Do we need this loop at all ?

Oh, sorry. We can not write like that, because registerOperandSubQueries accept ordinal as parameter. But question about loop is still valid

Oh, sorry. We can not write like that, because registerOperandSubQueries accept ordinal as parameter. But question about loop is still valid

I agree, but compared to lambda, in the current scenario, flattening it into two lines would be more intuitive. Operand 0 is the parameter list, and operand 1 is the lambda body

Why do we need two lines? Do we actually need registerOperandSubQueries for the parameters, is it even possible for a parameter to be a SubQuery?

I think this could be simplified from:

for (int i = 0; i < operands.size(); i++) { registerOperandSubQueries( i == 1 ? lambdaScope : parentScope, call, i);

to just:

// To make it easier to understand code int expressionOperand = 1 registerOperandSubQueries(lambdaScope, call, expressionOperand)

?

Why do we need two lines? Do we actually need registerOperandSubQueries for the parameters, is it even possible for a parameter to be a SubQuery?

I think this could be simplified from:

for (int i = 0; i < operands.size(); i++) { registerOperandSubQueries( i == 1 ? lambdaScope : parentScope, call, i);

to just:

// To make it easier to understand code int expressionOperand = 1 registerOperandSubQueries(lambdaScope, call, expressionOperand)

?

like this:
// Register the parameter list under the parent scope
// (parameters are visible to the lambda body, not the other way around).
registerOperandSubQueries(parentScope, call, 0);
// Register the expression body under the lambda scope so that
// nested lambdas can resolve outer lambda parameters via the
// scope chain, rather than treating them as table column names.
registerOperandSubQueries(lambdaScope, call, 1);

Why do you need to call registerOperandSubQueries for parameters.
registerOperandSubQueries(parentScope, call, 0);

registerOperandSubQueries does:

Registers any sub-queries inside a given call operand, and converts the
operand to a scalar sub-query if the operator requires it.

SqlLambda.parameters is list of SqlIdentitifier. Can it ever be/contain a SubQuery? I hope not.
So, registerOperandSubQueries(parentScope, call, 0) should do nothing. It should be safe to drop it

Good catch, you're right.
SqlLambda.parameters is a SqlNodeList whose elements are always SqlIdentifier (the parameter names). Tracing through registerOperandSubQueries → registerSubQueries:
parameters.getKind() is not QUERY, so the SCALAR_QUERY wrapping branch is skipped.
In the SqlNodeList branch, each SqlIdentifier is neither a QUERY nor a SqlCall/SqlNodeList, so it falls into the atomic-node branch and is ignored.
So registerOperandSubQueries(parentScope, call, 0) is effectively a no-op on a lambda's parameter list, and dropping it is safe.I have updated the code.

dssysolyatin · 2026-05-19T06:00:15Z

+          throw newValidationError(params.get(j),
+              RESOURCE.duplicateLambdaParameter(otherName));
+        }
+      }


Can we write something like:

final Set<String> seen = catalogReader.nameMatcher().createSet(); for (SqlNode param : lambdaExpr.getParameters()) { final String name = ((SqlIdentifier) param).getSimple(); if (!seen.add(name)) { throw newValidationError(param, RESOURCE.duplicateLambdaParameter(name)); } }

Can we write something like:

final Set<String> seen = catalogReader.nameMatcher().createSet(); for (SqlNode param : lambdaExpr.getParameters()) { final String name = ((SqlIdentifier) param).getSimple(); if (!seen.add(name)) { throw newValidationError(param, RESOURCE.duplicateLambdaParameter(name)); } }

done，thanks

mihaibudiu

If @dssysolyatin does not have other questions, let's merge this.

mihaibudiu · 2026-05-19T16:35:30Z

I don't think calcite has a "call" expression, which would allow you to call a closure: (x -> x + 1)(3). That would make such closures even more useful. Maybe we should file an issue about it?

dssysolyatin · 2026-05-19T19:06:47Z

Just last question - #4926 (comment) and feel free to merge after that

sonarqubecloud · 2026-05-20T13:11:36Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
90.6% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

xuzifu666 reviewed May 8, 2026

View reviewed changes

cjj2010 changed the title ~~[CALCITE-6242] The "exists" library function throws a "param not found" error when a column is used in lambda evaluation logic.~~ [CALCITE-6242] Enhance lambda closure parsing May 9, 2026

xuzifu666 approved these changes May 9, 2026

View reviewed changes

xuzifu666 added the discussion-in-jira There's open discussion in JIRA to be resolved before proceeding with the PR label May 11, 2026

cjj2010 changed the title ~~[CALCITE-6242] Enhance lambda closure parsing~~ [CALCITE-6242] Enhance lambda closure May 12, 2026

mihaibudiu reviewed May 12, 2026

View reviewed changes

mihaibudiu reviewed May 18, 2026

View reviewed changes

cjj2010 force-pushed the CALCITE-6242 branch 2 times, most recently from 4ef11b6 to 2056190 Compare May 19, 2026 02:54

dssysolyatin reviewed May 19, 2026

View reviewed changes

mihaibudiu approved these changes May 19, 2026

View reviewed changes

mihaibudiu added the LGTM-will-merge-soon Overall PR looks OK. Only minor things left. label May 19, 2026

cjj2010 force-pushed the CALCITE-6242 branch 2 times, most recently from 4b33f31 to 5a993bf Compare May 20, 2026 04:56

dssysolyatin approved these changes May 20, 2026

View reviewed changes

[CALCITE-6242] Enhance lambda closure

d248e7e

cjj2010 force-pushed the CALCITE-6242 branch from 3af4ae8 to d248e7e Compare May 20, 2026 12:47


		!ok

		select *

Conversation

cjj2010 commented May 8, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xuzifu666 left a comment

Choose a reason for hiding this comment

Uh oh!

xuzifu666 commented May 11, 2026

Uh oh!

mihaibudiu left a comment

Choose a reason for hiding this comment

Uh oh!

cjj2010 commented May 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mihaibudiu left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dssysolyatin May 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dssysolyatin May 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dssysolyatin May 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dssysolyatin May 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mihaibudiu left a comment

Choose a reason for hiding this comment

Uh oh!

mihaibudiu commented May 19, 2026

Uh oh!

dssysolyatin commented May 19, 2026

Uh oh!

sonarqubecloud Bot commented May 20, 2026

Quality Gate passed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

cjj2010 commented May 18, 2026 •

edited

Loading

dssysolyatin May 19, 2026 •

edited

Loading

dssysolyatin May 19, 2026 •

edited

Loading

dssysolyatin May 20, 2026 •

edited

Loading

dssysolyatin May 19, 2026 •

edited

Loading