From: Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us>
To: Gavin Sherry <swm(at)linuxworld(dot)com(dot)au>
Cc: pgsql-hackers(at)postgresql(dot)org
Date: 2002-04-14 19:03:11
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers pgsql-patches

Gavin, I see where you are going with the patch; creating a list in
gram.y and stuffing CopyStmt directly there. However, I can't find any
other instance of our stuffing things like that in gram.y. We do have
cases using options like COPY in CREATE USER, and we do use DefElem.

I realize it will require changes to other files like copy.c. However,
it seems like the cleanest solution. I guess I am not excited about
adding another way to handle WITH options into the code. Now, if you
want to argue that CREATE USER shouldn't use DefElem either, we can
discuss that, but I think we need to be consistent in how we handle COPY
vs. the other commands that use parameters.

See commands/user.c for an example of how it uses DefElem, and the tests
done to make sure conflicting arguments are not used or in copy's case,
specified twice. It just seems like that is the cleanest way to go.

One idea I had for the code is to allow BINARY as $2, and WITH OIDS in
its current place, and all options in the new WITH location, and
concatentate them together into one DefElem list in gram.y, and pass
that to copy.c. That way, you can allow BINARY and others at the end
too and the list is in one central place.


Gavin Sherry wrote:
> On Sun, 14 Apr 2002, Bruce Momjian wrote:
> >
> > Gavin, I will do the legwork on this if you wish. I think we need to
> No matter. I intended to submit a patch to fix this.
> > use DefElem to store the COPY params, rather than using specific fields
> > in CopyStmt.
> DefElem would have required modification of code outside the parser (to
> keep utility.c and DoCopy() happy) or otherwise an even messier loop
> executed as a result of CopyStmt than I have given in the attached patch,
> plus other issues with Yacc.
> The patch attached maintains backward compatibility. The syntax is as
> follows:
> [USING DELIMITERS <delimiter>]
> [WITH [ DELIMITER <delimiter> | NULL AS <char> | OIDS ]]
> I was also going to allow BINARY in the WITH list, but there seems to be
> little point.
> Note that if you execute a query such as:
> COPY pg_class TO '/some/path/file/out'
> The code will give preference to WITH DELIMITER.
> If no one can find fault with this or my implementation, I'll follow up
> with documentation and psql patches (not sure that there is much point
> patching pg_dump).
> Gavin


[ Attachment, skipping... ]

> ---------------------------(end of broadcast)---------------------------
> TIP 2: you can get off all lists at once with the unregister command
> (send "unregister YourEmailAddressHere" to majordomo(at)postgresql(dot)org)

Bruce Momjian |
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

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Lamar Owen 2002-04-14 19:15:39 Re: Redhat 7.2.93 performance (was:Re: PostgreSQL 7.2.1-2PGDG RPMs available for RedHat-skipjack 7.2.93 and RedHat 6.2/SPARC)
Previous Message Luciano Miguel Ferreira Rocha 2002-04-14 19:00:11 Re: Redhat 7.2.93 performance (was:Re: PostgreSQL 7.2.1-2PGDG RPMs available for RedHat-skipjack 7.2.93 and RedHat 6.2/SPARC)

Browse pgsql-patches by date

  From Date Subject
Next Message Bruce Momjian 2002-04-14 23:04:06 Re: contrib/DBMirror (Replication).
Previous Message Bruce Momjian 2002-04-14 18:55:29 Re: missing patch