fix: time_bucket() silently dropped from projections#18
Merged
farhan-syah merged 2 commits intomainfrom Apr 9, 2026
Merged
Conversation
…t detection When a query uses GROUP BY with an alias (e.g. GROUP BY b) or a positional ordinal (e.g. GROUP BY 1), the planner was failing to recognize the underlying time_bucket() call, causing it to be misclassified as a plain group column. Extract resolve_group_by_expr() to look up the originating SELECT expression for alias and ordinal references, and try_extract_time_bucket() to isolate the time_bucket detection logic. The GROUP BY loop now resolves before checking, so all three forms — direct, alias, ordinal — correctly set the bucket interval.
…jections Add a computed_columns field to the timeseries physical plan node, allowing scalar projection expressions (e.g. time_bucket) to be evaluated per-row during raw scan execution. - Propagate computed_columns bytes through the bridge plan, dispatch, and all plan construction sites (scan, aggregate, auto_tier, alert, native plan builder) - Evaluate computed columns in the Data Plane raw scan handler by converting rmpv rows to nodedb_types::Value, applying expressions, and re-encoding - Implement eval_time_bucket in the expression evaluator with support for both argument orders and integer-second intervals - Replace the local parse_interval_to_ms implementation in nodedb-sql with a delegation to nodedb_types::kv_parsing::parse_interval_to_ms - Refactor extract_bucket_interval to scan all arguments for the interval literal, enabling time_bucket with timestamp-first argument order - Refactor raw_scan signature to use a RawScanParams struct
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
1 participant
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.
Summary
Fixes #17 —
time_bucket()was silently dropped from projections after the nodedb-sql migration. All variants (alias, ordinal, swapped args, integer seconds, bare projection) now work correctly.Root causes:
time_bucket()when it appeared literally asast::Expr::Functionin GROUP BY. SQLGROUP BY aliasandGROUP BY 1were treated as plain column names, sobucket_interval_msstayed 0 and the Data Plane returned empty results.extract_bucket_intervalonly handledtime_bucket('interval', col)— not swapped arg ordertime_bucket(col, 'interval')or integer secondstime_bucket(3600, col).SELECT time_bucket(...) FROM twas silently dropped because the Timeseries scan path had no computed column support andtime_bucketwasn't registered as an evaluable scalar function.Changes:
resolve_group_by_expr()to resolve GROUP BY aliases/ordinals to their SELECT expressions before checking fortime_bucket. Fixedextract_bucket_intervalto handle both arg orders and integer-seconds form. Consolidated duplicateparse_interval_to_msto use canonicalnodedb_types::kv_parsing.time_bucketas an evaluable scalar function indatetime.rs. Consolidated duplicateparse_interval_to_ms.computed_columnsfield toTimeseriesOp::Scan. AddedRawScanParamsstruct. Applied computed column evaluation in raw scan path viaapply_computed_columns_rmpv().point.rs.Test plan
GROUP BY alias,GROUP BY 1,GROUP BY time_bucket(...)all resolve correctlytime_bucket(timestamp, '1 hour')workstime_bucket(3600, timestamp)worksSELECT time_bucket(...) FROM t LIMIT 3returns bucketed timestamps (not SELECT *)COUNT(*)baseline unaffected (100000)Closes #17