Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Allow examine_simple_variable() to work on INSERT RETURNING Vars.
authorTom Lane <tgl@sss.pgh.pa.us>
Mon, 8 Jan 2024 16:48:44 +0000 (11:48 -0500)
committerTom Lane <tgl@sss.pgh.pa.us>
Mon, 8 Jan 2024 16:48:44 +0000 (11:48 -0500)
Since commit 599b33b94, this function assumed that every RTE_RELATION
RangeTblEntry would have an associated RelOptInfo.  But that's not so:
we only build RelOptInfos for relations that are scanned by the query.
In particular the target of an INSERT won't have one, so that Vars
appearing in an INSERT ... RETURNING list will not have an associated
RelOptInfo.  This apparently wasn't a problem before commit f7816aec2
taught examine_simple_variable() to drill down into CTEs containing
INSERT RETURNING, but it is now.

To fix, add a fallback code path that gets the userid to use directly
from the RTEPermissionInfo associated with the RTE.  (Sadly, we must
have two code paths, because not every RTE has a RTEPermissionInfo
either.)

Per report from Alexander Lakhin.  No back-patch, since the case is
apparently unreachable before f7816aec2.

Discussion: https://postgr.es/m/608a4886-6c60-0f9e-97d5-591256bd4150@gmail.com

src/backend/optimizer/util/relnode.c
src/backend/utils/adt/selfuncs.c
src/include/optimizer/pathnode.h
src/test/regress/expected/with.out
src/test/regress/sql/with.sql

index 5a9ce44532db082deea55949e4ba1fd06358b74b..22d01cef5b3075977df10ea5a51e8be55dfd8869 100644 (file)
@@ -420,6 +420,19 @@ find_base_rel(PlannerInfo *root, int relid)
    return NULL;                /* keep compiler quiet */
 }
 
+/*
+ * find_base_rel_noerr
+ *   Find a base or otherrel relation entry, returning NULL if there's none
+ */
+RelOptInfo *
+find_base_rel_noerr(PlannerInfo *root, int relid)
+{
+   /* use an unsigned comparison to prevent negative array element access */
+   if ((uint32) relid < (uint32) root->simple_rel_array_size)
+       return root->simple_rel_array[relid];
+   return NULL;
+}
+
 /*
  * find_base_rel_ignore_join
  *   Find a base or otherrel relation entry, which must already exist.
index dbcd98d9851665336ce80ad8bd6e45f551734784..cea777e9d40f3c2c322fa508116e1afcfb88bff1 100644 (file)
 #include "optimizer/paths.h"
 #include "optimizer/plancat.h"
 #include "parser/parse_clause.h"
+#include "parser/parse_relation.h"
 #include "parser/parsetree.h"
 #include "statistics/statistics.h"
 #include "storage/bufmgr.h"
@@ -5434,17 +5435,30 @@ examine_simple_variable(PlannerInfo *root, Var *var,
 
        if (HeapTupleIsValid(vardata->statsTuple))
        {
-           RelOptInfo *onerel = find_base_rel(root, var->varno);
+           RelOptInfo *onerel = find_base_rel_noerr(root, var->varno);
            Oid         userid;
 
            /*
             * Check if user has permission to read this column.  We require
             * all rows to be accessible, so there must be no securityQuals
-            * from security barrier views or RLS policies.  Use
-            * onerel->userid if it's set, in case we're accessing the table
-            * via a view.
+            * from security barrier views or RLS policies.
+            *
+            * Normally the Var will have an associated RelOptInfo from which
+            * we can find out which userid to do the check as; but it might
+            * not if it's a RETURNING Var for an INSERT target relation.  In
+            * that case use the RTEPermissionInfo associated with the RTE.
             */
-           userid = OidIsValid(onerel->userid) ? onerel->userid : GetUserId();
+           if (onerel)
+               userid = onerel->userid;
+           else
+           {
+               RTEPermissionInfo *perminfo;
+
+               perminfo = getRTEPermissionInfo(root->parse->rteperminfos, rte);
+               userid = perminfo->checkAsUser;
+           }
+           if (!OidIsValid(userid))
+               userid = GetUserId();
 
            vardata->acl_ok =
                rte->securityQuals == NIL &&
index 03a0e46e7068b50dd484a6bbdaaba6165db42d1c..c43d97b48a69fa820462fcce865180ca099514ea 100644 (file)
@@ -307,6 +307,7 @@ extern void expand_planner_arrays(PlannerInfo *root, int add_size);
 extern RelOptInfo *build_simple_rel(PlannerInfo *root, int relid,
                                    RelOptInfo *parent);
 extern RelOptInfo *find_base_rel(PlannerInfo *root, int relid);
+extern RelOptInfo *find_base_rel_noerr(PlannerInfo *root, int relid);
 extern RelOptInfo *find_base_rel_ignore_join(PlannerInfo *root, int relid);
 extern RelOptInfo *find_join_rel(PlannerInfo *root, Relids relids);
 extern RelOptInfo *build_join_rel(PlannerInfo *root,
index 69c56ce2077aefaf976f0381e06a04a52a3814c1..3cf969d938257ea251b6845140614758e9f0ea4a 100644 (file)
@@ -654,6 +654,24 @@ select count(*) from tenk1 a
                ->  CTE Scan on x
 (8 rows)
 
+explain (costs off)
+with x as materialized (insert into tenk1 default values returning unique1)
+select count(*) from tenk1 a
+  where unique1 in (select * from x);
+                         QUERY PLAN                         
+------------------------------------------------------------
+ Aggregate
+   CTE x
+     ->  Insert on tenk1
+           ->  Result
+   ->  Nested Loop
+         ->  HashAggregate
+               Group Key: x.unique1
+               ->  CTE Scan on x
+         ->  Index Only Scan using tenk1_unique1 on tenk1 a
+               Index Cond: (unique1 = x.unique1)
+(10 rows)
+
 -- SEARCH clause
 create temp table graph0( f int, t int, label text );
 insert into graph0 values
index 3ef98988663b89430bc50cfb4d28c85113d94cf2..ff68e21e2e6d226a1b33d23953525ed571f8fecd 100644 (file)
@@ -354,6 +354,11 @@ with x as materialized (select unique1 from tenk1 b)
 select count(*) from tenk1 a
   where unique1 in (select * from x);
 
+explain (costs off)
+with x as materialized (insert into tenk1 default values returning unique1)
+select count(*) from tenk1 a
+  where unique1 in (select * from x);
+
 -- SEARCH clause
 
 create temp table graph0( f int, t int, label text );