Re: Postgres_fdw join pushdown - wrong results with whole-row reference

From: Ashutosh Bapat <ashutosh(dot)bapat(at)enterprisedb(dot)com>
To: Etsuro Fujita <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp>, Rushabh Lathia <rushabh(dot)lathia(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Postgres_fdw join pushdown - wrong results with whole-row reference
Date: 2016-06-28 04:53:04
Message-ID: CAFjFpRcVQc8gCDwuy7RW3NrdpT25NLt5o_QTy6AN1on+4HbD9w@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Jun 28, 2016 at 9:00 AM, Etsuro Fujita <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp>
wrote:

> On 2016/06/27 18:56, Ashutosh Bapat wrote:
>
>> On Mon, Jun 27, 2016 at 3:06 PM, Etsuro Fujita
>> <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp <mailto:fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Rushabh Lathia 2016-06-28 05:52:20 ERROR: ORDER/GROUP BY expression not found in targetlist
Previous Message Michael Paquier 2016-06-28 03:40:37 Re: A couple of cosmetic changes around shared memory code