pg.ddx.io pgsql-bugs@postgresql.org mailing list archive
help / color / mirror / Atom feedBUG #17570: Unrecognized node type for query with statistics on expressions
10+ messages / 4 participants
[nested] [flat]
Commits from this thread:
2022-08-05 e5fc38ac30 7394028bea 7c6ce0475a Fix incorrect permissions-checking code for extended statistics.
* BUG #17570: Unrecognized node type for query with statistics on expressions
@ 2022-08-04 08:27 PG Bug reporting form <noreply@postgresql.org>
0 siblings, 2 replies; 10+ messages in thread
From: PG Bug reporting form @ 2022-08-04 08:27 UTC (permalink / raw)
To: pgsql-bugs@lists.postgresql.org; +Cc: a.kozhemyakin@postgrespro.ru
The following bug has been logged on the website:
Bug reference: 17570
Logged by: Alexander Kozhemyakin
Email address: a.kozhemyakin@postgrespro.ru
PostgreSQL version: 14.4
Operating system: ubunu 20.04
Description:
When executing the following script:
CREATE TABLE t(a INT, b VARCHAR);
INSERT INTO t(a, b) SELECT i, i FROM generate_series(1,100) s(i);
CREATE STATISTICS t_stats (mcv) ON (a), (b) FROM t;
ANALYZE t;
CREATE ROLE u;
--GRANT SELECT ON t TO u;
SET SESSION AUTHORIZATION u;
SELECT * FROM t WHERE a = 1 AND b::int = 1;
I get:
ERROR: unrecognized node type: 1697192808
And it fails with error "permission denied for table t" when condition is
just "a = 1" or "b::int = 1".
The same query executed successfully, if the select permission is granted to
user.
The first bad commit is a4d75c86b
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: BUG #17570: Unrecognized node type for query with statistics on expressions
@ 2022-08-04 10:33 Japin Li <japinli@hotmail.com>
parent: PG Bug reporting form <noreply@postgresql.org>
1 sibling, 0 replies; 10+ messages in thread
From: Japin Li @ 2022-08-04 10:33 UTC (permalink / raw)
To: a.kozhemyakin@postgrespro.ru; pgsql-bugs@lists.postgresql.org
On Thu, 04 Aug 2022 at 16:27, PG Bug reporting form <noreply@postgresql.org> wrote:
> The following bug has been logged on the website:
>
> Bug reference: 17570
> Logged by: Alexander Kozhemyakin
> Email address: a.kozhemyakin@postgrespro.ru
> PostgreSQL version: 14.4
> Operating system: ubunu 20.04
> Description:
>
> When executing the following script:
> CREATE TABLE t(a INT, b VARCHAR);
> INSERT INTO t(a, b) SELECT i, i FROM generate_series(1,100) s(i);
> CREATE STATISTICS t_stats (mcv) ON (a), (b) FROM t;
> ANALYZE t;
> CREATE ROLE u;
> --GRANT SELECT ON t TO u;
> SET SESSION AUTHORIZATION u;
> SELECT * FROM t WHERE a = 1 AND b::int = 1;
>
> I get:
> ERROR: unrecognized node type: 1697192808
>
> And it fails with error "permission denied for table t" when condition is
> just "a = 1" or "b::int = 1".
>
> The same query executed successfully, if the select permission is granted to
> user.
> The first bad commit is a4d75c86b
Yeah, it is a bug.
When we don't have table privilege, we check individual columns, however, it
passes a wrong pointer to pull_varattnos(), which leads to this error.
There has another bug when checking the attribute privilege, since the bitmap
contains the offset by FirstLowInvalidHeapAttributeNumber.
Attach a patch to fix it. Any thoughts?
--
Regrads,
Japin Li.
ChengDu WenWu Information Technology Co.,Ltd.
Attachments:
fix-unrecognized-node-type-for-query-with-statistics.diff (text/x-patch, 1.0K) download
diff --git a/src/backend/statistics/extended_stats.c b/src/backend/statistics/extended_stats.c
index d2aa8d0ca3..93ba333ddc 100644
--- a/src/backend/statistics/extended_stats.c
+++ b/src/backend/statistics/extended_stats.c
@@ -1619,7 +1619,7 @@ statext_is_compatible_clause(PlannerInfo *root, Node *clause, Index relid,
/* Don't have table privilege, must check individual columns */
if (*exprs != NIL)
{
- pull_varattnos((Node *) exprs, relid, &clause_attnums);
+ pull_varattnos((Node *) *exprs, relid, &clause_attnums);
clause_attnums = bms_add_members(clause_attnums, *attnums);
}
else
@@ -1639,6 +1639,9 @@ statext_is_compatible_clause(PlannerInfo *root, Node *clause, Index relid,
while ((attnum = bms_next_member(clause_attnums, attnum)) >= 0)
{
+ /* bit #s are offset by FirstLowInvalidHeapAttributeNumber */
+ attnum = attnum + FirstLowInvalidHeapAttributeNumber;
+
if (pg_attribute_aclcheck(rte->relid, attnum, userid,
ACL_SELECT) != ACLCHECK_OK)
return false;
* Re: BUG #17570: Unrecognized node type for query with statistics on expressions
@ 2022-08-04 10:37 Richard Guo <guofenglinux@gmail.com>
parent: PG Bug reporting form <noreply@postgresql.org>
1 sibling, 1 reply; 10+ messages in thread
From: Richard Guo @ 2022-08-04 10:37 UTC (permalink / raw)
To: a.kozhemyakin@postgrespro.ru, PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
On Thu, Aug 4, 2022 at 4:28 PM PG Bug reporting form <noreply@postgresql.org>
wrote:
> The following bug has been logged on the website:
>
> Bug reference: 17570
> Logged by: Alexander Kozhemyakin
> Email address: a.kozhemyakin@postgrespro.ru
> PostgreSQL version: 14.4
> Operating system: ubunu 20.04
> Description:
>
> When executing the following script:
> CREATE TABLE t(a INT, b VARCHAR);
> INSERT INTO t(a, b) SELECT i, i FROM generate_series(1,100) s(i);
> CREATE STATISTICS t_stats (mcv) ON (a), (b) FROM t;
> ANALYZE t;
> CREATE ROLE u;
> --GRANT SELECT ON t TO u;
> SET SESSION AUTHORIZATION u;
> SELECT * FROM t WHERE a = 1 AND b::int = 1;
>
> I get:
> ERROR: unrecognized node type: 1697192808
Thanks for the report! I can reproduce it in HEAD. Propose the attached
for fix.
Thanks
Richard
Attachments:
v1-0001-fix-bug-17570.patch (application/octet-stream, 2.1K) download
From cbc0550b2c2fa75cd3ec6123e5af96928959ad39 Mon Sep 17 00:00:00 2001
From: Richard Guo <guofenglinux@gmail.com>
Date: Thu, 4 Aug 2022 18:29:12 +0800
Subject: [PATCH v1] fix bug 17570
---
src/backend/statistics/extended_stats.c | 27 +++++++++++++------------
1 file changed, 14 insertions(+), 13 deletions(-)
diff --git a/src/backend/statistics/extended_stats.c b/src/backend/statistics/extended_stats.c
index d2aa8d0ca3..c7f23edf2a 100644
--- a/src/backend/statistics/extended_stats.c
+++ b/src/backend/statistics/extended_stats.c
@@ -1615,31 +1615,32 @@ statext_is_compatible_clause(PlannerInfo *root, Node *clause, Index relid,
if (pg_class_aclcheck(rte->relid, userid, ACL_SELECT) != ACLCHECK_OK)
{
Bitmapset *clause_attnums = NULL;
+ int attnum = -1;
/* Don't have table privilege, must check individual columns */
if (*exprs != NIL)
{
- pull_varattnos((Node *) exprs, relid, &clause_attnums);
+ pull_varattnos((Node *) *exprs, relid, &clause_attnums);
clause_attnums = bms_add_members(clause_attnums, *attnums);
}
else
clause_attnums = *attnums;
- if (bms_is_member(InvalidAttrNumber, clause_attnums))
+ while ((attnum = bms_next_member(clause_attnums, attnum)) >= 0)
{
- /* Have a whole-row reference, must have access to all columns */
- if (pg_attribute_aclcheck_all(rte->relid, userid, ACL_SELECT,
- ACLMASK_ALL) != ACLCHECK_OK)
- return false;
- }
- else
- {
- /* Check the columns referenced by the clause */
- int attnum = -1;
+ /* attnum is offset by FirstLowInvalidHeapAttributeNumber */
+ AttrNumber attno = attnum + FirstLowInvalidHeapAttributeNumber;
- while ((attnum = bms_next_member(clause_attnums, attnum)) >= 0)
+ if (attno == InvalidAttrNumber)
+ {
+ /* Have a whole-row reference, must have access to all columns */
+ if (pg_attribute_aclcheck_all(rte->relid, userid, ACL_SELECT,
+ ACLMASK_ALL) != ACLCHECK_OK)
+ return false;
+ }
+ else
{
- if (pg_attribute_aclcheck(rte->relid, attnum, userid,
+ if (pg_attribute_aclcheck(rte->relid, attno, userid,
ACL_SELECT) != ACLCHECK_OK)
return false;
}
--
2.31.0
* Re: BUG #17570: Unrecognized node type for query with statistics on expressions
@ 2022-08-04 11:25 Richard Guo <guofenglinux@gmail.com>
parent: Richard Guo <guofenglinux@gmail.com>
0 siblings, 1 reply; 10+ messages in thread
From: Richard Guo @ 2022-08-04 11:25 UTC (permalink / raw)
To: a.kozhemyakin@postgrespro.ru, PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
On Thu, Aug 4, 2022 at 6:37 PM Richard Guo <guofenglinux@gmail.com> wrote:
>
> On Thu, Aug 4, 2022 at 4:28 PM PG Bug reporting form <
> noreply@postgresql.org> wrote:
>
>> The following bug has been logged on the website:
>>
>> Bug reference: 17570
>> Logged by: Alexander Kozhemyakin
>> Email address: a.kozhemyakin@postgrespro.ru
>> PostgreSQL version: 14.4
>> Operating system: ubunu 20.04
>> Description:
>>
>> When executing the following script:
>> CREATE TABLE t(a INT, b VARCHAR);
>> INSERT INTO t(a, b) SELECT i, i FROM generate_series(1,100) s(i);
>> CREATE STATISTICS t_stats (mcv) ON (a), (b) FROM t;
>> ANALYZE t;
>> CREATE ROLE u;
>> --GRANT SELECT ON t TO u;
>> SET SESSION AUTHORIZATION u;
>> SELECT * FROM t WHERE a = 1 AND b::int = 1;
>>
>> I get:
>> ERROR: unrecognized node type: 1697192808
>
>
> Thanks for the report! I can reproduce it in HEAD. Propose the attached
> for fix.
>
Oops.. sorry I didn't notice Japin's email before I hit the 'send' for
my reply. The two patches are almost the same, except that before we
check against whole-row reference, I also adjust the attnum by
FirstLowInvalidHeapAttributeNumber.
Thanks
Richard
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: BUG #17570: Unrecognized node type for query with statistics on expressions
@ 2022-08-04 14:59 Japin Li <japinli@hotmail.com>
parent: Richard Guo <guofenglinux@gmail.com>
0 siblings, 1 reply; 10+ messages in thread
From: Japin Li @ 2022-08-04 14:59 UTC (permalink / raw)
To: Richard Guo <guofenglinux@gmail.com>; +Cc: a.kozhemyakin@postgrespro.ru; pgsql-bugs@lists.postgresql.org
On Thu, 04 Aug 2022 at 19:25, Richard Guo <guofenglinux@gmail.com> wrote:
> On Thu, Aug 4, 2022 at 6:37 PM Richard Guo <guofenglinux@gmail.com> wrote:
>
>>
>> On Thu, Aug 4, 2022 at 4:28 PM PG Bug reporting form <
>> noreply@postgresql.org> wrote:
>>
>>> The following bug has been logged on the website:
>>>
>>> Bug reference: 17570
>>> Logged by: Alexander Kozhemyakin
>>> Email address: a.kozhemyakin@postgrespro.ru
>>> PostgreSQL version: 14.4
>>> Operating system: ubunu 20.04
>>> Description:
>>>
>>> When executing the following script:
>>> CREATE TABLE t(a INT, b VARCHAR);
>>> INSERT INTO t(a, b) SELECT i, i FROM generate_series(1,100) s(i);
>>> CREATE STATISTICS t_stats (mcv) ON (a), (b) FROM t;
>>> ANALYZE t;
>>> CREATE ROLE u;
>>> --GRANT SELECT ON t TO u;
>>> SET SESSION AUTHORIZATION u;
>>> SELECT * FROM t WHERE a = 1 AND b::int = 1;
>>>
>>> I get:
>>> ERROR: unrecognized node type: 1697192808
>>
>>
>> Thanks for the report! I can reproduce it in HEAD. Propose the attached
>> for fix.
>>
>
> Oops.. sorry I didn't notice Japin's email before I hit the 'send' for
> my reply. The two patches are almost the same, except that before we
> check against whole-row reference, I also adjust the attnum by
> FirstLowInvalidHeapAttributeNumber.
>
Doesn't matter.
Attached to new patch add a test-case from Alexander Kozhemyakin.
--
Regrads,
Japin Li.
ChengDu WenWu Information Technology Co.,Ltd.
Attachments:
v1-0001-Fix-unrecognized-node-type-about-statistics-on-ex.patch (text/x-patch, 0B) download
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: BUG #17570: Unrecognized node type for query with statistics on expressions
@ 2022-08-04 15:42 Tom Lane <tgl@sss.pgh.pa.us>
parent: Japin Li <japinli@hotmail.com>
0 siblings, 2 replies; 10+ messages in thread
From: Tom Lane @ 2022-08-04 15:42 UTC (permalink / raw)
To: Japin Li <japinli@hotmail.com>; +Cc: Tomas Vondra <tomas.vondra@enterprisedb.com>; Richard Guo <guofenglinux@gmail.com>; a.kozhemyakin@postgrespro.ru; pgsql-bugs@lists.postgresql.org
Japin Li <japinli@hotmail.com> writes:
> Attached to new patch add a test-case from Alexander Kozhemyakin.
It appears to me that this is actually very thoroughly broken.
* statext_is_compatible_clause_internal is not on board with
offsetting the attnums.
* Neither is this check for a whole-row reference (line 1628, in HEAD):
if (bms_is_member(InvalidAttrNumber, clause_attnums))
* Although the coverage report claims this code is exercised, the
fact that nothing failed when you made only a partial fix says it's
not exercised well enough.
* The comments for statext_is_compatible_clause suck. If they'd
defined what the arguments are, perhaps this mess would have been
prevented. For extra credit, it'd be really nice to define what
"is compatible" means. I'd sure not have thought that that would
include permissions checks.
regards, tom lane
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: BUG #17570: Unrecognized node type for query with statistics on expressions
@ 2022-08-05 00:49 Richard Guo <guofenglinux@gmail.com>
parent: Tom Lane <tgl@sss.pgh.pa.us>
1 sibling, 1 reply; 10+ messages in thread
From: Richard Guo @ 2022-08-05 00:49 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Japin Li <japinli@hotmail.com>; Tomas Vondra <tomas.vondra@enterprisedb.com>; a.kozhemyakin@postgrespro.ru, PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
On Thu, Aug 4, 2022 at 11:42 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> * statext_is_compatible_clause_internal is not on board with
> offsetting the attnums.
Correct. Good catch! Fix this in v2 patch.
> * Neither is this check for a whole-row reference (line 1628, in HEAD):
>
> if (bms_is_member(InvalidAttrNumber, clause_attnums))
Actually this has been fixed in my v1 patch [1].
> * Although the coverage report claims this code is exercised, the
> fact that nothing failed when you made only a partial fix says it's
> not exercised well enough.
That's true. We need to provide a better test case to cover this.
>
> * The comments for statext_is_compatible_clause suck. If they'd
> defined what the arguments are, perhaps this mess would have been
> prevented. For extra credit, it'd be really nice to define what
> "is compatible" means. I'd sure not have thought that that would
> include permissions checks.
>
Concur with that.
[1]:
https://www.postgresql.org/message-id/CAMbWs4_PPrJ7JFXqewQfkMLP6-2kHi%2B5%3D11U0csQVTx7iq7esw%40mail...
Thanks
Richard
Attachments:
v2-0001-fix-bug-17570.patch (application/octet-stream, 2.5K) download
From 81574ca50b4702b1450f995ecbd4a2ea472a73b0 Mon Sep 17 00:00:00 2001
From: Richard Guo <guofenglinux@gmail.com>
Date: Thu, 4 Aug 2022 18:29:12 +0800
Subject: [PATCH v2] fix bug 17570
---
src/backend/statistics/extended_stats.c | 31 ++++++++++++++-----------
1 file changed, 17 insertions(+), 14 deletions(-)
diff --git a/src/backend/statistics/extended_stats.c b/src/backend/statistics/extended_stats.c
index d2aa8d0ca3..f200751e3a 100644
--- a/src/backend/statistics/extended_stats.c
+++ b/src/backend/statistics/extended_stats.c
@@ -1347,7 +1347,9 @@ statext_is_compatible_clause_internal(PlannerInfo *root, Node *clause,
if (!AttrNumberIsForUserDefinedAttr(var->varattno))
return false;
- *attnums = bms_add_member(*attnums, var->varattno);
+ *attnums =
+ bms_add_member(*attnums,
+ var->varattno - FirstLowInvalidHeapAttributeNumber);
return true;
}
@@ -1615,31 +1617,32 @@ statext_is_compatible_clause(PlannerInfo *root, Node *clause, Index relid,
if (pg_class_aclcheck(rte->relid, userid, ACL_SELECT) != ACLCHECK_OK)
{
Bitmapset *clause_attnums = NULL;
+ int attnum = -1;
/* Don't have table privilege, must check individual columns */
if (*exprs != NIL)
{
- pull_varattnos((Node *) exprs, relid, &clause_attnums);
+ pull_varattnos((Node *) *exprs, relid, &clause_attnums);
clause_attnums = bms_add_members(clause_attnums, *attnums);
}
else
clause_attnums = *attnums;
- if (bms_is_member(InvalidAttrNumber, clause_attnums))
+ while ((attnum = bms_next_member(clause_attnums, attnum)) >= 0)
{
- /* Have a whole-row reference, must have access to all columns */
- if (pg_attribute_aclcheck_all(rte->relid, userid, ACL_SELECT,
- ACLMASK_ALL) != ACLCHECK_OK)
- return false;
- }
- else
- {
- /* Check the columns referenced by the clause */
- int attnum = -1;
+ /* attnum is offset by FirstLowInvalidHeapAttributeNumber */
+ AttrNumber attno = attnum + FirstLowInvalidHeapAttributeNumber;
- while ((attnum = bms_next_member(clause_attnums, attnum)) >= 0)
+ if (attno == InvalidAttrNumber)
+ {
+ /* Have a whole-row reference, must have access to all columns */
+ if (pg_attribute_aclcheck_all(rte->relid, userid, ACL_SELECT,
+ ACLMASK_ALL) != ACLCHECK_OK)
+ return false;
+ }
+ else
{
- if (pg_attribute_aclcheck(rte->relid, attnum, userid,
+ if (pg_attribute_aclcheck(rte->relid, attno, userid,
ACL_SELECT) != ACLCHECK_OK)
return false;
}
--
2.31.0
* Re: BUG #17570: Unrecognized node type for query with statistics on expressions
@ 2022-08-05 01:25 Richard Guo <guofenglinux@gmail.com>
parent: Tom Lane <tgl@sss.pgh.pa.us>
1 sibling, 0 replies; 10+ messages in thread
From: Richard Guo @ 2022-08-05 01:25 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Japin Li <japinli@hotmail.com>; Tomas Vondra <tomas.vondra@enterprisedb.com>; a.kozhemyakin@postgrespro.ru, PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
On Thu, Aug 4, 2022 at 11:42 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> * The comments for statext_is_compatible_clause suck. If they'd
> defined what the arguments are, perhaps this mess would have been
> prevented. For extra credit, it'd be really nice to define what
> "is compatible" means. I'd sure not have thought that that would
> include permissions checks.
I think by 'compatible' it just means the clause is in the form that can
be estimated using MCV lists. Maybe we can define it as that.
In statext_is_compatible_clause() we have the permission checks to see
if we are allowed to read all required attributes, which indeed needs to
be mentioned in the function comments.
Thanks
Richard
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: BUG #17570: Unrecognized node type for query with statistics on expressions
@ 2022-08-05 09:03 Richard Guo <guofenglinux@gmail.com>
parent: Richard Guo <guofenglinux@gmail.com>
0 siblings, 1 reply; 10+ messages in thread
From: Richard Guo @ 2022-08-05 09:03 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Japin Li <japinli@hotmail.com>; Tomas Vondra <tomas.vondra@enterprisedb.com>; a.kozhemyakin@postgrespro.ru, PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
On Fri, Aug 5, 2022 at 8:49 AM Richard Guo <guofenglinux@gmail.com> wrote:
> On Thu, Aug 4, 2022 at 11:42 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
>> * statext_is_compatible_clause_internal is not on board with
>> offsetting the attnums.
>
>
> Correct. Good catch! Fix this in v2 patch.
>
I realized the fix in v2 is not correct. The attnums generated by
statext_is_compatible_clause is expected to be NOT offsetting by its
caller, i.e. statext_mcv_clauselist_selectivity. Revise that in v3 patch
by doing the offsetting when composing clause_attnums that need to have
permission check.
> * Although the coverage report claims this code is exercised, the
>> fact that nothing failed when you made only a partial fix says it's
>> not exercised well enough.
>
>
> That's true. We need to provide a better test case to cover this.
>
Also I'm trying to provide a better test case and at last come up with a
case in v3 like:
SELECT * FROM tststats.priv_test_tbl where a = 1 and
tststats.priv_test_tbl.* > (1, 1) is not null;
For this case, the clause 'a = 1' would cover that the attnum comes from
statext_is_compatible_clause_internal. And the clause 'priv_test_tbl.* >
(1, 1) is not null' would cover that the attnum comes from 'exprs' and
it has a whole-row reference.
Thanks
Richard
Attachments:
v3-0001-fix-bug-17570.patch (application/octet-stream, 4.0K) download
From 28bcf2ea5c6f7dc14deabe6ddfc2623b5e7b5bc6 Mon Sep 17 00:00:00 2001
From: Richard Guo <guofenglinux@gmail.com>
Date: Thu, 4 Aug 2022 18:29:12 +0800
Subject: [PATCH v3] fix bug 17570
---
src/backend/statistics/extended_stats.c | 38 ++++++++++++++-----------
src/test/regress/expected/stats_ext.out | 3 ++
src/test/regress/sql/stats_ext.sql | 3 ++
3 files changed, 27 insertions(+), 17 deletions(-)
diff --git a/src/backend/statistics/extended_stats.c b/src/backend/statistics/extended_stats.c
index d2aa8d0ca3..db621b206c 100644
--- a/src/backend/statistics/extended_stats.c
+++ b/src/backend/statistics/extended_stats.c
@@ -1615,31 +1615,35 @@ statext_is_compatible_clause(PlannerInfo *root, Node *clause, Index relid,
if (pg_class_aclcheck(rte->relid, userid, ACL_SELECT) != ACLCHECK_OK)
{
Bitmapset *clause_attnums = NULL;
+ int attnum = -1;
/* Don't have table privilege, must check individual columns */
- if (*exprs != NIL)
+ while ((attnum = bms_next_member(*attnums, attnum)) >= 0)
{
- pull_varattnos((Node *) exprs, relid, &clause_attnums);
- clause_attnums = bms_add_members(clause_attnums, *attnums);
+ clause_attnums =
+ bms_add_member(clause_attnums,
+ attnum - FirstLowInvalidHeapAttributeNumber);
}
- else
- clause_attnums = *attnums;
- if (bms_is_member(InvalidAttrNumber, clause_attnums))
- {
- /* Have a whole-row reference, must have access to all columns */
- if (pg_attribute_aclcheck_all(rte->relid, userid, ACL_SELECT,
- ACLMASK_ALL) != ACLCHECK_OK)
- return false;
- }
- else
+ if (*exprs != NIL)
+ pull_varattnos((Node *) *exprs, relid, &clause_attnums);
+
+ attnum = -1;
+ while ((attnum = bms_next_member(clause_attnums, attnum)) >= 0)
{
- /* Check the columns referenced by the clause */
- int attnum = -1;
+ /* attnum is offset by FirstLowInvalidHeapAttributeNumber */
+ AttrNumber attno = attnum + FirstLowInvalidHeapAttributeNumber;
- while ((attnum = bms_next_member(clause_attnums, attnum)) >= 0)
+ if (attno == InvalidAttrNumber)
+ {
+ /* Have a whole-row reference, must have access to all columns */
+ if (pg_attribute_aclcheck_all(rte->relid, userid, ACL_SELECT,
+ ACLMASK_ALL) != ACLCHECK_OK)
+ return false;
+ }
+ else
{
- if (pg_attribute_aclcheck(rte->relid, attnum, userid,
+ if (pg_attribute_aclcheck(rte->relid, attno, userid,
ACL_SELECT) != ACLCHECK_OK)
return false;
}
diff --git a/src/test/regress/expected/stats_ext.out b/src/test/regress/expected/stats_ext.out
index 8f5fd546eb..ea70e26ff3 100644
--- a/src/test/regress/expected/stats_ext.out
+++ b/src/test/regress/expected/stats_ext.out
@@ -3213,6 +3213,9 @@ GRANT USAGE ON SCHEMA tststats TO regress_stats_user1;
SET SESSION AUTHORIZATION regress_stats_user1;
SELECT * FROM tststats.priv_test_tbl; -- Permission denied
ERROR: permission denied for table priv_test_tbl
+-- Check individual columns if we don't have table privilege
+SELECT * FROM tststats.priv_test_tbl where a = 1 and tststats.priv_test_tbl.* > (1, 1) is not null;
+ERROR: permission denied for table priv_test_tbl
-- Attempt to gain access using a leaky operator
CREATE FUNCTION op_leak(int, int) RETURNS bool
AS 'BEGIN RAISE NOTICE ''op_leak => %, %'', $1, $2; RETURN $1 < $2; END'
diff --git a/src/test/regress/sql/stats_ext.sql b/src/test/regress/sql/stats_ext.sql
index 5fd865f509..81f6a2ee17 100644
--- a/src/test/regress/sql/stats_ext.sql
+++ b/src/test/regress/sql/stats_ext.sql
@@ -1609,6 +1609,9 @@ GRANT USAGE ON SCHEMA tststats TO regress_stats_user1;
SET SESSION AUTHORIZATION regress_stats_user1;
SELECT * FROM tststats.priv_test_tbl; -- Permission denied
+-- Check individual columns if we don't have table privilege
+SELECT * FROM tststats.priv_test_tbl where a = 1 and tststats.priv_test_tbl.* > (1, 1) is not null;
+
-- Attempt to gain access using a leaky operator
CREATE FUNCTION op_leak(int, int) RETURNS bool
AS 'BEGIN RAISE NOTICE ''op_leak => %, %'', $1, $2; RETURN $1 < $2; END'
--
2.31.0
* Re: BUG #17570: Unrecognized node type for query with statistics on expressions
@ 2022-08-05 15:08 Tom Lane <tgl@sss.pgh.pa.us>
parent: Richard Guo <guofenglinux@gmail.com>
0 siblings, 0 replies; 10+ messages in thread
From: Tom Lane @ 2022-08-05 15:08 UTC (permalink / raw)
To: Richard Guo <guofenglinux@gmail.com>; +Cc: Japin Li <japinli@hotmail.com>; Tomas Vondra <tomas.vondra@enterprisedb.com>; a.kozhemyakin@postgrespro.ru, PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
Richard Guo <guofenglinux@gmail.com> writes:
> I realized the fix in v2 is not correct. The attnums generated by
> statext_is_compatible_clause is expected to be NOT offsetting by its
> caller, i.e. statext_mcv_clauselist_selectivity. Revise that in v3 patch
> by doing the offsetting when composing clause_attnums that need to have
> permission check.
Yeah, I was afraid that that route would require the patch to metastasize
into more places. Agreed on keeping the argument definition the same
and coping locally instead. But I don't think that you've done enough
to address the root cause of this bug, which is the woeful
under-documentation of the argument. I'll have a go at that part.
regards, tom lane
^ permalink raw reply [nested|flat] 10+ messages in thread
end of thread, other threads:[~2022-08-05 15:08 UTC | newest]
Thread overview: 10+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2022-08-04 08:27 BUG #17570: Unrecognized node type for query with statistics on expressions PG Bug reporting form <noreply@postgresql.org>
2022-08-04 10:33 ` Japin Li <japinli@hotmail.com>
2022-08-04 10:37 ` Richard Guo <guofenglinux@gmail.com>
2022-08-04 11:25 ` Richard Guo <guofenglinux@gmail.com>
2022-08-04 14:59 ` Japin Li <japinli@hotmail.com>
2022-08-04 15:42 ` Tom Lane <tgl@sss.pgh.pa.us>
2022-08-05 00:49 ` Richard Guo <guofenglinux@gmail.com>
2022-08-05 09:03 ` Richard Guo <guofenglinux@gmail.com>
2022-08-05 15:08 ` Tom Lane <tgl@sss.pgh.pa.us>
2022-08-05 01:25 ` Richard Guo <guofenglinux@gmail.com>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox