From e75f534d4662a4c0ba3ee0518c08b3692cdc0255 Mon Sep 17 00:00:00 2001 From: Corey Huinker Date: Thu, 21 Aug 2025 12:30:17 -0400 Subject: [PATCH v3 1/1] Have missing-stats query use security barrier views. Previously, the missing-stats query used pg_statistic and pg_statistic_ext_data, which meant that the queries would fail for non-superusers like pg_maintain as reported by Fujii Masao. Because the security barrier views will obscure certain statistics from the user, it is important that each EXISTS() test also apply the same filter in generating the list of attributes and extended stats to avoid false positives. This unfortunately means that we do not know if the columns the user can't see have stats or not, but the alternative is false positives. --- doc/src/sgml/ref/vacuumdb.sgml | 7 +++ src/bin/scripts/t/100_vacuumdb.pl | 96 +++++++++++++++++++++++++++++++ src/bin/scripts/vacuumdb.c | 62 ++++++++++++++------ 3 files changed, 147 insertions(+), 18 deletions(-) diff --git a/doc/src/sgml/ref/vacuumdb.sgml b/doc/src/sgml/ref/vacuumdb.sgml index 53147480515..f797abc273a 100644 --- a/doc/src/sgml/ref/vacuumdb.sgml +++ b/doc/src/sgml/ref/vacuumdb.sgml @@ -292,6 +292,13 @@ PostgreSQL documentation This option can only be used in conjunction with or . + + Note that only considers + statistics for tables the user has permission to read and extended + statistics for tables the user owns; if the user lacks those + privileges, vacuumdb may choose not to + analyze a relation even if it is missing some statistics. + diff --git a/src/bin/scripts/t/100_vacuumdb.pl b/src/bin/scripts/t/100_vacuumdb.pl index 945c30df156..3ee089f8a9f 100644 --- a/src/bin/scripts/t/100_vacuumdb.pl +++ b/src/bin/scripts/t/100_vacuumdb.pl @@ -240,7 +240,29 @@ $node->command_fails_like( $node->safe_psql('postgres', q| CREATE TABLE regression_vacuumdb_test AS select generate_series(1, 10) a, generate_series(2, 11) b; ALTER TABLE regression_vacuumdb_test ADD COLUMN c INT GENERATED ALWAYS AS (a + b); + ALTER TABLE regression_vacuumdb_test ENABLE ROW LEVEL SECURITY; + CREATE ROLE regression_noprivs LOGIN BYPASSRLS; + CREATE ROLE regression_rls LOGIN; + GRANT SELECT ON regression_vacuumdb_test TO regression_rls; |); +$node->issues_sql_unlike( + [ + 'vacuumdb', '--analyze-only', + '--missing-stats-only', '-t', + 'regression_vacuumdb_test', 'postgres', + '--username', 'regression_noprivs' + ], + qr/statement:\ ANALYZE/sx, + '--missing-stats-only with missing stats and unprivileged role'); +$node->issues_sql_unlike( + [ + 'vacuumdb', '--analyze-only', + '--missing-stats-only', '-t', + 'regression_vacuumdb_test', 'postgres', + '--username', 'regression_rls' + ], + qr/statement:\ ANALYZE/sx, + '--missing-stats-only with missing stats and role subject to RLS'); $node->issues_sql_like( [ 'vacuumdb', '--analyze-only', @@ -261,6 +283,24 @@ $node->issues_sql_unlike( $node->safe_psql('postgres', 'CREATE INDEX regression_vacuumdb_test_idx ON regression_vacuumdb_test (mod(a, 2));' ); +$node->issues_sql_unlike( + [ + 'vacuumdb', '--analyze-in-stages', + '--missing-stats-only', '-t', + 'regression_vacuumdb_test', 'postgres', + '--username', 'regression_noprivs' + ], + qr/statement:\ ANALYZE/sx, + '--missing-stats-only with missing index expression stats and unprivileged role'); +$node->issues_sql_unlike( + [ + 'vacuumdb', '--analyze-in-stages', + '--missing-stats-only', '-t', + 'regression_vacuumdb_test', 'postgres', + '--username', 'regression_rls' + ], + qr/statement:\ ANALYZE/sx, + '--missing-stats-only with missing index expression stats and role subject to RLS'); $node->issues_sql_like( [ 'vacuumdb', '--analyze-in-stages', @@ -281,6 +321,24 @@ $node->issues_sql_unlike( $node->safe_psql('postgres', 'CREATE STATISTICS regression_vacuumdb_test_stat ON a, b FROM regression_vacuumdb_test;' ); +$node->issues_sql_unlike( + [ + 'vacuumdb', '--analyze-only', + '--missing-stats-only', '-t', + 'regression_vacuumdb_test', 'postgres', + '--username', 'regression_noprivs' + ], + qr/statement:\ ANALYZE/sx, + '--missing-stats-only with missing extended stats and unprivileged role'); +$node->issues_sql_unlike( + [ + 'vacuumdb', '--analyze-only', + '--missing-stats-only', '-t', + 'regression_vacuumdb_test', 'postgres', + '--username', 'regression_rls' + ], + qr/statement:\ ANALYZE/sx, + '--missing-stats-only with missing extended stats and role subject to RLS'); $node->issues_sql_like( [ 'vacuumdb', '--analyze-only', @@ -302,6 +360,24 @@ $node->safe_psql('postgres', "CREATE TABLE regression_vacuumdb_child (a INT) INHERITS (regression_vacuumdb_test);\n" . "INSERT INTO regression_vacuumdb_child VALUES (1, 2);\n" . "ANALYZE regression_vacuumdb_child;\n"); +$node->issues_sql_unlike( + [ + 'vacuumdb', '--analyze-in-stages', + '--missing-stats-only', '-t', + 'regression_vacuumdb_test', 'postgres', + '--username', 'regression_noprivs' + ], + qr/statement:\ ANALYZE/sx, + '--missing-stats-only with missing inherited stats and unprivileged role'); +$node->issues_sql_unlike( + [ + 'vacuumdb', '--analyze-in-stages', + '--missing-stats-only', '-t', + 'regression_vacuumdb_test', 'postgres', + '--username', 'regression_rls' + ], + qr/statement:\ ANALYZE/sx, + '--missing-stats-only with missing inherited stats and role subject to RLS'); $node->issues_sql_like( [ 'vacuumdb', '--analyze-in-stages', @@ -321,9 +397,29 @@ $node->issues_sql_unlike( $node->safe_psql('postgres', "CREATE TABLE regression_vacuumdb_parted (a INT) PARTITION BY LIST (a);\n" + . "ALTER TABLE regression_vacuumdb_parted ENABLE ROW LEVEL SECURITY;\n" + . "GRANT SELECT ON regression_vacuumdb_parted TO regression_rls;\n" . "CREATE TABLE regression_vacuumdb_part1 PARTITION OF regression_vacuumdb_parted FOR VALUES IN (1);\n" . "INSERT INTO regression_vacuumdb_parted VALUES (1);\n" . "ANALYZE regression_vacuumdb_part1;\n"); +$node->issues_sql_unlike( + [ + 'vacuumdb', '--analyze-only', + '--missing-stats-only', '-t', + 'regression_vacuumdb_parted', 'postgres', + '--username', 'regression_noprivs' + ], + qr/statement:\ ANALYZE/sx, + '--missing-stats-only with missing partition stats and unprivileged role'); +$node->issues_sql_unlike( + [ + 'vacuumdb', '--analyze-only', + '--missing-stats-only', '-t', + 'regression_vacuumdb_parted', 'postgres', + '--username', 'regression_rls' + ], + qr/statement:\ ANALYZE/sx, + '--missing-stats-only with missing partition stats and row subject to RLS'); $node->issues_sql_like( [ 'vacuumdb', '--analyze-only', diff --git a/src/bin/scripts/vacuumdb.c b/src/bin/scripts/vacuumdb.c index fd236087e90..244c0a885e9 100644 --- a/src/bin/scripts/vacuumdb.c +++ b/src/bin/scripts/vacuumdb.c @@ -973,38 +973,54 @@ retrieve_objects(PGconn *conn, vacuumingOptions *vacopts, " WHERE a.attrelid OPERATOR(pg_catalog.=) c.oid\n" " AND a.attnum OPERATOR(pg_catalog.>) 0::pg_catalog.int2\n" " AND NOT a.attisdropped\n" + " AND pg_catalog.has_column_privilege(c.oid, a.attnum, 'select'::pg_catalog.text)\n" + " AND (c.relrowsecurity OPERATOR(pg_catalog.=) false\n" + " OR NOT pg_catalog.row_security_active(c.oid))\n" " AND a.attstattarget IS DISTINCT FROM 0::pg_catalog.int2\n" " AND a.attgenerated OPERATOR(pg_catalog.<>) " CppAsString2(ATTRIBUTE_GENERATED_VIRTUAL) "\n" - " AND NOT EXISTS (SELECT NULL FROM pg_catalog.pg_statistic s\n" - " WHERE s.starelid OPERATOR(pg_catalog.=) a.attrelid\n" - " AND s.staattnum OPERATOR(pg_catalog.=) a.attnum\n" - " AND s.stainherit OPERATOR(pg_catalog.=) p.inherited))\n"); + " AND NOT EXISTS (SELECT NULL FROM pg_catalog.pg_stats s\n" + " WHERE s.schemaname OPERATOR(pg_catalog.=) ns.nspname\n" + " AND s.tablename OPERATOR(pg_catalog.=) c.relname\n" + " AND s.attname OPERATOR(pg_catalog.=) a.attname\n" + " AND s.inherited OPERATOR(pg_catalog.=) p.inherited))\n"); /* extended stats */ appendPQExpBufferStr(&catalog_query, " OR EXISTS (SELECT NULL FROM pg_catalog.pg_statistic_ext e\n" + " JOIN pg_catalog.pg_namespace en" + " ON en.oid OPERATOR(pg_catalog.=) e.stxnamespace\n" " WHERE e.stxrelid OPERATOR(pg_catalog.=) c.oid\n" + " AND pg_catalog.pg_has_role(c.relowner, 'USAGE'::text)\n" + " AND (c.relrowsecurity OPERATOR(pg_catalog.=) false\n" + " OR NOT pg_catalog.row_security_active(c.oid))\n" " AND e.stxstattarget IS DISTINCT FROM 0::pg_catalog.int2\n" - " AND NOT EXISTS (SELECT NULL FROM pg_catalog.pg_statistic_ext_data d\n" - " WHERE d.stxoid OPERATOR(pg_catalog.=) e.oid\n" - " AND d.stxdinherit OPERATOR(pg_catalog.=) p.inherited))\n"); + " AND NOT EXISTS (SELECT NULL FROM pg_catalog.pg_stats_ext d\n" + " WHERE d.statistics_schemaname OPERATOR(pg_catalog.=) en.nspname\n" + " AND d.statistics_name OPERATOR(pg_catalog.=) e.stxname\n" + " AND d.inherited OPERATOR(pg_catalog.=) p.inherited))\n"); /* expression indexes */ appendPQExpBufferStr(&catalog_query, " OR EXISTS (SELECT NULL FROM pg_catalog.pg_attribute a\n" " JOIN pg_catalog.pg_index i" " ON i.indexrelid OPERATOR(pg_catalog.=) a.attrelid\n" + " JOIN pg_catalog.pg_class ic" + " ON ic.oid OPERATOR(pg_catalog.=) i.indexrelid\n" " WHERE i.indrelid OPERATOR(pg_catalog.=) c.oid\n" " AND i.indkey[a.attnum OPERATOR(pg_catalog.-) 1::pg_catalog.int2]" " OPERATOR(pg_catalog.=) 0::pg_catalog.int2\n" " AND a.attnum OPERATOR(pg_catalog.>) 0::pg_catalog.int2\n" " AND NOT a.attisdropped\n" + " AND pg_catalog.has_column_privilege(ic.oid, a.attnum, 'select'::pg_catalog.text)\n" + " AND (ic.relrowsecurity OPERATOR(pg_catalog.=) false\n" + " OR NOT pg_catalog.row_security_active(ic.oid))\n" " AND a.attstattarget IS DISTINCT FROM 0::pg_catalog.int2\n" - " AND NOT EXISTS (SELECT NULL FROM pg_catalog.pg_statistic s\n" - " WHERE s.starelid OPERATOR(pg_catalog.=) a.attrelid\n" - " AND s.staattnum OPERATOR(pg_catalog.=) a.attnum\n" - " AND s.stainherit OPERATOR(pg_catalog.=) p.inherited))\n"); + " AND NOT EXISTS (SELECT NULL FROM pg_catalog.pg_stats s\n" + " WHERE s.schemaname OPERATOR(pg_catalog.=) ns.nspname\n" + " AND s.tablename OPERATOR(pg_catalog.=) ic.relname\n" + " AND s.attname OPERATOR(pg_catalog.=) a.attname\n" + " AND s.inherited OPERATOR(pg_catalog.=) p.inherited))\n"); /* inheritance and regular stats */ appendPQExpBufferStr(&catalog_query, @@ -1017,25 +1033,35 @@ retrieve_objects(PGconn *conn, vacuumingOptions *vacopts, CppAsString2(ATTRIBUTE_GENERATED_VIRTUAL) "\n" " AND c.relhassubclass\n" " AND NOT p.inherited\n" + " AND pg_catalog.has_column_privilege(c.oid, a.attnum, 'select'::pg_catalog.text)\n" + " AND (c.relrowsecurity OPERATOR(pg_catalog.=) false\n" + " OR NOT pg_catalog.row_security_active(c.oid))\n" " AND EXISTS (SELECT NULL FROM pg_catalog.pg_inherits h\n" " WHERE h.inhparent OPERATOR(pg_catalog.=) c.oid)\n" - " AND NOT EXISTS (SELECT NULL FROM pg_catalog.pg_statistic s\n" - " WHERE s.starelid OPERATOR(pg_catalog.=) a.attrelid\n" - " AND s.staattnum OPERATOR(pg_catalog.=) a.attnum\n" - " AND s.stainherit))\n"); + " AND NOT EXISTS (SELECT NULL FROM pg_catalog.pg_stats s\n" + " WHERE s.schemaname OPERATOR(pg_catalog.=) ns.nspname\n" + " AND s.tablename OPERATOR(pg_catalog.=) c.relname\n" + " AND s.attname OPERATOR(pg_catalog.=) a.attname\n" + " AND s.inherited))\n"); /* inheritance and extended stats */ appendPQExpBufferStr(&catalog_query, " OR EXISTS (SELECT NULL FROM pg_catalog.pg_statistic_ext e\n" + " JOIN pg_catalog.pg_namespace en" + " ON en.oid OPERATOR(pg_catalog.=) e.stxnamespace\n" " WHERE e.stxrelid OPERATOR(pg_catalog.=) c.oid\n" " AND e.stxstattarget IS DISTINCT FROM 0::pg_catalog.int2\n" " AND c.relhassubclass\n" " AND NOT p.inherited\n" + " AND pg_catalog.pg_has_role(c.relowner, 'USAGE'::text)\n" + " AND (c.relrowsecurity OPERATOR(pg_catalog.=) false\n" + " OR NOT pg_catalog.row_security_active(c.oid))\n" " AND EXISTS (SELECT NULL FROM pg_catalog.pg_inherits h\n" " WHERE h.inhparent OPERATOR(pg_catalog.=) c.oid)\n" - " AND NOT EXISTS (SELECT NULL FROM pg_catalog.pg_statistic_ext_data d\n" - " WHERE d.stxoid OPERATOR(pg_catalog.=) e.oid\n" - " AND d.stxdinherit))\n"); + " AND NOT EXISTS (SELECT NULL FROM pg_catalog.pg_stats_ext d\n" + " WHERE d.statistics_schemaname OPERATOR(pg_catalog.=) en.nspname\n" + " AND d.statistics_name OPERATOR(pg_catalog.=) e.stxname\n" + " AND d.inherited))\n"); appendPQExpBufferStr(&catalog_query, " )\n"); } -- 2.39.5 (Apple Git-154)