Re: Skip ExecCheckRTPerms in CTAS with no data

From: Anastasia Lubennikova <a(dot)lubennikova(at)postgrespro(dot)ru>
To: Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Skip ExecCheckRTPerms in CTAS with no data
Date: 2020-11-09 19:48:09
Message-ID: 80c0bbeb-050f-e35f-f195-b66d0985b42c@postgrespro.ru
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 29.09.2020 14:39, Bharath Rupireddy wrote:
> On Mon, Sep 28, 2020 at 7:48 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> writes:
>>> In case of CTAS with no data, we actually do not insert the tuples
>>> into the created table, so we can skip checking for the insert
>>> permissions. Anyways, the insert permissions will be checked when the
>>> tuples are inserted into the table.
>> I'd argue this is wrong. You don't get to skip permissions checks
>> in ordinary queries just because, say, there's a LIMIT 0 on the
>> query.
>>
> Right, when there's a select with limit 0 clause, we do check for the
> select permissions. But my point is, we don't check insert
> permissions(or select or update etc.) when we create a plain table
> using CREATE TABLE test_tbl(a1 INT). Of course, we do check create
> permissions. Going by the similar point, shouldn't we also check only
> create permission(which is already being done as part of
> DefineRelation) and skip the insert permission(the change this patch
> does) for the new table being created as part of CREATE TABLE test_tbl
> AS SELECT * FROM test_tbl2? However select permission will be checked
> for test_tbl2. The insert permissions will be checked anyways before
> inserting rows into the table created in CTAS.
>
> With Regards,
> Bharath Rupireddy.
> EnterpriseDB: http://www.enterprisedb.com
>
I see Tom's objection above. Still, I tend to agree that if 'WITH NO
DATA' was specified explicitly, CREATE AS should behave more like a
utility statement rather than a regular query. So I think that this
patch can be useful in some use-cases and I definitely don't see any
harm it could cause. Even the comment in the current code suggests that
it is an option.

I took a look at the patch. It is quite simple, so no comments about the
code. It would be good to add a test to select_into.sql to show that it
only applies to 'WITH NO DATA' and that subsequent insertions will fail
if permissions are not set.

Maybe we should also mention it a documentation, but I haven't found any
specific paragraph about permissions on CTAS.

--
Anastasia Lubennikova
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Bruce Momjian 2020-11-09 19:56:53 Re: public schema default ACL
Previous Message Stephen Frost 2020-11-09 19:26:41 Re: Disable WAL logging to speed up data loading