mirror of
https://github.com/GreptimeTeam/greptimedb.git
synced 2026-09-06 13:38:59 +00:00
Ning Sun
6f1dd0cb76
* feat: allow widening the time index column's timestamp unit via ALTER TABLE ... MODIFY COLUMN Previously MODIFY COLUMN rejected the time index column outright. Now the time index unit can be widened (Second -> Milli -> Micro -> Nano), which is lossless for data that fits the target unit: historical data in old SSTs is cast to the new unit on read by the existing schema-compat layer, and compaction rewrites it lazily. Narrowing and non-timestamp targets remain rejected; tag columns keep being rejected. Widening is rejected if any SST's time range would overflow the target unit's i64 range (e.g. millisecond -> nanosecond beyond year 2262), since the cast would silently null those values. Read-path correctness for old-unit SSTs (verified by new engine e2e tests and sqlness WHERE queries): - row-group min/max pruning: parquet statistics of a timestamp column are raw integers in the file's unit; when the region metadata's type differs (also the case for altered field columns), stats are now interpreted in the file's type and converted to the expected type before pruning. Without this, a new-unit predicate silently pruned whole row groups of old-unit files (wrong results, rows missing). - SST-level simple filters are skipped for columns whose file type differs from the expected type; the predicate is applied by the query layer's residual filter above the region scan. Also drop the stale "timestamp columns cannot change type" debug_assert. - retry idempotency: a same-type ModifyColumnType on the time index validates as a no-op and `need_alter` returns false, so a retried alter procedure (region already altered before the previous attempt failed) converges instead of aborting forever. - add TimeUnit ordering and ConcreteDataType::is_timestamp_unit_widening_to - relax ModifyColumnType validation in store-api and table metadata - tests: unit tests in datatypes/store-api/table; mito2 engine e2e tests (flushed SST + new writes + reopen + retry + overflow + predicate scans, cross-unit dedup, mixed-unit SSTs, compaction); sqlness cases incl. partitioned table and WHERE filters over old-unit data Signed-off-by: Ning Sun <sunning@greptime.com> * fix: resolve parquet filter issue * test: provide sqlness tests * refactor: drop trivial test cases and shorten comments Review pass over the branch's additions: - datatypes: keep a representative subset of the widening-matrix asserts - table: collapse the three single-branch rejection blocks into one loop - mito2: drop the boundary gt_eq and post-compaction predicate asserts (covered by the exact-filter regression test and sqlness); drop the engine-level gt_eq/lt_eq casts (full operator matrix stays in the cast_timestamp_unit unit tests) - sqlness: drop a bare full scan already covered by the filter above it - shorten function doc comments across datatypes/store-api/table/mito2/ recordbatch to the essential semantics Signed-off-by: Ning Sun <sunning@greptime.com> * fix: drop physical prefilter for columns whose file type differs Follow-up to the review feedback on CompatBatch/prune reader/filter handling for widened time index units. Between/InList/IsNull predicates are prefiltered by PhysicalFilterContext, which builds its physical expression against the FILE's schema while the predicate literals are in the expected (post-alter) unit. Evaluating them against an old-unit SST raised a cross-unit comparison error (Timestamp(ms) >= Timestamp(µs)) that failed the whole scan. Physical prefilter predicates are best-effort pruning hints (the query layer re-applies them above the scan), so drop the prefilter when the column's file type differs from the expected type, mirroring the simple-filter strategy. Verified: Between and a non-rewritten (large) InList on old-unit data no longer error and filter exactly end-to-end (sqlness), and no matching rows are lost at the engine level (engine test). Signed-off-by: Ning Sun <sunning@greptime.com> * test: add direct unit tests for stats cast and prefilter drop The two-step stats cast (reinterpret raw Int64 stats in the file's timestamp type, then rescale to the expected type) and the physical prefilter drop on file/expected type mismatch were only covered end-to-end; add localized unit tests so a regression fails at the exact site: - stats.rs (previously no tests): RowGroupPruningStats min/max over a hand-built RowGroupMetaData — passthrough with no expected metadata, passthrough on same type, and rescale (1000ms -> 1_000_000us, not 1000us) on a widened expected unit - reader.rs: PhysicalFilterContext::new_opt keeps a Between prefilter when file and expected types match and drops it on unit mismatch Signed-off-by: Ning Sun <sunning@greptime.com> * fix: tolerate mixed time units range cache key coverage check * docs: flag mixed-unit hazard in the (unwired) series index The series index stores per-series min/max ts as raw Int64 in the unit of the region metadata at write time, and the searcher builds its range predicates from a single per-region metadata. After a time index unit widen, files of one region would carry mixed units, so a per-file unit (or an index rebuild on such alters) is required before this index is wired into scans. Leave notes at both sites. Signed-off-by: Ning Sun <sunning@greptime.com> * test: cover mixed-unit compaction for sparse encoding and strict windows Compaction-path audit follow-up. The compat cast and window math were already covered for dense regions; add the two remaining e2e scenarios: - sparse primary key encoding (used by metric-engine physical regions): widening then compacting mixed-unit files rewrites the old-unit time index correctly through the sparse compaction compat path - strict-window manual compaction: each window output trims rows with a predicate built in the region's new unit against an old-unit file; every instant must survive exactly once (no loss, no cross-window duplication), rescaled Also documents the audit finding that Regular ranged (manual) compaction never trims rows: TwcsPicker sets output_time_range to None and the request time range only selects candidate windows. Signed-off-by: Ning Sun <sunning@greptime.com> * test: cover time index unit change in FlatCompatBatch directly The compat layer's rescaling of a widened time index was only verified end-to-end; add direct unit tests for both paths: - dense: identical units skip compat entirely; a widened unit rescales the time index column (1000ms -> 1_000_000us, not reinterpreted) while other columns pass through and the output schema matches the expected metadata - compact sparse (the metric-engine compaction path): same rescaling Signed-off-by: Ning Sun <sunning@greptime.com> * refactor: move timestamp unit division into common-time The exact unit division (UnitQuotient + div_mod_units) is time semantics, not filter logic; move it next to TimeUnit in common-time with a compact test covering representable/non-representable values, negative (floor) instants, and quotient overflow. The ScalarValue helpers stay in filter.rs since common-time has no datafusion dependency. Signed-off-by: Ning Sun <sunning@greptime.com> * test: compat rescales a widened time index and fills an added column together The realistic multi-alter sequence (widen at T1, add column at T2, read a T0 SST) exercises cast and default-fill in the same compute_index_and_fields pass; assert both in one output batch. Signed-off-by: Ning Sun <sunning@greptime.com> * fix: preflight time index widening overflow before any region alters Address review feedback on the overflow guard: - preflight: when the frontend operator receives a widening alter on the time index, run an existence scan (ts outside the target unit's i64 range, LIMIT 1, via the query engine so it covers every region of the table in both standalone and distributed modes) BEFORE any DDL task is submitted. A region that fits can no longer commit the new schema while another region rejects the alter with a non-retryable error. File and row-group pruning keep the scan cheap when nothing overflows. The per-region check in mito2 stays as the final guard for data written after the preflight (the remaining race window); without a validate-only wire field (region.proto lives in the external greptime-proto repo) a fully atomic two-phase validate/commit is out of scope here. - fast path: cast_timestamp_unit returns the filter unchanged when the literal is already in the target unit, skipping the div-mod rebuild. Signed-off-by: Ning Sun <sunning@greptime.com> * fix: address review comments * fix: address auto review comments * fix: remove time index widening overflow preflight Overflow needs timestamps beyond the target unit's i64 range (~year 2262 for nanoseconds), which real workloads never write, so the two existence scans before every widening alter are not worth the cost. Region validation already rejects the alter when an SST's time range overflows the target unit; it now logs the rejection (with the offending file) and returns a deterministic client-facing message. Signed-off-by: Ning Sun <sunning@greptime.com> * test: make sqlness test stable * fix: log instead of rejecting time index widening overflow Overflowing values cast to NULL on read but do not otherwise affect reads or writes, so the alter is allowed; the region-level check now only logs (with the offending file) when an SST's time range exceeds the target unit's i64 range. Signed-off-by: Ning Sun <sunning@greptime.com> --------- Signed-off-by: Ning Sun <sunning@greptime.com>