[gupta-sahil01]

What changes were proposed in this PR?

The Filter operator hard-codes OR semantics — a tuple passes as soon as any one
predicate matches. Users who need AND (e.g. age > 30 AND country = "US") have to
chain multiple Filter operators, which adds workflow clutter and an extra hop per
condition.

This PR makes the combination configurable:

• New PredicateCombinator enum (OR → "any (OR)", AND → "all (AND)"),
  following the @JsonValue / @JsonCreator pattern already used by
  ComparisonType in the same package.
• New predicateCombinator property on SpecializedFilterOpDesc, defaulting to
  OR. The UI dropdown renders automatically from the JSON schema, so there are no
  frontend changes.
• SpecializedFilterOpExec now selects forall for AND and exists for OR.
• Docs and descriptions updated to stop claiming OR-only behavior.

Backward compatibility: the default is OR, and a stored descriptor without the
field deserializes to OR, so existing workflows behave exactly as before and no
migration is needed.

One behavioral note for reviewers: an empty predicate list continues to drop every
tuple in both modes. A bare forall returns true on an empty list, which would
have silently flipped an empty Filter from "drop everything" to "pass everything", so
the AND branch is guarded with predicates.nonEmpty &&. There is a test covering this.

Nested predicate trees (e.g. (A AND B) OR C) are intentionally out of scope — they
need more extensive UI work and are noted as a follow-up in the issue.

Any related issues, documentation, discussions?

Closes #6939

Operator documentation updated in the same PR:
docs/reference/operators/data-cleaning/filter.md.

How was this PR tested?

New and updated unit tests, all run locally via
sbt "WorkflowOperator/testOnly *filter*" — 67 tests across 7 suites, all passing.

• PredicateCombinatorSpec (new): wire names via getName, case-insensitive
  fromString, rejection of an unknown name, and the Jackson round-trip.
• SpecializedFilterOpDescSpec: the property defaults to OR; a hand-written
  descriptor JSON with no predicateCombinator field deserializes to OR
  (this is the backward-compatibility claim above); an explicit AND survives the
  round-trip through the polymorphic LogicalOp base; operatorInfo advertises the
  new behavior.
• SpecializedFilterOpExecSpec: with two predicates where only one matches, OR
  keeps the tuple and AND drops it; AND keeps it when both match; AND with an empty
  predicate list drops everything; AND and OR agree on a single predicate. A
  before block resets the shared descriptor between tests so a mode set in one
  test cannot leak into the ones after it.

The empty-list guard was additionally verified by mutation: removing
predicates.nonEmpty && from the executor makes exactly one test fail, confirming
the case is genuinely covered rather than incidentally passing.

Was this PR authored or co-authored using generative AI tooling?

Generated-by: Claude Code (Claude Opus 5)

[github-actions]

Automated Reviewer Suggestions

Based on the git blame history of the changed files, we recommend the following reviewers:

• Committers with relevant context: @parshimers
  You can request their reviews formally with /request-review @parshimers.

• Contributors with relevant context: @Yicong-Huang, @aglinxinyuan
  You can notify them by mentioning @Yicong-Huang, @aglinxinyuan in a comment.

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.94%. Comparing base (ec3a9dd) to head (8425ccd).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #8484      +/-   ##
============================================
- Coverage     93.94%   93.94%   -0.01%     
- Complexity     4827     4833       +6     
============================================
  Files          1209     1210       +1     
  Lines         49705    49717      +12     
  Branches       6074     6075       +1     
============================================
+ Hits          46695    46706      +11     
  Misses         1522     1522              
- Partials       1488     1489       +1     

Flag	Coverage Δ		*Carryforward flag
access-control-service	80.18% <ø> (ø)
agent-service	99.32% <ø> (ø)		Carriedforward from ec3a9dd
amber	89.92% <100.00%> (+<0.01%)	⬆️
computing-unit-managing-service	75.05% <ø> (ø)
config-service	87.12% <ø> (ø)
file-service	83.65% <ø> (ø)
frontend	96.77% <ø> (ø)		Carriedforward from ec3a9dd
notebook-migration-service	83.73% <ø> (ø)
pyamber	98.47% <ø> (ø)		Carriedforward from ec3a9dd
workflow-compiling-service	77.19% <ø> (ø)

*This pull request uses carry forward flags. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[github-actions]

⚠️ Benchmark changes need a look

🟢 2 better · 🔴 3 worse · ⚪ 10 noise (<±5%) · 0 without baseline

