On Tue, Jun 28, 2016 at 9:00 AM, Etsuro Fujita <fujita.ets...@lab.ntt.co.jp>
wrote:

> On 2016/06/27 18:56, Ashutosh Bapat wrote:
>
>> On Mon, Jun 27, 2016 at 3:06 PM, Etsuro Fujita
>> <fujita.ets...@lab.ntt.co.jp <mailto:fujita.ets...@lab.ntt.co.jp>> wrote:
>>
>
>     I found another bug in error handling of whole-row references in
>>     join pushdown; conversion_error_callback fails to take into account
>>     that get_relid_attribute_name(Oid relid, AttrNumber attnum) can't
>>     handle whole-row references (ie, attnum=0), in which case that would
>>     cause cache lookup errors.  Attached is a small patch to address
>>     this issue.
>>
>
> Do you have any testcase reproducing the bug here? It would be good to
>> include that test in the regression.
>>
>
> Done.
>
> There is a always a possibility that a user would create a table (which
>> can be used as target for the foreign table) with column named
>> 'wholerow', in which case s/he will get confused with this error message.
>>
>
> By grepping I found that there are error messages that use "whole-row
> table reference", "whole-row reference",


These two should be fine.


> or "wholerow", so the use of "wholerow" seems to me reasonable.  (And IMO
> I think "wholerow" would most likely fit into that errcontext().)
>

As an error message text, which is not referring to any particular column,
these are fine. But in this case, we are specifically referring to a
particular column. That reference can be confusing. The text ' column
"wholerow" of foreign table "ft1"' is confusing either way. A lay user who
doesn't have created table with "wholerow" column, s/he will not understand
which column the error context refers to. For a user who has created table
with "wholerow" column, he won't find any problem with the data in that
column.

Ideally, we should point out the specific column that faced the conversion
problem and report it, instead of saying the whole row reference conversion
caused the problem. But that may be too difficult. Or at least the error
message should read "whole row reference of foreign table ft1".

-- 
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company

Reply via email to