Re: [PATCHES] WITH DELIMITERS in COPY

Tom Lane wrote:
> Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> > Gavin Sherry wrote:
> >> CREATE DATABASE also fills out a list in the same fashion =). I will
> >> however have a look at revising this patch to use DefElem later today.
>
> > Oh, I see that now. Which method do people prefer. We should probably
> > make them all use the same mechanism.
>
> Consistency? Who needs consistency ;-) ?
>
> Seriously, I do not see a need to change either of these approaches
> just for the sake of changing it. CREATE DATABASE is okay as-is, and
> so are the statements that use DefElem. I tend to like DefElem better
> for the statements that we change around frequently ... for instance
> the recent changes to the set of volatility keywords for functions
> didn't require any changes to the grammar or the parsenode definitions.
> But I think that a simple struct definition is easier to understand,
> so I favor that for stable feature sets.
>
> As for which one is better suited for COPY, I don't have a strong
> opinion, but lean to DefElem. Seems like COPY will probably keep
> accreting new features.

The code that bothered me about the CREATE DATABASE param processing
was:

/* process additional options */
foreach(l, $5)
{
List *optitem = (List *) lfirst(l);

switch (lfirsti(optitem))
{
case 1:
n->dbpath = (char *) lsecond(optitem);
break;
case 2:
n->dbtemplate = (char *) lsecond(optitem);
break;
case 3:
n->encoding = lfirsti(lnext(optitem));
break;
case 4:
n->dbowner = (char *) lsecond(optitem);
break;
}
}

I see what it is doing, but it seems quite unclear. Seeing that people
are using this as a pattern for other param processing, I will work on a
patch to convert this to DefElem.

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026