Skip to content

feat: Implement date_part scalar function - #27005

Open
devanbenz wants to merge 123 commits into
master-1.xfrom
db/76/date_part
Open

feat: Implement date_part scalar function #27005
devanbenz wants to merge 123 commits into
master-1.xfrom
db/76/date_part

Conversation

@devanbenz

@devanbenz devanbenz commented Dec 3, 2025

Copy link
Copy Markdown

Implements date_part(part, expression), which extracts a component from a timestamp.

Signature:

  • Exactly 2 args, in order: date_part('<part>', time)
    • arg 1: string literal naming the part (case-insensitive)
    • arg 2: the time VarRef, nothing else
  • Returns int64, evaluated in the query timezone (tz(...)), default UTC

Parts:

part value
year calendar year
quarter quarter of year, [1, 4]
month month of year, [1, 12]
week ISO-8601 week of year, [1, 53]
day day of month, [1, 31]
hour / minute / second [0,23] / [0,59] / [0,59]
millisecond / microsecond / nanosecond seconds-of-minute scaled to the unit plus the sub-second component, e.g. 45.123s returns 45123 for millisecond
dow day of week, Sunday = 0 to Saturday = 6
isodow ISO-8601 day of week, Monday = 1 to Sunday = 7
doy day of year, [1, 366]
epoch seconds since Unix epoch (whole seconds)

week is the ISO week and year is the calendar year, so the two can disagree at
year boundaries. For example 2023-01-01 returns week 52.

Examples:

-- weekdays only
SELECT * FROM some_measurement
WHERE time >= now() - 10d AND time <= now()
  AND date_part('dow', time) != 0 AND date_part('dow', time) != 6

SELECT value, date_part('hour', time) FROM some_measurement

SELECT rules

  • Must be paired with an anchor, meaning a stored field or a non-date_part
    aggregate or selector. date_part-only selects are rejected.
  • Multiple date_part fields and aliases are allowed, and may nest in expressions
    such as date_part('hour', time) + 1.

GROUP BY date_part rules

  • Allowed alongside time(). Other calls in GROUP BY are rejected.
  • Requires exactly one non-date_part aggregate or selector in the SELECT list.
    Raw selects and multiple aggregates are rejected.
  • Duplicate parts are deduplicated.
  • A SELECTed date_part('part', time) must match a grouped part. A non-grouped
    part is rejected because it is undefined for the bucket. A non-active grouped
    part yields null in that series.
  • Output column is named after the canonical part such as year. A field or alias
    colliding with it is rejected.
  • Resolved from the bucket value, not the row timestamp.
  • fill(none) is always supported. fill(null) (the default) is supported for a
    bare GROUP BY date_part but rejected when combined with a time() interval; use
    fill(none). fill(previous), fill(linear), and fill(<value>) are rejected.

Subqueries

  • date_part is supported in the WHERE clause and in GROUP BY of a query over a
    subquery source, computed from the timestamps the subquery emits. tz(...) is
    honored.
  • The anchor rule applies through subqueries: the innermost statement must select
    a stored field or a non-date_part aggregate or selector.

See #27001 for 1.x limitations.

@devanbenz devanbenz self-assigned this Dec 4, 2025
@devanbenz devanbenz linked an issue Dec 8, 2025 that may be closed by this pull request
@devanbenz
devanbenz marked this pull request as ready for review December 9, 2025 21:50
Comment thread query/cursor.go Outdated
Comment thread query/date_part.go Outdated
Comment thread query/date_part.go Outdated

