Re: ExecRTCheckPerms() and many prunable partitions

From: Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>
To: Amit Langote <amitlangote09(at)gmail(dot)com>
Cc: Ian Lawrence Barwick <barwick(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, David Rowley <dgrowleyml(at)gmail(dot)com>, Greg Stark <stark(at)mit(dot)edu>, Zhihong Yu <zyu(at)yugabyte(dot)com>, Julien Rouhaud <rjuju123(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: ExecRTCheckPerms() and many prunable partitions
Date: 2022-11-16 11:44:02
Message-ID: 20221116114402.merw5gdwkzlc35t4@alvherre.pgsql
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 2022-Nov-10, Alvaro Herrera wrote:

> I couldn't find any cross-check that
> some perminfo element that we obtain for a rte does actually match the
> relation we wanted to check. Maybe we could add a test in some central
> place that perminfo->relid equals rte->relid?

I hadn't looked hard enough. This is already in GetRTEPermissionInfo().

> A related point is that concatenating lists doesn't seem to worry about
> not processing one element multiple times and ending up with bogus offsets.

> I think the API of ConcatRTEPermissionInfoLists is a bit weird. Why not
> have the function return the resulting list instead, just like
> list_append? It is more verbose, but it seems easier to grok.

Another point related to this. I noticed that everyplace we do
ConcatRTEPermissionInfoLists, it is followed by list_append'ing the RT
list themselves. This is strange. Maybe that's the wrong way to look
at this, and instead we should have a function that does both things
together: pass both rtables and rtepermlists and smash them all
together.

I attach your 0001 again with a bunch of other fixups (I don't include
your 0002ff). I've pushed this to see the CI results, and so far it's
looking good (hasn't finished yet though):
https://cirrus-ci.com/build/5126818977021952

I'll have a look at 0002 now.

--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/

Attachment Content-Type Size
v25-0001-Rework-query-relation-permission-checking.patch text/x-diff 145.9 KB
v25-0002-fix-typo.patch text/x-diff 937 bytes
v25-0003-Simplify-comment-a-little-bit.patch text/x-diff 910 bytes
v25-0004-change-ConcatRTEPermissionInfoLists-API.patch text/x-diff 3.4 KB
v25-0005-lfirst-lfirst_node.patch text/x-diff 3.6 KB
v25-0006-fixup-change-ConcatRTEPermissionInfoLists-API.patch text/x-diff 2.3 KB
v25-0007-perminfoindex-is-unsigned-also-test-for-0.patch text/x-diff 1.3 KB
v25-0008-Note-assert-is-redundant.-Remove-it.patch text/x-diff 985 bytes
v25-0009-Stylistic-changes.patch text/x-diff 3.7 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Michail Nikolaev 2022-11-16 12:23:46 Re: Slow standby snapshot
Previous Message Ranier Vilela 2022-11-16 11:33:58 Re: Avoid overhead open-close indexes (catalog updates)