Re: TRUNCATE on foreign table

2021年4月1日(木) 18:53 Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com>:
>
> On 2021/04/01 0:09, Kohei KaiGai wrote:
> > What does the "permission checks" mean in this context?
> > The permission checks on the foreign tables involved are already checked
> > at truncate_check_rel(), by PostgreSQL's standard access control.
>
> I meant that's the permission check that happens in the remote server side.
> For example, when the foreign table is defined on postgres_fdw and truncated,
> TRUNCATE command is issued to the remote server via postgres_fdw and
> it checks the permission of the table before performing actual truncation.
>
>
> > Please assume an extreme example below:
> > 1. I defined a foreign table with file_fdw onto a local CSV file.
> > 2. Someone tries to scan the foreign table, and ACL allows it.
> > 3. I disallow the read remission of the CSV using chmod, after the above step,
> > but prior to the query execution.
> > 4. Someone's query moved to the execution stage, then IterateForeignScan()
> > raises an error due to OS level permission checks.
> >
> > FDW is a mechanism to convert from/to external data sources to/from PostgreSQL's
> > structured data, as literal. Once we checked the permissions of
> > foreign-tables by
> > database ACLs, any other permission checks handled by FDW driver are a part of
> > execution (like, OS permission check when file_fdw read(2) the
> > underlying CSV files).
> > And, we have no reliable way to check entire permissions preliminary,
> > like OS file
> > permission check or database permission checks by remote server. Even
> > if a checker
> > routine returned a "hint", it may be changed at the execution time.
> > Somebody might
> > change the "truncate" permission at the remote server.
> >
> > How about your opinions?
>
> I agree that something like checker routine might not be so useful and
> also be overkill. I was thinking that it's better to truncate the foreign tables first
> and the local ones later. Otherwise it's a waste of cycles to truncate
> the local tables if the truncation on foreign table causes an permission error
> on the remote server.
>
> But ISTM that the order of tables to truncate that the current patch provides
> doesn't cause any actual bugs. So if you think the current order is better,
> I'm ok with that for now. In this case, the comments for ExecuteTruncate()
> should be updated.
>
It is fair enough for me to reverse the order of actual truncation.

How about the updated comments below?

This is a multi-relation truncate. We first open and grab exclusive
lock on all relations involved, checking permissions (local database
ACLs even if relations are foreign-tables) and otherwise verifying
that the relation is OK for truncation. In CASCADE mode, ...(snip)...
Finally all the relations are truncated and reindexed. If any foreign-
tables are involved, its callback shall be invoked prior to the truncation
of regular tables.

> BTW, the latest patch doesn't seem to be applied cleanly to the master
> because of commit 27e1f14563. Could you rebase it?
>
Onishi-san, go ahead. :-)

Best regards,
--
HeteroDB, Inc / The PG-Strom Project
KaiGai Kohei <kaigai(at)heterodb(dot)com>