const (
DatePartString = "date_part"
DatePartTimeString = "date_part_time"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is date_part_time? I don't see any tests for it.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's used to create a reference to time since time is an auxiliary field https://github.com/influxdata/influxdb/pull/27005/files#diff-609a7e16be956ed6386e1a4a4efadf600b7d4de7dcfea27330dc692d1e901dc8R930-R944 I'm going to create some ValueMapper tests for this.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gwossum I can add tests for this but it would likely require exporting

type valueMapper struct {
and testing it. We don't currently have any valueMapper specific tests. It's basically just a struct filled with maps so we would likely just be testing go's map functionality, which may not be worth the effort?

@davidby-influx davidby-influx left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some changes from the first pass. Will review again after changes.

Comment thread query/compile_test.go
Comment thread query/date_part.go
Comment thread query/date_part.go Outdated
Comment thread query/date_part.go Outdated
Comment thread query/date_part.go Outdated
Comment thread tsdb/engine/tsm1/iterator.gen.go Outdated
Comment thread tsdb/engine/tsm1/iterator.gen.go Outdated
Comment thread tsdb/engine/tsm1/iterator.gen.go Outdated
Comment thread tsdb/engine/tsm1/iterator.gen.go Outdated
Comment thread tsdb/engine/tsm1/iterator.gen.go.tmpl Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 29 out of 31 changed files in this pull request and generated no new comments.

Files not reviewed (1)
  • query/internal/internal.pb.go: Generated file

@davidby-influx davidby-influx left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. query/subquery.go:133 — user-triggerable process panic / DoS · CONFIRMED

mapAuxField now returns a datePartMap for any GROUP BY date_part dimension name, including the subquery's aggregate driver. But NewIteratorMapper's driver switch only handles FieldMap/TagMap and panics on the default case.

  • Trigger: SELECT count(year) FROM (SELECT year FROM cpu) GROUP BY date_part('year', time) — a subquery whose aggregate argument is named after a date part.
  • Impact: panic: unable to create iterator mapper with driver expression type: query.datePartMap (iterator_mapper.go:84), fired inside an errgroup goroutine spawned by buildCursor — the handler's recover can't catch it, so influxd crashes. Any user who can run queries can trigger it. Reproduced live by the verifier.
  1. query/date_part.go:429 — negative epoch groups sort out of order · CONFIRMED

computeDimKey encodes the signed int64 as unsigned big-endian: binary.BigEndian.PutUint64(buf[:], uint64(val)). A negative int64 becomes a huge uint64, so its bytes sort after every non-negative value, and reduce() sorts these DimKey strings to order the emitted rows.

  • Trigger: date_part('epoch', time) over data spanning 1970 (InfluxDB ns timestamps reach back to 1677), e.g. SELECT count(value) FROM cpu GROUP BY date_part('epoch', time).
  • Impact: pre-1970 (negative) buckets are emitted at the wrong end instead of chronologically. Silent for all other parts (year/month/dow/hour are non-negative), which is why the positive-only tests pass.
  1. query/iterator.go:1051 — rolling-upgrade mis-grouping across the wire codec · PLAUSIBLE

encode/decodeIteratorOptions add proto fields DatePartDimensions (23) and NeedTimeRef (24). An older remote data node drops the unknown fields, so it never computes the date_part aux value.

  • Impact: during a mixed-version rolling upgrade (new coordinator, old data node), a distributed GROUP BY date_part(...) query returns points with no date_part aux; len(aux) < len(dims) collapses everything into one ungrouped series — silently wrong results, no error. Verifier confirmed the mechanism but couldn't fully exercise the cross-version path, hence PLAUSIBLE.
  1. query/compile.go:975 — observable error-text change · CONFIRMED (low impact)

The invalid-GROUP-BY-function error was reworded from "only time() calls allowed in dimensions" to "only time() and date_part() calls allowed in dimensions". Any client/driver/test that string-matches the old text (e.g. for GROUP BY now()) silently stops matching. Arguably a correct improvement — flagged only because it's an observable behavior change.

Cleanups

  1. query/iterator.go:722DatePartDimension.Name is redundant derivable state · CONFIRMED

Name is always Expr.String(), yet it's round-tripped over the wire and read at some sites (select.go:686/719, subquery.go:64) while cursor.go:279 ignores it and recomputes Expr.String(). It's self-documented as a footgun: a decoded/hand-built dimension whose Name diverges silently yields an unpopulated grouped column. Drop the field; use Expr.String() everywhere.

  1. query/select.go:710 — two if len(opt.DatePartDimensions) > 0 loops over the same slice · CONFIRMED

Lines 682–691 and 710–721 both guard on the same condition and iterate the same dimensions (one appends the Field, the other the Aux/auxKeys). Easy to drift — add a dim to one loop but not the other and you get a column with no backing aux slot. Merge into one pass.

  1. query/cursor.go:487newFilterCursor defeats the NeedTimeRef cache · CONFIRMED

opt.NeedTimeRef is computed once and documented as caching the AST walk "to avoid repeatedly walking the condition AST for every iterator creation" — but newFilterCursor calls conditionNeedsTimeRef(filter) again per subquery filter cursor, re-walking the whole tree. Pass opt.NeedTimeRef (or opt) in.

  1. query/cursor.go:165 — duplicated "expr contains a date_part Call" WalkFunc · PLAUSIBLE

The same WalkFunc{ if call.Name == DatePartString ... } idiom appears in scannerCursorNeedsDatePart (cursor.go:171), conditionNeedsTimeRef (iterator.go:755), and inline in compile.go (validateDatePartSelectFields, validateDatePartAnchor). Change how date_part is identified and you must update every copy; miss one → inconsistent detection. Hoist exprContainsDatePart(expr).

  1. models/rows.go:36 (and :19) — FNV-hashing an already-sorted slice to compare it · CONFIRMED

SameSeries computes a full FNV64a hash of both GroupingKeys slices on every per-row comparison (chunked responses in handler.go:874), even though the slices are already sorted (emitter.sortedKeys). cmd/influx/cli/cli.go:884 does the same comparison with slices.Equal. The hash is more work, inconsistent with the CLI, and two distinct key sets that collide under FNV64a would merge into one series. Use slices.Equal.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 31 out of 33 changed files in this pull request and generated no new comments.

Files not reviewed (1)
  • query/internal/internal.pb.go: Generated file

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 31 out of 33 changed files in this pull request and generated no new comments.

Files not reviewed (1)
  • query/internal/internal.pb.go: Generated file
Comments suppressed due to low confidence (2)

query/emitter.go:80

  • Emitter stores row.GroupingKeys directly in e.groupingKeys, but that map is reused and cleared by cursor scanning (scannerCursorBase.Scan calls clear(row.GroupingKeys)). This aliases mutable state across iterations and can cause grouping keys to change while the emitter is still coalescing rows, splitting/merging series incorrectly. Clone the map (or store an immutable representation) before saving it on the emitter.
func (e *Emitter) createRow(series Series, groupingKeys map[string]struct{}, values []interface{}) {
	e.series = series
	e.groupingKeys = groupingKeys
	e.row = &models.Row{
		Name:         series.Name,

query/iterator.gen.go.tmpl:569

  • When an aux value is nil and defaultValue == SkipDefault, ScanAt currently leaves m[k.Val] unchanged. With date_part grouping, reduce intentionally sets non-active date_part aux slots to nil so those dimension column values should become null; leaving the map entry untouched can leak a prior row's value into the current row. Clear the map entry when no default fill is applied.
			// Insert the fill value if one was specified.
			if s.defaultValue != SkipDefault {
				m[k.Val] = castToType(s.defaultValue, k.Type)
			}
		}

@davidby-influx

Copy link
Copy Markdown
Contributor

The first suppressed copilot finding above does point to a future-proofing we might want to do:

What is true is the fragility the finding is reaching for:

  • The Cursor interface doc (query/cursor.go:79-80) explicitly invites Row reuse,
    and the comment at query/cursor.go:227-229 says the clear exists so "callers
    that reuse the Row across scans keep the allocation." There is such a caller —
    the *IteratorMapper types in query/iterator.gen.go (itr.cur.Scan(&itr.row)),
    which reuse a Row field. They don't read GroupingKeys, so they're fine today.

  • If anyone hoists var row Row out of the Emitter loop — a natural-looking
    optimization, since the current code allocates a Row and a Values slice per
    scan — the bug becomes real and silent: e.groupingKeys and row.GroupingKeys
    would be the same map, sameGroupingKeys would always return true, and rows
    with different date_part grouping dimensions would be merged into one
    models.Row.

If you want that hazard removed rather than documented, the zero-allocation fix
is to drop the e.groupingKeys field and compare against the immutable sorted
slice already stored on the row:

	} else if e.series.SameSeries(row.Series) && sameGroupingKeys(e.row.GroupingKeys, row.GroupingKeys) {

	func sameGroupingKeys(sorted []string, b map[string]struct{}) bool {
		if len(sorted) != len(b) {
			return false
		}
		for _, k := range sorted {
			if _, ok := b[k]; !ok {
				return false
			}
		}
		return true
	}

e.row.GroupingKeys comes from sortedKeys, which builds a new slice, so it can't
alias cursor state.

@davidby-influx davidby-influx left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More coming,

Comment thread coordinator/statement_executor.go

@davidby-influx davidby-influx left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need tests for date_part roll-up across dst boundaries in both directions:

  • A date_part('day') roll-up where a single local day is 23 or 25 hours long and holds points on both sides of the transition.
  • GROUP BY date_part('hour', time) under tz() at fall-back, where local hour 1 occurs twice, the case where two distinct UTC instants must merge into one bucket.
  • Spring-forward's non-existent local hour (02:00–03:00) as a grouping key or a WHERE date_part('hour', time) = 2 filter.
  • Southern-hemisphere / non-US transition rules, or half-hour-offset zones (e.g. Australia/Lord_Howe).

@devanbenz

Copy link
Copy Markdown
Author

We need tests for date_part roll-up across dst boundaries in both directions:

* A date_part('day') roll-up where a single local day is 23 or 25 hours long and holds points on both sides of the transition.

* GROUP BY date_part('hour', time) under tz() at fall-back, where local hour 1 occurs twice, the case where two distinct UTC instants  must merge into one bucket.

* Spring-forward's non-existent local hour (02:00–03:00) as a grouping key or a WHERE date_part('hour', time) = 2 filter.

* Southern-hemisphere / non-US transition rules, or half-hour-offset zones (e.g. Australia/Lord_Howe).

I've added tests for DST and rollups 👍

@davidby-influx davidby-influx left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some more suggested tests, one optimization, a defensive change to the Emitter

Comment thread coordinator/statement_executor.go Outdated
Comment thread query/emitter.go Outdated
Comment thread query/emitter.go Outdated
Comment thread query/iterator.gen.go.tmpl

@davidby-influx davidby-influx left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One code nit, but needs spring forward tests, as well

Comment thread tests/server_test.go Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 32 out of 34 changed files in this pull request and generated 1 comment.

Files not reviewed (1)
  • query/internal/internal.pb.go: Generated file
Comments suppressed due to low confidence (1)

query/point.go:295

  • decodeAux assumes the date_part grouping key is stored as a raw 9-byte StringValue and passes it directly to decodeKey. If the encoding is changed to be protobuf-safe (see comment above), this decode path must be updated in lockstep (including validation/error handling) so date_part grouping still round-trips over the iterator wire codec.
		case auxDatePartKey:
			if pb[i].StringValue != nil {
				if key, err := decodeKey(*pb[i].StringValue); err == nil {
					aux[i] = key
				}

Comment thread query/point.go

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 32 out of 35 changed files in this pull request and generated no new comments.

Files not reviewed (1)
  • query/internal/internal.pb.go: Generated file
Comments suppressed due to low confidence (1)

query/iterator.gen.go.tmpl:568

  • In IteratorScanner.ScanAt, when an aux value is nil (or otherwise hits the default case) and defaultValue is SkipDefault, the map entry for that key is left untouched. With date_part grouping, non-active dimension aux slots are intentionally nil, so this can leave a stale value from the previous point in the evaluation map and leak into result columns / expression evaluation.

Consider clearing the key when no default is applied (e.g. delete(m, k.Val)) so nil aux values reliably produce NULL for that column.

		default:
			// Insert the fill value if one was specified.
			if s.defaultValue != SkipDefault {
				m[k.Val] = castToType(s.defaultValue, k.Type)
			}

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 32 out of 35 changed files in this pull request and generated no new comments.

Files not reviewed (1)
  • query/internal/internal.pb.go: Generated file
Suppressed comments (1)

query/iterator.go:1110

  • decodeIteratorOptions decodes DatePartDimensions by casting the on-wire Expr int directly to DatePartExpr without range validation. If an iterator options payload is received with an unknown/invalid expr (e.g. mixed-version cluster, corruption), this can lead to silent wrong results in some paths (e.g. datePartMap used as an iterator-mapper driver will treat an invalid expr as a nil value and some aggregates may quietly return 0 instead of failing fast).

Consider validating d.GetExpr() here (must be Year <= expr < Invalid) and returning an error if out of range before constructing the DimensionGrouper. This makes the system fail loudly instead of producing misleading output.

	// Decode date_part GROUP BY dimensions and rebuild the grouper from them.
	if dims := pb.GetDatePartDimensions(); len(dims) > 0 {
		opt.DatePartDimensions = make([]DatePartDimension, len(dims))
		for i, d := range dims {
			opt.DatePartDimensions[i] = DatePartDimension{
				Expr: DatePartExpr(d.GetExpr()),
			}
		}
		opt.DimensionGrouper = NewDatePartGrouper(opt.DatePartDimensions)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

1.x area/influxql Issues related to InfluxQL query language kind/enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[1.x] Add date_part scalar function to influxdb

7 participants