pgsql: Fix incorrect permissions-checking code for extended statistics.

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: pgsql-committers(at)lists(dot)postgresql(dot)org
Subject: pgsql: Fix incorrect permissions-checking code for extended statistics.
Date: 2022-08-05 16:46:58
Message-ID: E1oK0TS-000S5k-FQ@gemulon.postgresql.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers

Fix incorrect permissions-checking code for extended statistics.

Commit a4d75c86b improved the extended-stats logic to allow extended
stats to be collected on expressions not just bare Vars. To apply
such stats, we first verify that the user has permissions to read all
columns used in the stats. (If not, the query will likely fail at
runtime, but the planner ought not do so.) That had to get extended
to check permissions of columns appearing within such expressions,
but the code for that was completely wrong: it applied pull_varattnos
to the wrong pointer, leading to "unrecognized node type" failures.
Furthermore, although you couldn't get to this because of that bug,
it failed to account for the attnum offset applied by pull_varattnos.

This escaped recognition so far because the code in question is not
reached when the user has whole-table SELECT privilege (which is the
common case), and because only subexpressions not specially handled
by statext_is_compatible_clause_internal() are at risk.

I think a large part of the reason for this bug is under-documentation
of what statext_is_compatible_clause() is doing and what its arguments
are, so do some work on the comments to try to improve that.

Per bug #17570 from Alexander Kozhemyakin. Patch by Richard Guo;
comments and other cosmetic improvements by me. (Thanks also to
Japin Li for diagnosis.) Back-patch to v14 where the bug came in.

Discussion: https://postgr.es/m/17570-f2f2e0f4bccf0965@postgresql.org

Branch
------
REL_14_STABLE

Details
-------
https://git.postgresql.org/pg/commitdiff/7c6ce0475a344d1f35b500898768530d0c02030a

Modified Files
--------------
src/backend/statistics/extended_stats.c | 124 ++++++++++++++++++++++----------
src/test/regress/expected/stats_ext.out | 4 ++
src/test/regress/sql/stats_ext.sql | 4 ++
3 files changed, 94 insertions(+), 38 deletions(-)

Browse pgsql-committers by date

  From Date Subject
Next Message Alvaro Herrera 2022-08-05 17:38:06 pgsql: Backpatch addition of .git-blame-ignore-revs
Previous Message Alvaro Herrera 2022-08-05 16:10:43 Re: pgsql: BRIN: mask BRIN_EVACUATE_PAGE for WAL consistency checking