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. regards -- Tomas Vondra
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 9a02481ee7e..4eacc79a9a8 100644 --- a/src/test/regress/expected/stats.out +++ b/src/test/regress/expected/stats.out @@ -1776,4 +1776,41 @@ 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 ( + n1 name, + n2 name, + n3 name, + n4 name, + n5 name, + n6 name, + n7 name, + n8 name, + n9 name, + n10 name, + n11 name, + n12 name, + n13 name, + n14 name, + n15 name, + n16 name, + n17 name, + n18 name, + n19 name, + n20 name) with (fillfactor=10, autovacuum_enabled=off); +INSERT INTO table_fillfactor +SELECT 'a', 'a', 'a', 'a', 'a', 'a', 'a', 'a', 'a', 'a', + 'a', 'a', 'a', 'a', 'a', 'a', 'a', 'a', 'a', 'a' +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 901e7bd56e3..88d0d8a095c 100644 --- a/src/test/regress/sql/stats.sql +++ b/src/test/regress/sql/stats.sql @@ -909,4 +909,40 @@ 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 ( + n1 name, + n2 name, + n3 name, + n4 name, + n5 name, + n6 name, + n7 name, + n8 name, + n9 name, + n10 name, + n11 name, + n12 name, + n13 name, + n14 name, + n15 name, + n16 name, + n17 name, + n18 name, + n19 name, + n20 name) with (fillfactor=10, autovacuum_enabled=off); + +INSERT INTO table_fillfactor +SELECT 'a', 'a', 'a', 'a', 'a', 'a', 'a', 'a', 'a', 'a', + 'a', 'a', 'a', 'a', 'a', 'a', 'a', 'a', 'a', 'a' +FROM generate_series(1,1000); + +SELECT * FROM check_estimated_rows('SELECT * FROM table_fillfactor'); + +DROP TABLE table_fillfactor; + -- End of Stats Test