Re: WIP Patch: Selective binary conversion of CSV file foreign tables

Hi Fujita-san,

Could you rebase this patch towards the latest tree?
It was unavailable to apply the patch cleanly.

I looked over the patch, then noticed a few points.

At ProcessCopyOptions(), defel->arg can be NIL, isn't it?
If so, cstate->convert_binary is not a suitable flag to check
redundant option. It seems to me cstate->convert_selectively
is more suitable flag to check it.

+ else if (strcmp(defel->defname, "convert_binary") == 0)
+ {
+ if (cstate->convert_binary)
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("conflicting or redundant options")));
+ cstate->convert_selectively = true;
+ if (defel->arg == NULL || IsA(defel->arg, List))
+ cstate->convert_binary = (List *) defel->arg;
+ else
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("argument to option \"%s\" must be a
list of column names",
+ defel->defname)));
+ }

At NextCopyFrom(), this routine computes default values if configured.
In case when these values are not referenced, it might be possible to
skip unnecessary calculations.
Is it unavailable to add logic to avoid to construct cstate->defmap on
unreferenced columns at BeginCopyFrom()?

Thanks,

2012/5/11 Etsuro Fujita <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp>:
>> -----Original Message-----
>> From: Robert Haas [mailto:robertmhaas(at)gmail(dot)com]
>> Sent: Friday, May 11, 2012 1:36 AM
>> To: Etsuro Fujita
>> Cc: pgsql-hackers(at)postgresql(dot)org
>> Subject: Re: [HACKERS] WIP Patch: Selective binary conversion of CSV file
>> foreign tables
>>
>> On Tue, May 8, 2012 at 7:26 AM, Etsuro Fujita
> <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp>
>> wrote:
>> > I would like to propose to improve parsing efficiency of
>> > contrib/file_fdw by selective parsing proposed by Alagiannis et
>> > al.[1], which means that for a CSV/TEXT file foreign table, file_fdw
>> > performs binary conversion only for the columns needed for query
>> > processing.  Attached is a WIP patch implementing the feature.
>>
>> Can you add this to the next CommitFest?  Looks interesting.
>
> Done.
>
> Best regards,
> Etsuro Fujita
>
>> --
>> Robert Haas
>> EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL
>> Company
>>
>
>
>
> --
> 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

--
KaiGai Kohei <kaigai(at)kaigai(dot)gr(dot)jp>