Compared against main ded7ba1 benchmarked on this same runner, so the delta is largely free of cross-runner hardware noise. The "7d avg" column still reflects the gh-pages dashboard. Treat <±5% as noise unless repeated.

Dashboard · Run

	config	throughput	MB/s	latency	max Δ latest / 7d
🔴	bs=10 sw=10 sl=64	419	0.256	24,036/31,032/31,032 us	🔴 +14.7% / 🔴 +92.3%
🟢	bs=100 sw=10 sl=64	925	0.565	105,845/138,018/138,018 us	🟢 -7.6% / 🔴 +27.0%
⚪	bs=1000 sw=10 sl=64	1,086	0.663	924,025/990,868/990,868 us	⚪ within ±5% / 🟢 -7.6%

Baseline details

Latest main ded7ba1 from same runner

config	metric	PR	latest main	7d avg	Δ latest	Δ 7d
bs=10 sw=10 sl=64	throughput	419 tuples/sec	453 tuples/sec	755.28 tuples/sec	-7.5%	-44.5%
bs=10 sw=10 sl=64	MB/s	0.256 MB/s	0.276 MB/s	0.461 MB/s	-7.2%	-44.5%
bs=10 sw=10 sl=64	p50	24,036 us	20,949 us	12,957 us	+14.7%	+85.5%
bs=10 sw=10 sl=64	p95	31,032 us	32,247 us	16,134 us	-3.8%	+92.3%
bs=10 sw=10 sl=64	p99	31,032 us	32,247 us	20,333 us	-3.8%	+52.6%
bs=100 sw=10 sl=64	throughput	925 tuples/sec	950 tuples/sec	980.1 tuples/sec	-2.6%	-5.6%
bs=100 sw=10 sl=64	MB/s	0.565 MB/s	0.58 MB/s	0.598 MB/s	-2.6%	-5.6%
bs=100 sw=10 sl=64	p50	105,845 us	103,451 us	101,894 us	+2.3%	+3.9%
bs=100 sw=10 sl=64	p95	138,018 us	149,307 us	108,718 us	-7.6%	+27.0%
bs=100 sw=10 sl=64	p99	138,018 us	149,307 us	122,482 us	-7.6%	+12.7%
bs=1000 sw=10 sl=64	throughput	1,086 tuples/sec	1,101 tuples/sec	1,011 tuples/sec	-1.4%	+7.4%
bs=1000 sw=10 sl=64	MB/s	0.663 MB/s	0.672 MB/s	0.617 MB/s	-1.3%	+7.4%
bs=1000 sw=10 sl=64	p50	924,025 us	905,258 us	996,422 us	+2.1%	-7.3%
bs=1000 sw=10 sl=64	p95	990,868 us	986,338 us	1,037,670 us	+0.5%	-4.5%
bs=1000 sw=10 sl=64	p99	990,868 us	986,338 us	1,072,152 us	+0.5%	-7.6%

Raw CSV

config_idx,batch_size,schema_width,string_len,num_batches,total_ms,total_tuples,total_bytes,tuples_per_sec,mb_per_sec,lat_p50_us,lat_p95_us,lat_p99_us
0,10,10,64,20,477.77,200,128000,419,0.256,24036.19,31031.70,31031.70
1,100,10,64,20,2161.79,2000,1280000,925,0.565,105845.27,138018.28,138018.28
2,1000,10,64,20,18413.24,20000,12800000,1086,0.663,924025.35,990867.74,990867.74

[gupta-sahil01]

/request-review @parshimers

[gupta-sahil01]

/request-review @xuang7

[gupta-sahil01]

Any updates here @aglinxinyuan ?

[aglinxinyuan]

Thanks for picking this up. The enum, the executor change and the empty-list guard look right, and defaulting to OR is the right call. Two things need fixing before this merges; smaller notes are inline.

1. After the rebase, Python export ignores AND. Since this branched, #8502 added generateStandaloneCode() to SpecializedFilterOpDesc. It joins the predicates with " | ". That file merges cleanly with your change; only SpecializedFilterOpDescSpec conflicts. So once rebased, an AND Filter still exports as OR: age > 30 AND country = "US" comes out as (…age > 30) | (…country == "US"), which keeps rows matching either predicate. Please:

   • join on the combinator:

     val joiner = if (predicateCombinator == PredicateCombinator.AND) " & " else " | "
     s"out1df = in1df[${conditions.mkString(joiner)}].reset_index(drop=True)"

   • fix the two comments that say the executor uses predicates.exists, one in the descriptor and one in its spec;
   • add a two-predicate AND export test.

   When you resolve the spec conflict, keep both sides.

