Skip to content

Specialize macro list folds#929

Merged
snazy merged 3 commits into
projectnessie:mainfrom
snazy:mac-li-fold
Jul 22, 2026
Merged

Specialize macro list folds#929
snazy merged 3 commits into
projectnessie:mainfrom
snazy:mac-li-fold

Conversation

@snazy

@snazy snazy commented Jul 21, 2026

Copy link
Copy Markdown
Member

Recognize map and filter macro fold shapes during planning and build result lists with a mutable evaluation-local buffer.

This avoids repeated accumulator list copying while preserving generic fold fallback for non-standard accumulator references.

Recognize map and filter macro fold shapes during planning and build result lists with a mutable evaluation-local buffer.

This avoids repeated accumulator list copying while preserving generic fold fallback for non-standard accumulator references.
@snazy
snazy requested a review from XN137 July 21, 2026 08:01
XN137
XN137 previously approved these changes Jul 22, 2026
IteratorT it = ((IterableT) foldRange).iterator();
while (it.hasNext() == True) {
it.next();
rangeCnt++;

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) ?

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.

Comment thread core/src/main/java/org/projectnessie/cel/interpreter/InterpretablePlanner.java Outdated
Comment thread core/src/main/java/org/projectnessie/cel/interpreter/InterpretablePlanner.java Outdated
continue;
}
if (include != True) {
result = noSuchOverload(null, Operator.Conditional.id, include);

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.

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!

@snazy
snazy merged commit 887ff30 into projectnessie:main Jul 22, 2026
3 checks passed
@snazy
snazy deleted the mac-li-fold branch July 22, 2026 13:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants