Skip to content

Preserve partition statistics across SQL writes, deletes, and bulk flushes - #477

Open
HamzaMPSY wants to merge 1 commit into
stac-utils:mainfrom
HamzaMPSY:fix/partition-stats-write-path-correctness
Open

Preserve partition statistics across SQL writes, deletes, and bulk flushes#477
HamzaMPSY wants to merge 1 commit into
stac-utils:mainfrom
HamzaMPSY:fix/partition-stats-write-path-correctness

Conversation

@HamzaMPSY

Copy link
Copy Markdown

Current main avoids the synchronous per-write ANALYZE introduced in
v0.9.11 by conservatively widening partition statistics during writes and
tightening them later.

While validating issue #471, I found correctness gaps in that newer
architecture:

  1. SQL INSERT/UPDATE batches do not always provide an incoming spatial
    extent. After an exact tightening, an item outside the previous extent
    could leave a non-NULL spatial bound that was too small. Partition
    pruning could then exclude a partition containing a matching item.

  2. DELETE could leave exact counts, temporal ranges, and spatial extents
    stale while dirty = false, meaning dirty-only maintenance would never
    revisit that partition.

  3. The Rust bulk loader prepares partitions before its COPY/flush
    transaction. Maintenance could tighten a partition between preparation
    and flush, allowing the later rows to commit without restoring
    conservative statistics.

Changes

  • Invalidate an exact spatial bound when SQL staging cannot provide an
    incoming extent. A NULL spatial bound is conservative and prevents unsafe
    partition pruning until deferred tightening recomputes it.

  • Record deleted partitions in a durable maintenance queue. The DELETE path
    remains cheap and avoids acquiring partition advisory locks after item-row
    locks, which would introduce lock-order inversion.

  • Include queued DELETE partitions in deferred statistics tightening and
    consume queue entries transactionally with the exact partition scan.

  • Recalculate staged temporal ranges, spatial extent, and count inside the
    bulk-flush transaction.

  • Acquire partition locks in deterministic order for multi-partition batches.

  • Regenerate the unreleased installation and migration SQL from the source
    fragments.

Correctness coverage

Regression tests cover:

  • tighten, then INSERT outside the previous spatial extent;

  • tighten, then UPDATE outside the previous spatial extent;

  • delete the final item, run deferred maintenance, and verify empty exact
    statistics;

  • prepare a bulk load, tighten in the intervening window, then flush and
    verify that statistics conservatively cover the committed rows;

  • privileges on the DELETE maintenance queue.

Performance

This change does not reintroduce synchronous ANALYZE.

Local Podman benchmark results:

Operation main patched Difference
━━━━━───── ━━━━━━ ━━━━━─ ━━━━━━━───
30k-row binary flush 206.441 ms 227.234 ms +20.793 ms (+10.1%)
────────── ────── ────── ──────────
Single DELETE 0.026 ms 0.035 ms +0.009 ms

The bulk-loader overhead is one set-based scan of the staging table, rather
than a full ANALYZE of the destination partition. DELETE only appends a
small indexed queue row; exact recomputation remains deferred.

The primary gain is correctness:

  • no false-negative spatial pruning after an out-of-extent write;
  • DELETE statistics eventually become exact;
  • bulk rows and conservative metadata become visible atomically;
  • current main's deferred-maintenance performance architecture is preserved.

Relation to #471

Current main already removes the original synchronous-ANALYZE write latency
reported in #471. This PR closes correctness gaps found while verifying that
replacement architecture; it does not revert to per-write analysis.

Validation

Passed:

  • formatting, Ruff, and type checks;
  • 370 PgTAP assertions;
  • all basic SQL fixtures;
  • pg_dump/pg_restore round trip;
  • full Rust test suite and doctests;
  • opt-in concurrent writers against real PostgreSQL;
  • a two-session DELETE/tightening handoff test.

scripts/test --migrations currently encounters an existing upstream
migration-chain failure before reaching this patch:
where_stats(text,boolean,jsonb) still depends on search_wheres when the
older table is dropped. The same failure reproduces without this branch, so
that unrelated migration-history repair is intentionally not included here.

How to test

Podman is sufficient; Colima is not required. With your existing Docker-compatible Podman setup:

scripts/test --pgtap --basicsql
scripts/test --rust
scripts/test --pgdump

Or run the complete default gate:

scripts/test

Optional real concurrent-writer coverage:

PGSTAC_TEST_DSN=postgresql://username:password@localhost:5439/pgstac_rs_test_rich
cargo test --manifest-path src/pgstac-rs/Cargo.toml
--features cli
--test concurrency concurrent_writers
-- --nocapture

The project wrappers currently spell the backend command as docker compose; using Podman’s Docker-compatibility layer is enough.

Checklist

  • Linting: Code is formatted and linted
  • Tests: Tests pass. I have included new tests for these changes where applicable.
  • Edge Cases: I have manually verified "unhappy paths" and edge cases beyond the basic success criteria (e.g., database connection timeouts, malformed input, strict mapping rejections).
  • Documentation: I have updated README.md to reflect any new environment variables, configuration changes, or breaking updates.
  • Accountability: I can explain the implementation logic for every line of code submitted.

AI tool usage

  • AI (Copilot or something similar) supported my development of this PR. See our policy about AI tool use. Use of AI tools must be indicated.

Policy: We require a "human-in-the-loop." You are the author and are fully accountable for all submitted code. Please ensure all tool-generated content is thoroughly reviewed before submission to ensure it is not an "extractive contribution" that squanders maintainer time.

Invalidate exact spatial bounds when SQL staging has no incoming extent. Queue deleted partitions for deferred tightening without reversing advisory-lock order. Revalidate temporal ranges, spatial extents, and counts inside the bulk-flush transaction. Regenerate the unreleased install and migration SQL from the source fragments.
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