Skip to content

Specialize macro list folds #929

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.

Sign up for GitHub

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

Merged
snazy merged 3 commits into from
Jul 22, 2026
Merged

Specialize macro list folds #929

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5b27615
Specialize macro list folds
snazy Jul 16, 2026
142501f
review / static
snazy Jul 22, 2026
e9e2fb9
fixes
snazy Jul 22, 2026
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
168 changes: 168 additions & 0 deletions core/src/main/java/org/projectnessie/cel/interpreter/Interpretable.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,18 @@
import static org.projectnessie.cel.interpreter.Coster.Cost.estimateCost;
import static org.projectnessie.cel.interpreter.Coster.costOf;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import org.projectnessie.cel.common.operators.Operator;
import org.projectnessie.cel.common.types.Err;
import org.projectnessie.cel.common.types.IterableT;
import org.projectnessie.cel.common.types.IteratorT;
import org.projectnessie.cel.common.types.ListT;
import org.projectnessie.cel.common.types.MapT;
import org.projectnessie.cel.common.types.Overloads;
import org.projectnessie.cel.common.types.StringT;
Expand All @@ -49,6 +52,7 @@
import org.projectnessie.cel.common.types.traits.FieldTester;
import org.projectnessie.cel.common.types.traits.Negater;
import org.projectnessie.cel.common.types.traits.Receiver;
import org.projectnessie.cel.common.types.traits.Sizer;
import org.projectnessie.cel.common.types.traits.Trait;
import org.projectnessie.cel.interpreter.Activation.VarActivation;
import org.projectnessie.cel.interpreter.AttributeFactory.Attribute;
Expand Down Expand Up @@ -1108,6 +1112,111 @@ public String toString() {
}
}

final class EvalListFold extends AbstractEval implements Coster {
Comment thread
XN137 marked this conversation as resolved.
final String iterVar;
final Interpretable iterRange;
final Interpretable filter;
final Interpretable transform;
private final TypeAdapter adapter;

EvalListFold(
long id,
String iterVar,
Interpretable iterRange,
Interpretable filter,
Interpretable transform,
TypeAdapter adapter) {
super(id);
this.iterVar = iterVar;
this.iterRange = iterRange;
this.filter = filter;
this.transform = transform;
this.adapter = adapter;
}

@Override
public Val eval(org.projectnessie.cel.interpreter.Activation ctx) {
Val foldRange = iterRange.eval(ctx);
if (!foldRange.type().hasTrait(Trait.IterableType)) {
return valOrErr(
foldRange, "got '%s', expected iterable type", foldRange.getClass().getName());
}

VarActivation iterCtx = new VarActivation();
iterCtx.parent = ctx;
iterCtx.name = iterVar;
List<Val> values = new ArrayList<>(listCapacity(foldRange));
IteratorT it = ((IterableT) foldRange).iterator();
while (it.hasNext() == True) {
iterCtx.val = it.next();

if (filter != null) {
Val include = filter.eval(iterCtx);
if (include == False) {
continue;
}
if (include != True) {
return noSuchOverload(null, Operator.Conditional.id, include);
}
}

Val value = transform.eval(iterCtx);
if (isUnknownOrError(value)) {
return value;
}
values.add(value);
}
return ListT.newValArrayList(adapter, values.toArray(new Val[0]));
}

private int listCapacity(Val foldRange) {
if (foldRange.type().hasTrait(Trait.SizerType)) {
long size = ((Sizer) foldRange).size().intValue();
if (size > 0 && size <= Integer.MAX_VALUE) {
return (int) size;
}
}
return 0;
}

@Override
public Cost cost() {
Cost range = estimateCost(iterRange);
Cost result = estimateCost(transform);
if (filter != null) {
result = result.add(estimateCost(filter));
}
Val foldRange = iterRange.eval(emptyActivation());
if (!foldRange.type().hasTrait(Trait.IterableType)) {
return Cost.Unknown;
}
long rangeCnt = 0L;
IteratorT it = ((IterableT) foldRange).iterator();
while (it.hasNext() == True) {
it.next();
rangeCnt++;

@XN137 XN137 Jul 22, 2026

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.

should this try to use Sizer first or instead (as we do in listCapacity) ?

@snazy snazy Jul 22, 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.

Nice catch! I already have a follow-up for that.

}
return range.add(result.multiply(rangeCnt));
}

@Override
public String toString() {
return "EvalListFold{"
+ "id="
+ id
+ ", iterVar='"
+ iterVar
+ '\''
+ ", iterRange="
+ iterRange
+ ", filter="
+ filter
+ ", transform="
+ transform
+ '}';
}
}

// Optional Intepretable implementations that specialize, subsume, or extend the core evaluation
// plan via decorators.

