expr: recover pushdown for day-only interval predicates#36706
Draft
antiguru wants to merge 1 commit into
Draft
Conversation
51e86ed to
fa54894
Compare
8650bc0 to
939e4e2
Compare
fa54894 to
eb53ce5
Compare
The previous commits in this stack marked add/sub_timestamp_interval (and the tz variants) as non-monotone in both arguments to preserve correctness in the presence of calendar-month / day-clamping arithmetic. But that conservative annotation costs pushdown for common temporal-filter predicates like 't_col - INTERVAL ''1'' day < literal', which were a motivating use case for filter pushdown in the first place. Recovers that pushdown by extending SpecialBinary with a DynamicMonotone handler: when the interval argument is a literal with months == 0, the operation reduces to adding a fixed number of microseconds, which is monotone in both arguments. In that case the interpreter computes a tight output range; in all other cases it falls back to anything(), preserving the soundness fix. The SpecialBinary's pushdownable hint is set to (true, false) so the Trace pass routes 't_col +/- INTERVAL_lit' predicates through pushdown regardless of months — at runtime the dynamic check decides whether to narrow.
939e4e2 to
6b3a33e
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on top of #36702.
Motivation
The parent PR (#36702) fixes the persist filter-pushdown audit panic by marking
add/sub_timestamp{,_tz}_intervalas non-monotone in both arguments. That's the right correctness answer (calendar-month arithmetic with day-clamping breaks monotonicity for month-bearing intervals), but it's too conservative for the common case of literal day-only intervals — predicates liket_col - INTERVAL '1' day < literalwere a motivating use case for filter pushdown and lose theirpushdown=line inEXPLAIN.Description
Extends
SpecialBinarywith a second handler variant:The
DynamicMonotonehandler forAdd/SubTimestamp{,Tz}Intervalinspects the right argument'sResultSpecand returns(true, true)if it is a single-value interval withmonths == 0, else(false, false). The standardflat_mapmachinery then either narrows precisely or falls back toanything(). Since the staticis_monotoneannotation isn't consulted when aSpecialBinaryis present, the parent PR's(false, false)annotation is still in effect for any code path that bypasses the special handler — so soundness is preserved.For
Trace(which can't see actual values), theSpecialBinary::pushdownableis set to(true, false). This makest_col +/- INTERVAL_litTrace-pushdownable regardless of whether the interval has months; at runtime,ColumnSpecseither narrows or hands backanything()based on the dynamic check, so we may attempt pushdown for a month-bearing literal but the predicate will be sound (it just won't narrow anything for that case).Also restores the
pushdown=lines intest/sqllogictest/filter-pushdown.sltthat the parent PR had to drop.Verification
interpret::tests::test_timestamp_plus_interval_dynamic_monotonecovers three scenarios:t_col - INTERVAL '1' day < litover a range that includes both matching and non-matching values → interpreter admits bothTrueandFalse(tight narrowing).True(impossible matches eliminated).t_col - INTERVAL '1' month < lit(month-bearing) → interpreter admits both outcomes (conservative fallback; soundness preserved).cargo test -p mz-expr --lib— all 52 tests pass.test/sqllogictest/filter-pushdown.sltupdated to reflect thatpushdown=is restored fort - INTERVAL '1' daypredicates.Known limitations
The recovery only kicks in when the interval is a constant with statically-zero months. Day-only column-valued intervals don't currently get pushdown — that's a smaller win to leave for later.
Generated by Claude Code