On 2/4/25 17:54, Tomas Vondra wrote: > On 2/4/25 16:02, Tomas Vondra wrote: >> ... >> >> Thanks for the report. And yeah, clamping it to 1 seems like the right >> fix for this. I wonder if it's worth inventing some sort of test for >> this, shouldn't be too hard I guess. >> >> In any case, I'll take care of the fix/backpatch soon. >> > > Here's a proposed fix, including a simple test in the stats suite. >
Here's a slightly improved fix, with a proper commit message and simplified test case. I plan to push this once the releases get out (I know it was stamped already, but ...). regards -- Tomas Vondra
From 02bd8d0ac771da4b460a2b1489f25f23d47495c1 Mon Sep 17 00:00:00 2001 From: Tomas Vondra <tomas.von...@postgresql.org> Date: Tue, 18 Feb 2025 12:05:30 +0100 Subject: [PATCH] Correct relation size estimate with low fillfactor Since commit 29cf61ade3, table_block_relation_estimate_size() considers fillfactor when estimating number of rows in a relation before the first ANALYZE. But the formula failed to consider that tuples may be larger than free space determined by fillfactor, ending with density 0. This ultimately means the relation is estimated to contain a single row. The executor however places at least one tuple per page, even with very low fillfactor values, i.e. the density should be at least 1. Fixed by clamping the density estimate using clamp_row_est(). Reported by Heikki Linnakangas. Fix by me, with regression test inspired by example provided by Heikki. Backpatch to 17, where the issue was introduced. Reported-by: Heikki Linnakangas Backpatch-through: 17 Discussion: https://postgr.es/m/2bf9d973-7789-4937-a7ca-0af9fb49c...@iki.fi --- src/backend/access/table/tableam.c | 3 +++ src/test/regress/expected/stats.out | 17 +++++++++++++++++ src/test/regress/sql/stats.sql | 16 ++++++++++++++++ 3 files changed, 36 insertions(+) diff --git a/src/backend/access/table/tableam.c b/src/backend/access/table/tableam.c index e18a8f8250f..a56c5eceb14 100644 --- a/src/backend/access/table/tableam.c +++ b/src/backend/access/table/tableam.c @@ -24,6 +24,7 @@ #include "access/syncscan.h" #include "access/tableam.h" #include "access/xact.h" +#include "optimizer/optimizer.h" #include "optimizer/plancat.h" #include "port/pg_bitutils.h" #include "storage/bufmgr.h" @@ -740,6 +741,8 @@ table_block_relation_estimate_size(Relation rel, int32 *attr_widths, tuple_width += overhead_bytes_per_tuple; /* note: integer division is intentional here */ density = (usable_bytes_per_page * fillfactor / 100) / tuple_width; + /* There's at least one row on the page, even with low fillfactor. */ + density = clamp_row_est(density); } *tuples = rint(density * (double) curpages); diff --git a/src/test/regress/expected/stats.out b/src/test/regress/expected/stats.out index 7d91f047bb3..093e6368dbb 100644 --- a/src/test/regress/expected/stats.out +++ b/src/test/regress/expected/stats.out @@ -1749,4 +1749,21 @@ SELECT COUNT(*) FROM brin_hot_3 WHERE a = 2; DROP TABLE brin_hot_3; SET enable_seqscan = on; +-- Test that estimation of relation size works with tuples wider than the +-- relation fillfactor. We create a table with wide inline attributes and +-- low fillfactor, insert rows and then see how many rows EXPLAIN shows +-- before running analyze. We disable autovacuum so that it does not +-- interfere with the test. +CREATE TABLE table_fillfactor ( + n char(1000) +) with (fillfactor=10, autovacuum_enabled=off); +INSERT INTO table_fillfactor +SELECT 'x' FROM generate_series(1,1000); +SELECT * FROM check_estimated_rows('SELECT * FROM table_fillfactor'); + estimated | actual +-----------+-------- + 1000 | 1000 +(1 row) + +DROP TABLE table_fillfactor; -- End of Stats Test diff --git a/src/test/regress/sql/stats.sql b/src/test/regress/sql/stats.sql index 11628ebc8a1..0a44e14d9f4 100644 --- a/src/test/regress/sql/stats.sql +++ b/src/test/regress/sql/stats.sql @@ -895,4 +895,20 @@ DROP TABLE brin_hot_3; SET enable_seqscan = on; +-- Test that estimation of relation size works with tuples wider than the +-- relation fillfactor. We create a table with wide inline attributes and +-- low fillfactor, insert rows and then see how many rows EXPLAIN shows +-- before running analyze. We disable autovacuum so that it does not +-- interfere with the test. +CREATE TABLE table_fillfactor ( + n char(1000) +) with (fillfactor=10, autovacuum_enabled=off); + +INSERT INTO table_fillfactor +SELECT 'x' FROM generate_series(1,1000); + +SELECT * FROM check_estimated_rows('SELECT * FROM table_fillfactor'); + +DROP TABLE table_fillfactor; + -- End of Stats Test -- 2.48.1
