Skip to content

Fix JSON-null datapoint queries and honor MODE gap filling - #3447

Open
ebariaux wants to merge 16 commits into
mainfrom
bugfix/queries-json-null
Open

ebariaux wants to merge 16 commits into
mainfrom
bugfix/queries-json-null

Conversation

@ebariaux

@ebariaux ebariaux commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

Stored JSON null values caused numeric and boolean datapoint queries to fail during SQL
casting. Normalize JSON null to SQL NULL before conversion, and preserve unavailable
values during boolean conversion instead of interpreting them as false.

This applies to historical and predicted datapoints:

  • All retains unavailable observations and their timestamps.
  • Numeric Nearest returns the nearest observation even if its value is unavailable.
  • MIN, MAX, AVG, SUM, MEDIAN and MODE ignore unavailable input values; buckets with no
    available values return null. COUNT still counts stored rows, including null-valued rows.
  • Boolean MODE and MEDIAN return numeric 1/0 values compatible with the query service.
  • DIFFERENCE preserves unavailable endpoints. Explicit nulls interrupt carry-forward;
    empty buckets still carry the previous bucket value when gap filling is enabled.
    When the range starts with empty buckets, the first available reading now produces an unavailable delta instead of its absolute value. Charts may therefore show a gap where they previously showed an artificial spike.
  • LTTB omits unavailable observations before downsampling. It does not retain null gap
    markers; an all-null range returns no points.

MODE now uses a grouped native aggregate, enabling Timescale gap filling. Numeric ties
still select the smaller value and boolean ties select true. Boolean MEDIAN retains its
threshold of at least 0.5 for a true/1 result.

Compatibility / migration

  1. MODE with gapFill=true now returns empty buckets across the inclusive requested range,
    including leading/trailing gaps and entirely empty ranges. A five-minute query from
    08:30 through 09:00 with observations only through 08:55 now returns an additional
    (09:00, unavailable) bucket. Callers must handle added rows and unavailable values.
    Use gapFill=false if only populated buckets are wanted. Fine-grained sparse ranges
    can now exceed the configured result limit (default 100000 points).
  2. DIFFERENCE after a leading empty bucket now returns an unavailable delta for the first
    available reading, rather than its absolute value. Callers must not assume a zero
    baseline when the previous bucket is unavailable.
  3. Boolean MODE/MEDIAN previously lost available results during conversion to the query
    service's numeric format. They now return 1 or 0, so existing callers may receive real
    values where they previously saw unavailable results.

Wire format is unchanged: datapoints use x for timestamp and y for value. The configured
JSON mapper omits null properties. A retained unavailable datapoint therefore appears as
{"x":1767225600000}, not necessarily with an explicit "y":null. Clients must handle an
absent y. Both forms deserialize to a null value. LTTB omits the observation entirely.
No database migration or rewrite of stored datapoints is required.

Validation

The core query suites passed 136 tests (132 new null-query cases plus four existing query
features), with none skipped and UTC explicitly configured on the test JVM. Coverage spans
historical/predicted data, NUMBER/BIG_NUMBER/BOOLEAN, both gap-fill settings, null-only and
empty ranges, ties, recovery after unavailable differences, query bounds and real LTTB
point selection. Each implementation step was checked against failing regressions.
The serialization probe also verified omitted y, explicit-null deserialization, and
preservation of zero and false. All 18 adjacent storage/export/deserialization tests
also passed, with none skipped.

MODE performance

Compared the previous null-safe custom MODE query against the grouped native query using
288 captured EXPLAIN ANALYZE plans on PostgreSQL 17.9 / TimescaleDB 2.26.4, with matching
indexes and seven-day chunks. Three measured warm-cache runs followed one warmup, with
old/new execution order alternated. All tested equal-output data queries improved.
For 20000 numeric observations across 4000 buckets, median execution fell from 3123 ms to
9.2 ms: the old plan scanned the 20000-row CTE 4000 times. Large numeric and boolean
buckets also improved despite sort spills. These are synthetic database execution times,
not end-to-end production guarantees.
Sparse minute-level gap filling across a year now generates 525600 rows instead of 365;
the count query increased from 0.55 ms to 56.8 ms and the default limit rejects that range.
All benchmark data and objects were rolled back.

Replace correlated frequency queries with grouped MODE aggregates.
Preserve null handling and tie-breaking, and cover empty ranges and leading/trailing gaps with regression tests.

BREAKING CHANGE: MODE with gapFill=true now includes empty buckets as null-valued results across the inclusive query range.
Results may contain more rows and exceed the configured datapoint query limit.
Use gapFill=false to retain populated-bucket-only results.
Normalize JSON nulls before casting and remove the fallback that replaced unavailable differences with absolute readings.
Document carry-forward semantics and add regression coverage.

BREAKING CHANGE: With gap filling enabled, the first reading after a leading empty bucket now yields null instead of its absolute value as the difference.
@ebariaux ebariaux added Bug Error or issue in the system Breaking Change Indicates that a PR includes a breaking change labels Oct 19, 2026
@ebariaux
ebariaux requested a review from a team October 19, 2026 09:26

