Re: SQL/MED - file_fdw

On Tue, 14 Dec 2010 15:51:18 +0900
Itagaki Takahiro <itagaki(dot)takahiro(at)gmail(dot)com> wrote:
> On Tue, Dec 14, 2010 at 15:31, Shigeru HANADA <hanada(at)metrosystems(dot)co(dot)jp> wrote:
> > In addition to above, ResetCopyFrom() is necessary to support nested
> > loops which inner node is a ForeignScan.
>
> I think you can add ResetCopyFrom() to the core in your next file_fdw
> patch because the function is not used by COPY command.

Agreed. I tried your patch with adding ResetCopyFrom() to copy.c, and
found the patch works fine for superuser at least.

> I'll note other differences between the API and your FileState:
>
> - There are no superuser checks in the exported functions because
> the restriction should be only at CREATE/ALTER FOREIGN TABLE.
> If the superuser grants SELECT privileges to normal users, they
> should be able to read the file contents.
> (But we might need to hide the file path.)
> - errcontext and values/nulls arrays are not included in CopyState.
> They will be additionally kept in a holder of the CopyState.
> - You need to pass non-NULL filename. If it is NULL, the server
> tries to read data from the client.
> - The framework supports to read dumped binary files and files
> with OIDs. If you don't want to support them, please check
> parameters not to include those options.

All differences above wouldn't be serious problem, but I worry about
difference between file_fdw and COPY FROM.

"COPY FROM" is a command which INSERT data from a file essentially,
so it requires RowExclusiveLock on the target table. On the other
hand, file_fdw is a feature which reads data from a file through a
table, so it requires AccessShareLock on the source table.

Current export_copy patch doesn't allow non-superusers to fetch data
from files because BeginCopy() acquires RowExclusiveLock when
SELECTing from file_fdw table.

Using COPY routines from file_fdw might need another kind of
modularization, such as split file operation from COPY module and use
it from both of COPY and file_fdw. But it would require more code work,
and possibly performance tests.

Regards,
--
Shigeru Hanada