From 0b49cf4f97f5414cab6dee405b850579fea0806c Mon Sep 17 00:00:00 2001 From: discord9 <55937128+discord9@users.noreply.github.com> Date: Thu, 10 Sep 2026 16:50:00 +0800 Subject: [PATCH] feat: allow native precision instant LastRow selection Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com> --- src/query/src/optimizer/scan_hint.rs | 56 +++++++++++++++---- .../common/promql/native_time_selection.sql | 54 ++++++++++++++++-- 2 files changed, 94 insertions(+), 16 deletions(-) diff --git a/src/query/src/optimizer/scan_hint.rs b/src/query/src/optimizer/scan_hint.rs index ce1009f881..75cf0a520a 100644 --- a/src/query/src/optimizer/scan_hint.rs +++ b/src/query/src/optimizer/scan_hint.rs @@ -19,7 +19,6 @@ use arrow_schema::SortOptions; use common_function::aggrs::aggr_wrapper::aggr_state_func_name; use common_recordbatch::OrderOption; use common_recordbatch::filter::SimpleFilterEvaluator; -use common_time::timestamp::TimeUnit; use datafusion::datasource::DefaultTableSource; use datafusion_common::tree_node::{Transformed, TreeNodeRewriter}; use datafusion_common::{Column, Result}; @@ -139,8 +138,7 @@ impl ScanHintRule { /// predicate later rejects that row. Only recognized tag/time predicates are /// allowed: tags select whole series, and supported time predicates constrain /// the scan window before row selection. Field or unrecognized predicates are - /// conservatively rejected. Finer-than-millisecond timestamps are also excluded - /// because instant evaluation can conflate distinct samples at that precision. + /// conservatively rejected. /// /// This checks only attached predicates; the path allowlist separately rejects /// residual Filter nodes between InstantManipulate and the scan. @@ -149,14 +147,6 @@ impl ScanHintRule { provider: &DummyTableProvider, ) -> bool { let metadata = provider.region_metadata(); - // Instant evaluation is millisecond-based, so finer time units can - // conflate timestamps and must not use the LastRow hint. - if !matches!( - metadata.time_index_type().unit(), - TimeUnit::Second | TimeUnit::Millisecond - ) { - return false; - } for filter in &table_scan.filters { let Some(filter) = SimpleFilterEvaluator::try_new(filter) else { return false; @@ -1023,7 +1013,49 @@ mod test { } #[test] - fn single_evaluation_rejects_microsecond_and_nanosecond_time_index_casts() { + fn single_evaluation_uses_last_row_for_microsecond_and_nanosecond_time_indexes() { + for timestamp_type in [ + ConcreteDataType::timestamp_microsecond_datatype(), + ConcreteDataType::timestamp_nanosecond_datatype(), + ] { + let direct_provider = Arc::new(mock_table_provider_with_timestamp( + RegionId::new(1, 1), + timestamp_type.clone(), + )); + let direct = ScanHintRule + .rewrite( + single_evaluation(scan_plan(direct_provider, "direct")), + &OptimizerContext::default(), + ) + .unwrap() + .data; + assert_eq!( + scan_requests(&direct)[0].series_row_selector, + Some(TimeSeriesRowSelector::LastRow { after_merge: true }) + ); + + let projection_provider = Arc::new(mock_table_provider_with_timestamp( + RegionId::new(1, 1), + timestamp_type, + )); + let projection = LogicalPlanBuilder::from(scan_plan(projection_provider, "projection")) + .project(vec![col("ts")]) + .unwrap() + .build() + .unwrap(); + let projected = ScanHintRule + .rewrite(single_evaluation(projection), &OptimizerContext::default()) + .unwrap() + .data; + assert_eq!( + scan_requests(&projected)[0].series_row_selector, + Some(TimeSeriesRowSelector::LastRow { after_merge: true }) + ); + } + } + + #[test] + fn single_evaluation_rejects_lossy_microsecond_and_nanosecond_time_index_casts() { for timestamp_type in [ ConcreteDataType::timestamp_microsecond_datatype(), ConcreteDataType::timestamp_nanosecond_datatype(), diff --git a/tests/cases/standalone/common/promql/native_time_selection.sql b/tests/cases/standalone/common/promql/native_time_selection.sql index d6ec7d3d13..a206cd8dc6 100644 --- a/tests/cases/standalone/common/promql/native_time_selection.sql +++ b/tests/cases/standalone/common/promql/native_time_selection.sql @@ -27,13 +27,36 @@ INSERT INTO native_time_us VALUES (1000000, 'window', 3), (1000001, 'window', 4); +-- The native projection and exact zero-lookback bounds must reach the scan; +-- the 1s+tick row must not displace 201. +-- SQLNESS REPLACE (RoundRobinBatch.*) REDACTED +-- SQLNESS REPLACE (peers.*) REDACTED +-- SQLNESS REPLACE (Hash.*) REDACTED +-- SQLNESS REPLACE (RepartitionExec:.*) RepartitionExec: REDACTED +-- SQLNESS REPLACE native_time_us.__table_id\s*=\s*UInt32\(\d+\) native_time_us.__table_id=UInt32(REDACTED) +TQL EXPLAIN (1, 1, '1s', '0s') native_time_us{series="exact"}; + +-- The actual memtable scan must use LastRow { after_merge: true } with native +-- zero-lookback bounds; it must select exact 1s rather than the future tick. +-- SQLNESS REPLACE (RoundRobinBatch.*) REDACTED +-- SQLNESS REPLACE (Hash.*) REDACTED +-- SQLNESS REPLACE (-+) - +-- SQLNESS REPLACE (\s\s+) _ +-- SQLNESS REPLACE (peers.*) REDACTED +-- SQLNESS REPLACE region=\d+\(\d+,\s+\d+\) region=REDACTED +-- SQLNESS REPLACE (flat_format.*) REDACTED +TQL ANALYZE VERBOSE (1, 1, '1s', '0s') native_time_us{series="exact"}; + +-- The same-series future tick is in the memtable, while exact 1s remains selected. +TQL EVAL (1, 1, '1s', '0s') native_time_us{series="exact"}; + -- Future-only selection is empty before flushing, exercising the memtable path. TQL EVAL (1, 1, '1s', '300s') native_time_us{series="future"}; ADMIN FLUSH_TABLE('native_time_us'); --- At 1s, selection keeps an exact native timestamp. -TQL EVAL (1, 1, '1s', '300s') native_time_us{series="exact"}; +-- The exact native sample remains selected from the flushed SST. +TQL EVAL (1, 1, '1s', '0s') native_time_us{series="exact"}; TQL EVAL (1, 1, '1s', '300s') timestamp(native_time_us{series="future"}); TQL EVAL (1, 1, '1s', '300s') timestamp(native_time_us{series="exact"}); @@ -89,13 +112,36 @@ INSERT INTO native_time_ns VALUES (1000000000, 'window', 3), (1000000001, 'window', 4); +-- The native projection and exact zero-lookback bounds must reach the scan; +-- the 1s+tick row must not displace 201. +-- SQLNESS REPLACE (RoundRobinBatch.*) REDACTED +-- SQLNESS REPLACE (peers.*) REDACTED +-- SQLNESS REPLACE (Hash.*) REDACTED +-- SQLNESS REPLACE (RepartitionExec:.*) RepartitionExec: REDACTED +-- SQLNESS REPLACE native_time_ns.__table_id\s*=\s*UInt32\(\d+\) native_time_ns.__table_id=UInt32(REDACTED) +TQL EXPLAIN (1, 1, '1s', '0s') native_time_ns{series="exact"}; + +-- The actual memtable scan must use LastRow { after_merge: true } with native +-- zero-lookback bounds; it must select exact 1s rather than the future tick. +-- SQLNESS REPLACE (RoundRobinBatch.*) REDACTED +-- SQLNESS REPLACE (Hash.*) REDACTED +-- SQLNESS REPLACE (-+) - +-- SQLNESS REPLACE (\s\s+) _ +-- SQLNESS REPLACE (peers.*) REDACTED +-- SQLNESS REPLACE region=\d+\(\d+,\s+\d+\) region=REDACTED +-- SQLNESS REPLACE (flat_format.*) REDACTED +TQL ANALYZE VERBOSE (1, 1, '1s', '0s') native_time_ns{series="exact"}; + +-- The same-series future tick is in the memtable, while exact 1s remains selected. +TQL EVAL (1, 1, '1s', '0s') native_time_ns{series="exact"}; + -- Future-only selection is empty before flushing, exercising the memtable path. TQL EVAL (1, 1, '1s', '300s') native_time_ns{series="future"}; ADMIN FLUSH_TABLE('native_time_ns'); --- At 1s, selection keeps an exact native timestamp. -TQL EVAL (1, 1, '1s', '300s') native_time_ns{series="exact"}; +-- The exact native sample remains selected from the flushed SST. +TQL EVAL (1, 1, '1s', '0s') native_time_ns{series="exact"}; TQL EVAL (1, 1, '1s', '300s') timestamp(native_time_ns{series="future"}); TQL EVAL (1, 1, '1s', '300s') timestamp(native_time_ns{series="exact"});