Skip to content

fix: count only live rows in the planner estimate - #868

Draft
OffgridwithJD wants to merge 1 commit into
mainfrom
audit/estimate-ignores-deletes
Draft

fix: count only live rows in the planner estimate#868
OffgridwithJD wants to merge 1 commit into
mainfrom
audit/estimate-ignores-deletes

Conversation

@OffgridwithJD

Copy link
Copy Markdown
Collaborator

Summary

  • relation_estimate_size is the only row count the planner sees. It summed row_group.row_count and ignored delete_vector.
  • After a bulk DELETE, ANALYZE recorded the live reltuples but EXPLAIN still priced the physical occupancy.

Test coverage

  • test/estimate_deleted.sh

Made with Cursor

Co-authored-by: Cursor <cursoragent@cursor.com>
@OffgridwithJD

Copy link
Copy Markdown
Collaborator Author

The fix works — measured

A/B against 8b39053, 400,000 rows, stripe_row_limit=2000, DELETE ... WHERE id % 10 = 0:

                  main      #868
plan estimate    400000    360000     (live rows = 360000)

The estimate goes from physical occupancy to the live count exactly. The defect
and the fix are both real.

What it costs, and this is the part I would want settled before it lands

relation_estimate_size is called by the planner for every query on the table,
and this adds one PgColumnarReadDeleteVectorList per row group. Each of those is
open_columnar_table("delete_vector") + a systable_beginscan + a detoast and
memcpy of every matching bitmap.

Two scale points, same box, 20 EXPLAINs each, both arms premise-gated on the
fixture actually having the groups and rows claimed:

row groups main #868 added
200 1.280 ms 2.141 ms +0.861 ms
800 2.814 ms 5.801 ms +2.987 ms

That is roughly 4 microseconds of planning time per row group, paid on every
query, including ones that touch no data. It is linear in group count, so at the
shipped stripe_row_limit=150000 a 100M-row table (~667 groups) pays about 2.7 ms
per plan and a 1B-row table (~6,700 groups) about 27 ms per plan. SELECT ... WHERE id = 42 currently plans in about 1.3 ms.

The anyDeletes short-circuit means a table with no deletes pays nothing, which is
the right instinct — but a table that has ever been DELETEd from pays on every
query forever, which is the common case this PR exists to serve.

A cheaper shape for the same benefit

The exactness the bitmap OR buys is not exactness the planner needs. *tuples is
an estimate; the planner rounds it into a cost.

delete_vector.deleted_count is already stored per row. Summing it per storage id
is one catalog scan for the whole relation instead of one per group:

SELECT sum(deleted_count) FROM pgcolumnar.delete_vector WHERE storage_id = ?

The reason the PR ORs bitmaps instead is that a row deleted twice would be
double-counted. But that error is bounded and one-directional — it can only
underestimate live rows — and the existing per-group clamp

if (deleted > rg->rowCount) deleted = rg->rowCount;

already contains it. A clamp against the storage total does the same job. An
estimate that is occasionally a little low is a far smaller problem than several
milliseconds on every plan.

If exactness really is wanted here, the other option is to cache it — the count
only changes when delete_vector changes.

Smaller notes

  • Moving pgcolumnar_group_deleted_count into columnar_delete_vector.c and
    sharing it is right; the alternative was a second copy that could drift.
  • The bit loop counts one bit at a time. If this stays per-group, pg_popcount
    over the whole mask with a masked final byte is the same answer and much less
    work.
  • PgColumnarStorageHasDeleteVector is called with the catalog snapshot and the
    per-group reads use the same one, so the count cannot straddle two snapshots.
    Worth a line in the comment saying that is deliberate.

I have not measured a table with many deletes and many columns, where the
per-group bitmaps are larger; the numbers above are the cheap case.

Reviewed as OffgridwithJD. Not approving — same account as the author.

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.

2 participants