Skip to content

Revert the query_store_stats compress_orderby - #2379

Merged
erikdarlingdata merged 1 commit into
devfrom
revert/2377-compress-orderby
Aug 19, 2026
Merged

erikdarlingdata merged 1 commit into
devfrom
revert/2377-compress-orderby

Conversation

@erikdarlingdata

Copy link
Copy Markdown
Owner

Reverts #2377. Refs #2378.

The soak found it: on a store with chunks already compressed under the old settings, compress_chunk(chunk, recompress => true) fails with

ERROR:  the orderby column "query_id" cannot have a bloom sparse index
DETAIL:  For orderby columns, a minmax sparse index is added automatically
         and cannot have bloom sparse index.

The existing compressed form of query_id carries a bloom sparse index, created when it was an ordinary column, and TimescaleDB will not have both that and the minmax index it adds automatically for an orderby column.

Why revert rather than document around it

query_store_backfill_enabled is true on the use1 store, and three of its five query_store_stats chunks (08-15, 08-16, 08-17) are compressed under the old settings. Late Query Store intervals landing in one of those chunks is a path that reaches automatic recompression — so this is reachable rather than theoretical, and the failure would be in ingestion rather than in a manual command someone chose to run.

Against that: the benefit is new-chunks-only, and the measured 2.9x is synthetic against a table that already sits at 4.5x, so the real-world gain is smaller than the headline. A storage optimisation is not worth carrying an unquantified ingestion risk into a release. The finding stands and the work is not wasted — #2378 keeps it, and it can return once the bloom interaction is handled.

The revert is only half of it

Verified, and it is the part that would have been missed: re-applying the pre-#2377 statement does NOT clear the orderby.

BEFORE revert-ALTER: query_id(ord1), collection_time(ord2)
ALTER TABLE
AFTER  revert-ALTER: query_id(ord1), collection_time(ord2)

ALTER ... SET (compress, compress_segmentby = ...) leaves an existing compress_orderby in place, so shipping this revert alone would leave every already-upgraded store exactly as broken as before while looking fixed.

Clearing it needs the default stated explicitly, which then restores recompression:

ALTER TABLE ... SET (..., timescaledb.compress_orderby = 'collection_time DESC')
  -> now: collection_time(ord1)
  -> compress_chunk(chunk, recompress => true) succeeds again

No released version ever carried #2377 — it existed only on dev and the three soak boxes — so that reset is a one-time operation on those three stores rather than anything the product needs to ship. I am applying it there directly rather than adding a permanent un-do statement to the codebase for a state no customer install can be in.

@claude

claude Bot commented Aug 19, 2026

Copy link
Copy Markdown

Reviewed. This is a clean, mechanical revert of #2377:

  • TimescaleSupport.cs: CompressionOrderBy dictionary and the per-table compress_orderby branch are fully removed; EnableCompressionSql(string) is back to the single unconditional ALTER TABLE it was before Compress query_store_stats on query identity instead of on time #2377. No dangling references to CompressionOrderBy/compress_orderby/the removed helper anywhere else in the tree.
  • Tests: both query_plan_dim is 63% of the Darling store and never shrinks — and query_store_stats compresses at 2.7x against siblings' 10-30x #2316-specific tests (CompressionSql_OrdersQueryStoreStatsByQueryIdentity_NotByTime, CompressionSql_LeavesEveryOtherTableOnTheDefaultOrder) and their private ExtractOrderBy helper are removed together, with no other test referencing them.
  • CHANGELOG.md: the query_store_stats changelog line is removed; the [#2316] reference-link definition correctly stays since two other entries still cite it, so no dangling link.
  • No Lite/Darling parity concern — this is Darling-only TimescaleDB compression config; Lite has no equivalent (DuckDB, no hypertable compression).

The PR body's own analysis (bloom-vs-minmax sparse index conflict on recompress => true for already-compressed chunks) is convincing and matches why a straight revert — rather than a forward-fix — is the right call here. It's also explicit and correct that re-applying the pre-#2377 ALTER does not clear an already-set compress_orderby on upgraded chunks, and that the fix for that (explicit compress_orderby = 'collection_time DESC' on the three affected soak stores) is being applied directly rather than shipped as code, since no released version ever carried #2377. That's a reasonable scope call given it's a one-time cleanup on non-customer boxes.

No correctness, security, or parity issues found.

@erikdarlingdata
erikdarlingdata merged commit a00c9aa into dev Aug 19, 2026
6 checks passed
@erikdarlingdata
erikdarlingdata deleted the revert/2377-compress-orderby branch August 19, 2026 20:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant