Re: SQL/MED - file_fdw

On Tue, 14 Dec 2010 15:51:18 +0900
Itagaki Takahiro <itagaki.takahiro@gmail.com> wrote:
> On Tue, Dec 14, 2010 at 15:31, Shigeru HANADA <hanada@metrosystems.co.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