Re: not null validation option in contrib/file_fdw

Thank you for the review.

> -----Original Message-----
> From: pgsql-hackers-owner(at)postgresql(dot)org
> [mailto:pgsql-hackers-owner(at)postgresql(dot)org] On Behalf Of Andrew Dunstan
> Sent: Friday, April 13, 2012 9:16 PM
> To: Shigeru HANADA
> Cc: Etsuro Fujita; pgsql-hackers(at)postgresql(dot)org
> Subject: Re: [HACKERS] not null validation option in contrib/file_fdw
>
>
>
> On 04/13/2012 07:21 AM, Shigeru HANADA wrote:
> > (2012/04/13 16:59), Etsuro Fujita wrote:
> >> I updated the patch added to CF 2012-Next [1]. Attached is the
> >> updated version of the patch.
> > I applied the patch and ran regression tests of file_fdw, and I got
> > SIGSEGV X-(
> >
> > The failure occurs in fileGetOptions, and it is caused by
> > list_delete_cell used in foreach loop; ListCell points delete target
> > has been free-ed in list_delete_cell, but foreach accesses it to get
> > next element.
> >
> > Some of backend functions which use list_delete_cell in loop use "for"
> > loop instead of foreach, and other functions exit the loop after
> > calling list_delete_cell. Since we can't stop searching non-COPY
> > options until meeting the end of the options list, we would need to
> > choose former ("for" loop), or create another list which contains only
> > valid COPY options and return it via other_options parameter.
> >
>
> Yes, the code in fileGetOptions() appears to be bogus.

Sorry, I will fix it.

> Also, "validate" is a terrible name for the option (and in the code)
IMNSHO.
> It's far too generic. "validate_not_null" or some such would surely be
> better.

I thought it would be used for not only NOT NULL but also CHECK and foreign
key constraints. That is, when a user sets the option to 'true', file_fdw
verifies that each tuple meets all kinds of constraints. So, how about
"validate_data_file" or simply "validate_file"?

Best regards,
Etsuro Fujita

> cheers
>
> andrew
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers(at)postgresql(dot)org) To make
> changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers