On 29.09.2020 14:39, Bharath Rupireddy wrote:
On Mon, Sep 28, 2020 at 7:48 PM Tom Lane <t...@sss.pgh.pa.us> wrote:
Bharath Rupireddy <bharath.rupireddyforpostg...@gmail.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
