Re: Error attribution in foreign scans

Lists: pgsql-hackers
From: Noah Misch <noah(at)leadboat(dot)com>
To: hanada(at)metrosystems(dot)co(dot)jp, pgsql-hackers(at)postgresql(dot)org
Cc: Itagaki Takahiro <itagaki(dot)takahiro(at)gmail(dot)com>
Subject: Error attribution in foreign scans
Date: 2011-02-07 12:17:34
Message-ID: 20110207121734.GB22669@tornado.gateway.2wire.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Suppose you create several file_fdw foreign tables, query them together, and
read(2) returns EIO for one of the files:

[local] postgres=# SELECT * FROM ft0, ft1, ft2;
ERROR: could not read from COPY file: Input/output error

The message does not show which foreign table yielded the error. We could evade
the problem in this case by adding a file name to the error message in the COPY
code, but that strategy doesn't translate to twitter_fdw, firebird_fdw, etc. We
need a convention for presenting foreign errors that clearly attributes them to
the originating foreign table. What should it be?

Perhaps something as simple as having the core foreign scan code push an error
context callback that does errcontext("scan of foreign table \"%s\"", tabname)?

Disclaimer: I have only skimmed SQL/MED patches other than copy_export.

Thanks,
nm


From: Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com>
To: Noah Misch <noah(at)leadboat(dot)com>
Cc: hanada(at)metrosystems(dot)co(dot)jp, pgsql-hackers(at)postgresql(dot)org, Itagaki Takahiro <itagaki(dot)takahiro(at)gmail(dot)com>
Subject: Re: Error attribution in foreign scans
Date: 2011-02-07 13:47:43
Message-ID: 4D4FF7FF.2090109@enterprisedb.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On 07.02.2011 14:17, Noah Misch wrote:
> Suppose you create several file_fdw foreign tables, query them together, and
> read(2) returns EIO for one of the files:
>
> [local] postgres=# SELECT * FROM ft0, ft1, ft2;
> ERROR: could not read from COPY file: Input/output error
>
> The message does not show which foreign table yielded the error. We could evade
> the problem in this case by adding a file name to the error message in the COPY
> code, but that strategy doesn't translate to twitter_fdw, firebird_fdw, etc. We
> need a convention for presenting foreign errors that clearly attributes them to
> the originating foreign table. What should it be?
>
> Perhaps something as simple as having the core foreign scan code push an error
> context callback that does errcontext("scan of foreign table \"%s\"", tabname)?

Yeah, an error context callback like that makes sense. In the case of
the file FDW, though, just including the filename in the error message
seems even better. Especially if the error is directly related to
failure in reading the file.

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com


From: Itagaki Takahiro <itagaki(dot)takahiro(at)gmail(dot)com>
To: Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com>
Cc: Noah Misch <noah(at)leadboat(dot)com>, hanada(at)metrosystems(dot)co(dot)jp, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Error attribution in foreign scans
Date: 2011-02-09 01:55:05
Message-ID: AANLkTikn6XqjvnW+XsdV3m=Jry81Ye8XXGOKnZhAto52@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Mon, Feb 7, 2011 at 22:47, Heikki Linnakangas
<heikki(dot)linnakangas(at)enterprisedb(dot)com> wrote:
> On Mon, Feb 7, 2011 at 21:17, Noah Misch <noah(at)leadboat(dot)com> wrote:
>> The message does not show which foreign table yielded the error.  We could evade
>> the problem in this case by adding a file name to the error message in the COPY
>> code,

> Yeah, an error context callback like that makes sense. In the case of the
> file FDW, though, just including the filename in the error message seems
> even better. Especially if the error is directly related to failure in
> reading the file.

What do you think about filenames in terms of security? We will allow
non-superusers to use existing foreign tables of file_fdw.
For reference, we hide some path settings in GUC variables.

We also reconsider privilege of fdwoptions, umoptions, etc. They could
contain password or server-side path, but all users can retrieve the
values. It's an existing issue, but will be more serious in 9.1.

--
Itagaki Takahiro


From: Noah Misch <noah(at)leadboat(dot)com>
To: Itagaki Takahiro <itagaki(dot)takahiro(at)gmail(dot)com>
Cc: Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com>, hanada(at)metrosystems(dot)co(dot)jp, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Error attribution in foreign scans
Date: 2011-02-11 07:03:57
Message-ID: 20110211070357.GA26971@tornado.leadboat.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Wed, Feb 09, 2011 at 10:55:05AM +0900, Itagaki Takahiro wrote:
> On Mon, Feb 7, 2011 at 22:47, Heikki Linnakangas
> <heikki(dot)linnakangas(at)enterprisedb(dot)com> wrote:
> > On Mon, Feb 7, 2011 at 21:17, Noah Misch <noah(at)leadboat(dot)com> wrote:
> >> The message does not show which foreign table yielded the error. ??We could evade
> >> the problem in this case by adding a file name to the error message in the COPY
> >> code,
>
> > Yeah, an error context callback like that makes sense. In the case of the
> > file FDW, though, just including the filename in the error message seems
> > even better. Especially if the error is directly related to failure in
> > reading the file.
>
> What do you think about filenames in terms of security? We will allow
> non-superusers to use existing foreign tables of file_fdw.
> For reference, we hide some path settings in GUC variables.

Comprehensively hiding the name from non-superusers is ideal, but it seems
adequate to document that the name will not be kept secret. The superuser could
always mask the name by creating a symbolic link in $PGDATA and referencing that
in the foreign table configuration.

> We also reconsider privilege of fdwoptions, umoptions, etc. They could
> contain password or server-side path, but all users can retrieve the
> values. It's an existing issue, but will be more serious in 9.1.

This would be good to get right by 9.1 (not sure what "right" is, though).