Skip to content

Comments

Handle unknown and disabled filters inline in AstFilterChain #1284

Merged
hs-lsong merged 5 commits intomasterfrom
local-dt-test
Feb 20, 2026
Merged

Handle unknown and disabled filters inline in AstFilterChain #1284
hs-lsong merged 5 commits intomasterfrom
local-dt-test

Conversation

@hs-lsong
Copy link
Collaborator

@hs-lsong hs-lsong commented Feb 5, 2026

Fall back to un-optimized filter chain for unknown filters

Summary

  • Handle unknown and disabled filters inline in AstFilterChain by setting value to null and continuing the chain, matching the non-chained EL evaluation behavior
  • Add parity tests confirming optimized and non-chained paths produce identical output for both disabled and unknown filters

Why

The optimized AstFilterChain previously handled unknown and disabled filters differently from the standard EL evaluation path. Unknown filters triggered a fallback to the unoptimized AST at parse time, and disabled filters returned null
immediately, aborting the entire chain. In the non-chained path, both cases propagate null through subsequent filters. This change makes the optimized path match that behavior exactly, removing the need for the fallback.

When AstFilterChain optimization is enabled and a filter chain contains
an unknown filter (e.g. local_dt), fall back to the standard nested
AstMethod evaluation at parse time instead of failing with an
"Unknown filter" error and returning null.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@hs-lsong hs-lsong marked this pull request as ready for review February 5, 2026 21:45
return null;
}
if (filter == null) {
interpreter.addError(
Copy link
Contributor

Choose a reason for hiding this comment

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

This does seem more complicated than it needs to be, and doesn't look to handle disabled filters the same as the non-chained logic.

I think for both instances they will make value null. Could that work instead of building the custom error?

value = null;
continue;

hs-lsong and others added 4 commits February 18, 2026 14:02
…ehavior

The non-chained path in JinjavaInterpreterResolver skips disabled filters
and passes the value through to the next filter. The chained path was
incorrectly returning null for the entire expression, discarding the input
and aborting remaining filters.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The previous fix used `continue` alone, but the non-chained path
propagates null through subsequent filters rather than skipping.
Setting value = null before continuing matches that behavior exactly.

Adds parity test confirming optimized and non-chained paths produce
identical output for disabled filters in a chain.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Unknown filters are now handled the same as disabled filters in
AstFilterChain: set value to null and continue. This matches the
non-chained behavior where null propagates through subsequent filters.

The parse-time hasUnknownFilter check and buildUnoptimizedFromSpecs
fallback are no longer needed and are removed.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@hs-lsong hs-lsong changed the title Fall back to un-optimized filter chain for unknown filters Handle unknown and disabled filters inline in AstFilterChain Feb 18, 2026
@hs-lsong hs-lsong requested a review from jasmith-hs February 18, 2026 23:03
@hs-lsong hs-lsong merged commit 0e5510e into master Feb 20, 2026
7 checks passed
@hs-lsong hs-lsong deleted the local-dt-test branch February 20, 2026 14:06
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