Expand Down Expand Up @@ -1677,6 +1786,65 @@ public String toString() {
}
}

/** EvalExhaustiveListFold evaluates every filter and transform without short-circuiting. */
final class EvalExhaustiveListFold extends AbstractEval implements Coster {
private final EvalListFold fold;

EvalExhaustiveListFold(EvalListFold fold) {
super(fold.id);
this.fold = fold;
}

@Override
public Val eval(org.projectnessie.cel.interpreter.Activation ctx) {
Val foldRange = fold.iterRange.eval(ctx);
if (!foldRange.type().hasTrait(Trait.IterableType)) {
return valOrErr(
foldRange, "got '%s', expected iterable type", foldRange.getClass().getName());
}

VarActivation iterCtx = new VarActivation();
iterCtx.parent = ctx;
iterCtx.name = fold.iterVar;
List<Val> values = new ArrayList<>(fold.listCapacity(foldRange));
Val result = null;
IteratorT it = ((IterableT) foldRange).iterator();
while (it.hasNext() == True) {
iterCtx.val = it.next();

Val include = fold.filter != null ? fold.filter.eval(iterCtx) : True;
Val value = fold.transform.eval(iterCtx);
if (include == False) {
continue;
}
if (include != True) {
result = noSuchOverload(null, Operator.Conditional.id, include);

@XN137 XN137 Jul 22, 2026

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.

minor sonnet finding:

  EvalExhaustiveListFold.eval() handles errors inconsistently depending on their source.

  Filter errors overwrite result unconditionally:
  if (include != True) {
      result = noSuchOverload(null, Operator.Conditional.id, include);  // always overwrites
      continue;
  }

  Transform errors only set result if it's still null:
  if (result == null) {
      if (isUnknownOrError(value)) {
          result = value;  // only captures the first
      }
  }

  The consequence: if a transform error occurs on element 1, then a filter error occurs on element 2, the transform error is silently replaced by the filter error. Conversely, multiple transform errors always return the first one, while
  multiple filter errors return the last one. There's also no test covering the filter-returns-error path in exhaustive mode at all (both existing exhaustive tests use include == False or a transform that errors, not a filter that returns
  a non-boolean).

  The fix would be to guard the filter-error assignment with the same result == null check:
  if (include != True) {
      if (result == null) {
          result = noSuchOverload(null, Operator.Conditional.id, include);
      }
      continue;
  }

  This is a low-severity issue since exhaustive mode is primarily for cost estimation and observability rather than production evaluation paths, and erroring filter conditions are rare. But the inconsistency is real and would be easy to
  fix.

@snazy snazy Jul 22, 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.

Good catch! I'll keep it in mind!

continue;
}
if (result == null) {
if (isUnknownOrError(value)) {
result = value;
} else {
values.add(value);
}
}
}
return result != null
? result
: ListT.newValArrayList(fold.adapter, values.toArray(new Val[0]));
}

@Override
public Cost cost() {
return fold.cost();
}

@Override
public String toString() {
return "EvalExhaustiveListFold{" + fold + '}';
}
}

/** evalAttr evaluates an Attribute value. */
final class EvalAttr extends AbstractEval
implements InterpretableAttribute, Coster, Qualifier, Attribute {
Expand Down
5 changes: 5 additions & 0 deletions core/src/main/java/org/projectnessie/cel/interpreter/InterpretableDecorator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,11 @@
import org.projectnessie.cel.interpreter.Interpretable.EvalExhaustiveAnd;
import org.projectnessie.cel.interpreter.Interpretable.EvalExhaustiveConditional;
import org.projectnessie.cel.interpreter.Interpretable.EvalExhaustiveFold;
import org.projectnessie.cel.interpreter.Interpretable.EvalExhaustiveListFold;
import org.projectnessie.cel.interpreter.Interpretable.EvalExhaustiveOr;
import org.projectnessie.cel.interpreter.Interpretable.EvalFold;
import org.projectnessie.cel.interpreter.Interpretable.EvalList;
import org.projectnessie.cel.interpreter.Interpretable.EvalListFold;
import org.projectnessie.cel.interpreter.Interpretable.EvalMap;
import org.projectnessie.cel.interpreter.Interpretable.EvalOr;
import org.projectnessie.cel.interpreter.Interpretable.EvalSetMembership;
Expand Down Expand Up @@ -107,6 +109,9 @@ static InterpretableDecorator decDisableShortcircuits() {
expr.step,
expr.result);
}
if (i instanceof EvalListFold) {
return new EvalExhaustiveListFold((EvalListFold) i);
}
if (i instanceof InterpretableAttribute) {
InterpretableAttribute expr = (InterpretableAttribute) i;
if (expr.attr() instanceof ConditionalAttribute) {
Expand Down
Loading
Loading