Horton

Re: ExecRTCheckPerms() and many prunable partitions

Amit Langote <amitlangote09@gmail.com>

View thread View on postgr.es
From: Amit Langote <amitlangote09@gmail.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: David Rowley <dgrowleyml@gmail.com>, Greg Stark <stark@mit.edu>, Zhihong Yu <zyu@yugabyte.com>, Julien Rouhaud <rjuju123@gmail.com>, Alvaro Herrera <alvherre@alvh.no-ip.org>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2022-10-04T03:45:30Z
Lists: pgsql-hackers 
On Thu, Jul 28, 2022 at 6:18 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> ... One more thing: maybe we should rethink where to put
> extraUpdatedCols.  Between the facts that it's not used for
> actual permissions checks, and that it's calculated by the
> rewriter not parser, it doesn't seem like it really belongs
> in RelPermissionInfo.  Should we keep it in RangeTblEntry?
> Should it go somewhere else entirely?  I'm just speculating,
> but now is a good time to think about it.

After fixing the issue related to this mentioned by Andres, I started
thinking more about this, especially the "Should it go somewhere else
entirely?" part.

I've kept extraUpdatedCols in RangeTblEntry in the latest patch, but
perhaps it makes sense to put that into Query?  So, the stanza in
RewriteQuery() that sets extraUpdatedCols will be changed as:

@@ -3769,7 +3769,8 @@ RewriteQuery(Query *parsetree, List *rewrite_events)
                                    NULL, 0, NULL);

            /* Also populate extraUpdatedCols (for generated columns) */
-           fill_extraUpdatedCols(rt_entry, rt_perminfo, rt_entry_relation);
+           parsetree->extraUpdatedCols =
+               get_extraUpdatedCols(rt_entry, rt_perminfo, rt_entry_relation);
        }
        else if (event == CMD_MERGE)
        {

Then, like withCheckOptionLists et al, have grouping_planner()
populate an extraUpdatedColsBitmaps in ModifyTable.
ExecInitModifyTable() will assign them directly into ResultRelInfos.
That way, anyplace in the executor that needs to look at
extraUpdatedCols of a given result relation can get that from its
ResultRelInfo, rather than from the RangeTblEntry as now.

Thoughts?


--
Thanks, Amit Langote
EDB: http://www.enterprisedb.com

Commits

  1. Add a test case for a316a3bc

  2. Correctly set userid of subquery relations' child rels

  3. Fix buggy recursion in flatten_rtes_walker().

  4. Remove some dead code in selfuncs.c

  5. Update outdated comment in ApplyRetrieveRule

  6. Rework query relation permission checking

  7. Generalize ri_RootToPartitionMap to use for non-partition children

  8. Stop accessing checkAsUser via RTE in some cases

  9. Add 'missing_ok' argument to build_attrmap_by_name