[PATCH] Test coverage for pg_clear_attribute_stats() null arguments
Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.
This thread has been committed, so CI has stopped here. Anything below is the last result it produced.
You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:
docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t253585psql -h localhost -U postgresBuilt from patchset v3 (message #3), September 02, 2026 at 01:44 AM.
Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:
git clone --branch t253585_3 https://github.com/hackorum-dev/postgres.gitIn a checkout you already have, add the fork once:
git remote add hackorum https://github.com/hackorum-dev/postgres.gitthen, for this patchset and every later one:
git fetch hackorum t253585_3 && git checkout t253585_3Patchset v3 (message #3) is on t253585_3
Hi hackers,
pg_clear_attribute_stats() has no test coverage for its null-argument
checks, while pg_restore_relation_stats(), pg_restore_attribute_stats(),
pg_restore_ext_stats() and pg_clear_ext_stats() all do. It is called
exactly once in the whole test suite, and only on its success path.
That gap let a real bug live from v18 until 11ed011ae22([1]https://github.com/postgres/postgres/commit/11ed011ae22a620b01a43634b5eea2f9af5c709c)
The attached patch adds four cases, one per required argument. It is
test-only, with no behaviour change. The checks run at the top of the
function, before the relation lookup, so the expected output does not
depend on the state of the cluster.
[1]: https://github.com/postgres/postgres/commit/11ed011ae22a620b01a43634b5eea2f9af5c709c
Thanks,
Shihao
On Thu, Aug 27, 2026 at 10:44:58PM -0400, shihao zhong wrote:
pg_clear_attribute_stats() has no test coverage for its null-argument
checks, while pg_restore_relation_stats(), pg_restore_attribute_stats(),
pg_restore_ext_stats() and pg_clear_ext_stats() all do. It is called
exactly once in the whole test suite, and only on its success path.
That gap let a real bug live from v18 until 11ed011ae22([1])
Right.
The attached patch adds four cases, one per required argument. It is
test-only, with no behaviour change. The checks run at the top of the
function, before the relation lookup, so the expected output does not
depend on the state of the cluster.[1]: https://github.com/postgres/postgres/commit/11ed011ae22a620b01a43634b5eea2f9af5c709c
Hmm. I don't think that this is ambitious enough, we also missing
tests for error cases related to the following pieces for
pg_clear_attribute_stats():
- System columns defined, should be rejected.
- Missing relation or namespace.
- Missing column.
- Perhaps sequence and view case rejection?
- Something with inherited = true?
Could you expand the patch to do more of this stuff, please?
--
Michael
Could you expand the patch to do more of this stuff, please?
v2 attached. On top of the four null arguments it covers a missing
schema, relation and column, a system column, and a sequence and a view
for the relkind rejection.
For inherited I used a functional case rather than an error one: plant an
inherited and a non-inherited row for one column, clear the inherited
one, and check that the other survives.
Thanks,
Shihao
On Tue, Sep 01, 2026 at 09:32:02PM -0400, shihao zhong wrote:
v2 attached. On top of the four null arguments it covers a missing
schema, relation and column, a system column, and a sequence and a view
for the relkind rejection.
The cases lacked a bit of consistency in terms of input values,
ordering and the comments, so fixed a bit of all that.. And done.
For inherited I used a functional case rather than an error one: plant an
inherited and a non-inherited row for one column, clear the inherited
one, and check that the other survives.
That does the job. Thanks.
--
Michael