mengw15 opened a new issue, #6901:
URL: https://github.com/apache/texera/issues/6901

   ### Task Summary
   
   Two pure `workflow-operator` classes have small, sharply-defined coverage 
gaps that their existing specs miss: `AggregationOperation` 
(`common/workflow-operator/.../aggregate/AggregationOperation.scala`, ~81%) and 
`IntervalJoinOpExec` 
(`common/workflow-operator/.../intervalJoin/IntervalJoinOpExec.scala`, ~84%). 
Both are dependency-free in-memory logic — no DB, no storage, no clock — so the 
remaining branches are cheap to pin. Extend the existing 
`AggregationOperationSpec` and `IntervalOpExecSpec` rather than adding new 
files.
   
   ### Behavior to add
   
   **AggregationOperation** (`AggregationOperationSpec`, currently 7 blocks)
   - `getNumericalValue`, TIMESTAMP branch — AVERAGE over a TIMESTAMP column is 
the only route in and nothing exercises it. Two timestamps an hour apart should 
average to their epoch-millis midpoint.
   - `getAggregationAttribute`'s unknown-function branch — throws 
`RuntimeException`, distinct from the `UnsupportedOperationException` that 
`getAggFunc` throws and that is already tested. Reach it with an unset 
`aggFunction` and assert on the message.
   - `countAgg`'s COUNT(\*) vs COUNT(column) split — with a column containing 
nulls, `attribute = ""` (count-all) must yield a higher count than naming the 
column.
   - `averageAgg` with a null field — a null row must not inflate the divisor.
   - `minAgg`/`maxAgg` empty-input finalisers — see below.
   
   **IntervalJoinOpExec** (`IntervalOpExecSpec`, currently 14 blocks)
   - The `case None` fallback of `desc.timeIntervalType` (the `plusDays` 
default). Every existing test goes through the 6-argument `IntervalJoinOpDesc` 
constructor, which forces `Some(...)`; the field is declared `var 
timeIntervalType: Option[TimeIntervalType] = _`, i.e. initialised to `null` 
rather than `None`. Build a TIMESTAMP desc with `timeIntervalType` left unset, 
serialize it, and assert day semantics — whether Jackson materialises `None` or 
`null` here is exactly the unobvious part worth pinning.
   - `processNumValue`'s "below the left bound" returns for the exclusive-bound 
variants; existing tests mostly probe the in-range and above-range outcomes.
   - The multi-element drain in `removeTooSmallTupleInRightCache` / 
`removeTooSmallTupleInLeftCache` — feed three stale right-table rows followed 
by one far-future left row and assert the output size proves all three were 
evicted in a single call.
   
   Keep TIMESTAMP fixtures away from Feb 29 and DST boundaries; the interval 
arithmetic goes through `plusYears`/`plusMonths` calendar math.
   
   ### A defect the AggregationOperation work will surface
   
   `maxAgg` seeds its accumulator with 
`AttributeTypeUtils.minValue(attributeType)` but its finaliser collapses to 
`null` when the partial equals 
`AttributeTypeUtils.**max**Value(attributeType)`. `minAgg` is consistent — it 
seeds `maxValue` and finalises against `maxValue`. So `maxAgg` has it both ways 
round: MAX over an empty input returns the type's *minimum* instead of `null`, 
and a legitimate MAX result that happens to equal the type's maximum is wrongly 
collapsed to `null`. Fix it in the same PR (#6878 is the precedent for 
fix-plus-tests) or pin current behavior and file it separately — but do not 
encode the current result as if it were intended.
   
   ### Task Type
   
   - [ ] Refactor / Cleanup
   - [ ] DevOps / Deployment / CI
   - [x] Testing / QA
   - [ ] Documentation
   - [ ] Performance
   - [ ] Other
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to