2. required = true makes saved Filters invalid. See the inline comment. "No migration needed" holds for the backend, but not for the frontend validator.

Also, docs/reference/operators/data-cleaning/_index.md:42 still says Filter uses OR only.

[aglinxinyuan]

Two things here:

• required = true puts predicateCombinator in the schema's required list.
  • Workflows saved before this PR don't have that key, and nothing fills in schema defaults when a workflow loads.
  • The frontend validator then rejects every existing Filter: Run shows "Invalid Workflow" until someone opens each Filter's property panel, and Form View readers can't fix it at all.
  • Projection's isDrop and Keyword Search's isCaseSensitive went in the same way, but here it's free to avoid. Making it optional keeps the default and the preselected dropdown, like Histogram2DOpDesc's normalization property does.
• Could you declare it above predicates, as Add option to combine Filter operator predicates with AND instead of only OR #6939 proposed? Right now the dropdown renders below every predicate row and the add button, so with several predicates you can't see which mode applies.

Suggested change

	@JsonProperty(value = "predicates", required = true)
	@JsonPropertyDescription("multiple predicates in OR")
	@JsonPropertyDescription("multiple predicates to be combined")
	var predicates: List[FilterPredicate] = List.empty
	@JsonProperty(required = true, defaultValue = "any (OR)")
	@JsonSchemaTitle("Combine Predicates With")
	@JsonPropertyDescription("how to combine multiple predicates")
	var predicateCombinator: PredicateCombinator = PredicateCombinator.OR
	@JsonProperty(required = false, defaultValue = "any (OR)")
	@JsonSchemaTitle("Combine Predicates With")
	@JsonPropertyDescription("how to combine multiple predicates")
	var predicateCombinator: PredicateCombinator = PredicateCombinator.OR
	@JsonProperty(value = "predicates", required = true)
	@JsonPropertyDescription("multiple predicates to be combined")
	var predicates: List[FilterPredicate] = List.empty

[aglinxinyuan]

Nit: with case _, a future third combinator would silently run as OR. Naming OR explicitly gets a compiler warning instead, and choosing the function once avoids re-matching on every tuple. No null case is needed: getPhysicalOp serializes without nulls, so the executor always gets the OR default.

Suggested change

	setFilterFunc((tuple: Tuple) =>
	desc.predicateCombinator match {
	case PredicateCombinator.AND =>
	desc.predicates.nonEmpty && desc.predicates.forall(_.evaluate(tuple))
	case _ => desc.predicates.exists(_.evaluate(tuple))
	}
	)
	setFilterFunc(desc.predicateCombinator match {
	case PredicateCombinator.AND =>
	(tuple: Tuple) => desc.predicates.nonEmpty && desc.predicates.forall(_.evaluate(tuple))
	case PredicateCombinator.OR =>
	(tuple: Tuple) => desc.predicates.exists(_.evaluate(tuple))
	})

[aglinxinyuan]

Every AND case puts int = 0 first, and that predicate is true for nonNullTuple. So no test shows the first predicate can reject a row under AND: predicates.tail.forall(...) or predicates.last.evaluate(...) would pass the whole suite. Could you add the same pair in the other order?

  it should "drop a tuple under AND when the first predicate fails" in {
    opDesc.predicates = List(
      new FilterPredicate("string", ComparisonType.EQUAL_TO, "world"),
      new FilterPredicate("int", ComparisonType.EQUAL_TO, "0")
    )
    opDesc.predicateCombinator = PredicateCombinator.AND
    val opExec = new SpecializedFilterOpExec(objectMapper.writeValueAsString(opDesc))
    opExec.open()
    assert(opExec.processTuple(nonNullTuple, inputPort).isEmpty)
    opExec.close()
  }

Minor: line 148 does nothing, because the before hook already resets the combinator to OR.

[aglinxinyuan]

Could you also test bad input at this level? Two cases:

• "predicateCombinator": "xor" read as LogicalOp should fail with a ValueInstantiationException that names the field.
• An executor built from JSON with "predicateCombinator": null should behave as OR.

AGENTS.md's test rules ask for bad-input coverage on user input.

[aglinxinyuan]

Optional: Jackson already reads a @JsonValue enum back from its value, and most enums in this module (JoinType, for example) don't have this creator. Fine to keep if you'd rather match ComparisonType.

[aglinxinyuan]

If the property becomes optional, drop the ✓, and if it moves above Predicates, move this row up too. Also worth one line saying that "all (AND)" with no predicates keeps no rows, since most people expect "all" of nothing to keep everything.