normalizeIntegralOffset in WindowBoundConverter widens a ROWS offset to i64 only when integralValue(offset) is present, so a decimal, floating-point or interval literal takes the value.isEmpty() early return above that branch and is emitted with its own type. Per algebra.proto at spec v0.102.0, a BOUNDS_TYPE_ROWS offset_expr "must evaluate to a non-negative number of rows and must have type int64", which the converter's own comment on that branch cites.
Measured on aacec8db against the TPC-H ORDERS schema, with partition by O_CUSTKEY order by O_ORDERDATE:
rows between 5.0 preceding and current row
-> lowerBound=Preceding{DecimalLiteral{precision=2, scale=1}} (unscaled 50, i.e. 5.0)
rows between 5.00 preceding and current row
-> lowerBound=Preceding{DecimalLiteral{precision=3, scale=2}} (unscaled 500, i.e. 5.00)
rows between interval '-1' day preceding and current row
-> lowerBound=Preceding{IntervalDayLiteral{days=-1, seconds=0, subseconds=0, precision=6}}
rows between 5 preceding and current row (control)
-> lowerBound=Preceding{I64Literal{5}}
Calcite accepts all three. Its ROWS guard rejects !isExact(), stripTrailingZeros().scale() > 0 and a negative value, so 5.0 passes because it strips to scale 0 — and the guard does not apply to interval literals at all, which is why the third case arrives carrying a negative distance as well, hitting #1315 at the same time. All three round-trip through proto unchallenged.
Worth noting that #1309 changes one SQL-reachable case here in passing: its new zero check runs above the isRows branch, so rows between 0.0 preceding now yields CurrentRow where it previously yielded Preceding{decimal<2,1> 0.0}. That is the right outcome, but it means the ROWS path is already partly normalized for non-integral literals while the int64 requirement stays unenforced.
The fix is to decide what a non-integral ROWS offset means before widening: a value with a zero effective scale can be converted to i64 exactly, and anything else (a genuine fraction, or an interval) is not a row count and should be rejected with UnsupportedOperationException, matching how an integral RANGE offset that does not fit the ordering type is handled today.
#1198 tracks the core-side validation gap for the RANGE half of the same spec paragraph, and #1230 the isthmus non-integral RANGE retype.
🤖 Generated with AI
normalizeIntegralOffsetinWindowBoundConverterwidens a ROWS offset toi64only whenintegralValue(offset)is present, so a decimal, floating-point or interval literal takes thevalue.isEmpty()early return above that branch and is emitted with its own type. Peralgebra.protoat spec v0.102.0, aBOUNDS_TYPE_ROWSoffset_expr"must evaluate to a non-negative number of rows and must have typeint64", which the converter's own comment on that branch cites.Measured on
aacec8dbagainst the TPC-HORDERSschema, withpartition by O_CUSTKEY order by O_ORDERDATE:Calcite accepts all three. Its ROWS guard rejects
!isExact(),stripTrailingZeros().scale() > 0and a negative value, so5.0passes because it strips to scale 0 — and the guard does not apply to interval literals at all, which is why the third case arrives carrying a negative distance as well, hitting #1315 at the same time. All three round-trip through proto unchallenged.Worth noting that #1309 changes one SQL-reachable case here in passing: its new zero check runs above the
isRowsbranch, sorows between 0.0 precedingnow yieldsCurrentRowwhere it previously yieldedPreceding{decimal<2,1> 0.0}. That is the right outcome, but it means the ROWS path is already partly normalized for non-integral literals while the int64 requirement stays unenforced.The fix is to decide what a non-integral ROWS offset means before widening: a value with a zero effective scale can be converted to
i64exactly, and anything else (a genuine fraction, or an interval) is not a row count and should be rejected withUnsupportedOperationException, matching how an integral RANGE offset that does not fit the ordering type is handled today.#1198 tracks the core-side validation gap for the RANGE half of the same spec paragraph, and #1230 the isthmus non-integral RANGE retype.
🤖 Generated with AI