-
Notifications
You must be signed in to change notification settings - Fork 95
feat: add lambda expression support #710
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
5ca6ff8
ed61127
a90c609
4053357
ed5de56
eb7b2b2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -37,6 +37,7 @@ public class ProtoExpressionConverter { | |
| private final Type.Struct rootType; | ||
| private final ProtoTypeConverter protoTypeConverter; | ||
| private final ProtoRelConverter protoRelConverter; | ||
| private final LambdaParameterStack lambdaParameterStack = new LambdaParameterStack(); | ||
|
|
||
| public ProtoExpressionConverter( | ||
| ExtensionLookup lookup, | ||
|
|
@@ -75,6 +76,19 @@ public FieldReference from(io.substrait.proto.Expression.FieldReference referenc | |
| reference.getDirectReference().getStructField().getField(), | ||
| rootType, | ||
| reference.getOuterReference().getStepsOut()); | ||
| case LAMBDA_PARAMETER_REFERENCE: | ||
| { | ||
| io.substrait.proto.Expression.FieldReference.LambdaParameterReference lambdaParamRef = | ||
| reference.getLambdaParameterReference(); | ||
|
|
||
| int stepsOut = lambdaParamRef.getStepsOut(); | ||
| Type.Struct lambdaParameters = lambdaParameterStack.get(stepsOut); | ||
|
|
||
| return FieldReference.newLambdaParameterReference( | ||
| reference.getDirectReference().getStructField().getField(), | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Something I'm curious about is why we use I'm unfamiliar with substrait-java, but does this current implementation support nested field access through lambda parameters? *This may be an unimportant issue especially if it's not common to access nested fields in lambdas
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. because the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, I see. I am still confused as to whether this supports nested field access through lambda parameters. I messed around w some tests through claude, and I believe it is unsupported. However, this is a nit and I'm not sure it will come up often in practice, so we can come back to this later if it ends up being an issue! |
||
| lambdaParameters, | ||
| stepsOut); | ||
| } | ||
| case ROOTTYPE_NOT_SET: | ||
| default: | ||
| throw new IllegalArgumentException("Unhandled type: " + reference.getRootTypeCase()); | ||
|
|
@@ -260,6 +274,27 @@ public Type visit(Type.Struct type) throws RuntimeException { | |
| } | ||
| } | ||
|
|
||
| case LAMBDA: | ||
| { | ||
| io.substrait.proto.Expression.Lambda protoLambda = expr.getLambda(); | ||
| Type.Struct parameters = | ||
| (Type.Struct) | ||
| protoTypeConverter.from( | ||
| io.substrait.proto.Type.newBuilder() | ||
| .setStruct(protoLambda.getParameters()) | ||
| .build()); | ||
|
|
||
| lambdaParameterStack.push(parameters); | ||
|
|
||
| Expression body; | ||
| try { | ||
| body = from(protoLambda.getBody()); | ||
| } finally { | ||
| lambdaParameterStack.pop(); | ||
| } | ||
|
|
||
| return Expression.Lambda.builder().parameters(parameters).body(body).build(); | ||
| } | ||
| // TODO enum. | ||
| case ENUM: | ||
| throw new UnsupportedOperationException("Unsupported type: " + expr.getRexTypeCase()); | ||
|
|
@@ -574,4 +609,42 @@ public Expression.SortField fromSortField(SortField s) { | |
| public static FunctionOption fromFunctionOption(io.substrait.proto.FunctionOption o) { | ||
| return FunctionOption.builder().name(o.getName()).addAllValues(o.getPreferenceList()).build(); | ||
| } | ||
|
|
||
| /** | ||
| * A stack for tracking lambda parameter types during expression parsing. | ||
| * | ||
| * <p>When parsing nested lambda expressions, each lambda's parameters are pushed onto this stack. | ||
| * Lambda parameter references use "stepsOut" to indicate which enclosing lambda they reference: | ||
| * | ||
| * <ul> | ||
| * <li>stepsOut=0 refers to the innermost (current) lambda | ||
| * <li>stepsOut=1 refers to the next enclosing lambda | ||
| * <li>stepsOut=N refers to N levels up | ||
| * </ul> | ||
| */ | ||
| private static class LambdaParameterStack { | ||
limameml marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| private final List<Type.Struct> stack = new ArrayList<>(); | ||
|
|
||
| void push(Type.Struct parameters) { | ||
| stack.add(parameters); | ||
| } | ||
|
|
||
| void pop() { | ||
| if (stack.isEmpty()) { | ||
| throw new IllegalArgumentException("Lambda parameter stack is empty"); | ||
| } | ||
| stack.remove(stack.size() - 1); | ||
| } | ||
|
|
||
| Type.Struct get(int stepsOut) { | ||
| int index = stack.size() - 1 - stepsOut; | ||
| if (index < 0 || index >= stack.size()) { | ||
| throw new IllegalArgumentException( | ||
| String.format( | ||
| "Lambda parameter reference with stepsOut=%d is invalid (current depth: %d)", | ||
| stepsOut, stack.size())); | ||
| } | ||
| return stack.get(index); | ||
| } | ||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.