@richturner richturner left a comment

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.

🤖 AI-assisted review: this review was prepared with the help of Claude Code and checked by me before posting.

Overall this looks good. I'd approve after a couple of small changes. The fix closes the cast failures from #3403 and adopts the null semantics the issue asked for.

What works well

  • The casts no longer fail. nullif(value, 'null'::jsonb) is applied before every numeric and boolean cast, across all four query types (All, Nearest, LTTB and Interval).
  • Booleans no longer turn null into false. The CASE … WHEN true THEN 1 WHEN false THEN 0 ELSE NULL END conversion means an average over [true, null] gives 1.0, not 0.5.
  • Boolean MODE and MEDIAN now return values. The ::integer on these is needed: AbstractDatapointService converts each result to Double with getValueCoerced, and a boolean result came back null there.
  • DIFFERENCE behaves as the issue asked. Removing the COALESCE(..., numeric_value) fallback stops a reading after a gap from showing up as a fake jump. locf keeps its default, so empty buckets still carry the previous value but an explicit null breaks the chain. The test expectations match the 0, null, null, null, 10 example from the issue.
  • MODE is simpler and faster. PostgreSQL's built-in MODE() WITHIN GROUP skips nulls, and the ordering (ASC for numbers, DESC for booleans) keeps the existing tie-breaking.
  • COUNT is unchanged, as the issue requested.
  • Test coverage is thorough. It covers historical and predicted storage, NUMBER, BIG_NUMBER and BOOLEAN, both gap-fill settings, all-null and empty ranges, range boundaries, and LTTB point selection compared against a series stored without the nulls.

Should change

  1. AssetDatapointNullQueryTest changes the JVM time zone and never restores it. setup() calls TimeZone.setDefault(TimeZone.getTimeZone("UTC")). Integration specs share one JVM, so every spec that runs afterwards also runs in UTC. That can hide time-zone bugs elsewhere, or make results depend on test order. At minimum, save the old zone and restore it in cleanupSpec. Ideally, find out why this spec needs UTC when AssetDatapointQueryTest doesn't. If buckets come out wrong outside UTC, that is a real bug that deserves its own issue rather than being hidden by the test.
  2. The MODE gap-fill change is a behaviour change bundled into a bug fix. It is well documented under "Compatibility / migration" and was clearly needed, because the old CTE was quietly ignoring gapFill. But it changes results for existing callers and can now hit the default 100,000-point limit on sparse, fine-grained ranges (as the PR's own benchmark shows). I'd like a maintainer to explicitly accept it in this PR, or split it out, and it needs a release note either way.

Nits

  • Repeated SQL fragment. nullif(value, 'null'::jsonb) and the boolean CASE appear about 12 times across four classes. One protected constant in AssetDatapointQuery would keep the next change consistent.
  • Undocumented tie-breaking. PostgreSQL's docs say mode() picks arbitrarily among equally frequent values. In practice it picks the first in sort order, and the tests pin that down. A one-line comment saying the query relies on this would help.
  • DIFFERENCE now returns null after a leading empty bucket. With gapFill=true, the first reading after an empty leading bucket now gives null instead of its absolute value. I think that's an improvement, because the old result was a fake spike. It is listed in the PR, but UI charts with DIFFERENCE and gap fill on (e.g. energy meters) will now show a gap where they used to show a spike.
  • Null values are omitted from the JSON. A null y is left out entirely, so a datapoint serialises as {"x":…}. Gap-filled aggregates already produced this, so the chart code should cope, but a quick check in the UI would be worthwhile.

Points 1 and 2 are the only ones I'd block on.

@ebariaux

ebariaux commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor Author
  1. was required because the JDBC connection was created with its time zone before the pseudo-clock changed the JVM timezone to UTC. And the pseudo-clock did not reset that timezone, so any specs run after one using the pseudo-clock were impacted.
    I changed the Pseudo-clock to not do that anymore and ran all tests afterwards (one needed fixing).

  2. that bug fix has been removed from this PR and is now tracked by issue MODE datapoint queries ignore gapFill and omit empty buckets #3480

  3. extracted constants

  4. added comment but part of MODE datapoint queries ignore gapFill and omit empty buckets #3480 now

  5. added precision in this PR description

  6. (Codex) Checked both chart components in Chromium with omitted and explicit-null y values. Neither causes errors or converts unavailable values to zero. Bar charts leave empty slots; line charts retain their existing behavior of filtering unavailable points and connecting the remaining readings.

Restore MODE's existing populated-bucket-only behavior for both gapFill
settings, retaining JSON-null handling and numeric/boolean tie-breaking.
Restore the corresponding expectations and document the MODE limitation.

The gap-fill behavior change and its regression coverage will be carried
by bugfix/mode-query-gap-fill as a separate follow-up.
@sonarqubecloud

Copy link
Copy Markdown

This branch has not been deployed

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

Labels

Breaking Change Indicates that a PR includes a breaking change Bug Error or issue in the system

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Datapoint queries fail on stored JSON null values for numeric and boolean attributes

2 participants