Re: NOT NULL violation error handling in file_fdw

From: Shigeru HANADA <shigeru(dot)hanada(at)gmail(dot)com>
To: Etsuro Fujita <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp>
Cc: Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: NOT NULL violation error handling in file_fdw
Date: 2012-03-13 06:53:03
Message-ID: 4F5EEECF.1090502@gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

(2012/03/12 19:21), Etsuro Fujita wrote:
> According to the following documentation on IterateForeignScan() in
> 50.2. Foreign Data Wrapper Callback Routines, I have created a patch to
> support the error handling in file_fdw. Please find attached a patch.
>
> Note that PostgreSQL's executor doesn't care whether the rows
> returned violate the NOT NULL constraints which were defined
> on the foreign table columns - but the planner does care, and
> may optimize queries incorrectly if NULL values are present
> in a column declared not to contain them. If a NULL value is
> encountered when the user has declared that none should be
> present, it may be appropriate to raise an error (just as you
> would need to do in the case of a data type mismatch).

Interesting. This patch could be applied cleanly, and it catches first
record which violates NOT NULL constraint. I have some comments for the
patch.

I worry performance degradation caused by checking NOT NULL constraints
for every row, though such overhead might be hidden by disk I/O. Do you
have any result of performance testing? Users might want to disable NOT
NULL checking for already-validated files.

In addition to performance issue, IMHO exporting
ExecBuildSlotValueDescription needs more consideration. Have you
examined calling ExecConstraints instead of copying NOT NULL check
codes? It requires fully-built ResultRelInfo, and it also checks CHECK
constraints which have not been supported on foreign tables, but it
seems the standard way to apply constraints on a tuple. If you don't
want to check CHECK constraints, another possible idea is to add new
external function ExecNotNull (or something) and move NOT NULL checking
codes from ExecConstraints, and call it from fileIterateForeignScan and
ExecConstraints.

Anyway, please add this patch to Commit Fest App for tracking.
https://commitfest.postgresql.org/action/commitfest_view?id=14

--
Shigeru Hanada

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Simon Riggs 2012-03-13 07:04:26 Re: SPGiST versus hot standby - question about conflict resolution rules
Previous Message Fujii Masao 2012-03-13 04:55:32 Re: Scaling XLog insertion (was Re: Moving more work outside WALInsertLock)