Re: [HACKERS] COPY WITH CSV FORCE QUOTE * -- REVIEW
Tom Lane wrote:
Andrew Dunstan writes:
I have reviewed this and made a small tweak in the docco. I'm just about
ready to commit this, but I'm still slightly worried that passing NULL
to denote all columns in this piece of grammar:
| FORCE QUOTE '*'
{
$$ = makeDefElem("force_quote", NULL);
}
might be less than robust - it just feels slightly hacky, so I'd
appreciate others' thoughts.
I agree, that's ugly. Why don't you use an A_Star node?
OK, Done and committed. Nice little addition.
cheers
andrew
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] COPY WITH CSV FORCE QUOTE * -- REVIEW
Andrew Dunstan writes:
> I have reviewed this and made a small tweak in the docco. I'm just about
> ready to commit this, but I'm still slightly worried that passing NULL
> to denote all columns in this piece of grammar:
> | FORCE QUOTE '*'
> {
> $$ = makeDefElem("force_quote", NULL);
> }
> might be less than robust - it just feels slightly hacky, so I'd
> appreciate others' thoughts.
I agree, that's ugly. Why don't you use an A_Star node?
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] COPY WITH CSV FORCE QUOTE * -- REVIEW
Josh Berkus wrote:
Stuff someone else should do:
a. review code
b. review code format
I am done with this review.
I have reviewed this and made a small tweak in the docco. I'm just about
ready to commit this, but I'm still slightly worried that passing NULL
to denote all columns in this piece of grammar:
| FORCE QUOTE '*'
{
$$ = makeDefElem("force_quote", NULL);
}
might be less than robust - it just feels slightly hacky, so I'd
appreciate others' thoughts. If nobody else is bothered I will commit
the patch.
cheers
andrew
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] COPY WITH CSV FORCE QUOTE * -- REVIEW
Itagaki-san, On Apple OS 10.5: 1. new patch applied cleanly 2. new patch built cleanly 3. regression tests pass 4. Tested following operations: postgres=# COPY marchlog TO '/tmp/marchlog1.csv' with csv header; COPY 81097 postgres=# COPY marchlog TO '/tmp/marchlog2.csv' with csv header force quote *; COPY 81097 postgres=# COPY marchlog TO '/tmp/marchlog3.csv' with csv header force quote process_id; COPY 81097 postgres=# COPY marchlog TO '/tmp/marchlog4.csv' with csv force quote *; COPY 81097 postgres=# COPY marchlog TO '/tmp/marchlog5.csv' with force quote *; ERROR: COPY force quote available only in CSV mode STATEMENT: COPY marchlog TO '/tmp/marchlog5.csv' with force quote *; ERROR: COPY force quote available only in CSV mode postgres=# COPY reloadlog FROM '/tmp/marchlog2.csv' with csv header; COPY 81097 postgres-# \copy marchlog TO '/tmp/marchlog5.csv' with csv force quote *; postgres-# 5. Regression tests for FORCE QUOTE present. 6. Docs present; not sure how good they are, see prior discussion. Stuff someone else should do: a. review code b. review code format I am done with this review. -- Josh Berkus PostgreSQL Experts Inc. www.pgexperts.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] COPY WITH CSV FORCE QUOTE * -- REVIEW
Josh Berkus wrote: > On 7/16/09 12:53 PM, Robert Haas wrote: > > I think perhaps we should ask the patch author to remove the NOT NULL > > stuff first? > > Yes, current status is "Waiting on Author". OK, I removed "FORCE NOT NULL" stuff from the patch. The attached patch only adds "FORCE QUOTE *" feature. Regards, --- ITAGAKI Takahiro NTT Open Source Software Center force_quote_all-20090717.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] COPY WITH CSV FORCE QUOTE * -- REVIEW
On 7/16/09 12:53 PM, Robert Haas wrote: On Thu, Jul 16, 2009 at 2:47 PM, Josh Berkus wrote: Unless there are other things we want to test (CLOBs?) I think the patch is probably ready for code review of the FORCE QUOTE * portion. I think perhaps we should ask the patch author to remove the NOT NULL stuff first? Yes, current status is "Waiting on Author". -- Josh Berkus PostgreSQL Experts Inc. www.pgexperts.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] COPY WITH CSV FORCE QUOTE * -- REVIEW
On Thu, Jul 16, 2009 at 2:47 PM, Josh Berkus wrote: > Unless there are other things we want to test (CLOBs?) I think the patch is > probably ready for code review of the FORCE QUOTE * portion. I think perhaps we should ask the patch author to remove the NOT NULL stuff first? ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] COPY WITH CSV FORCE QUOTE * -- REVIEW
All, 1) Patch applies cleanly against CVS head. 2) Patch compiles and builds cleanly. 3) Unable to check docs because of general doc build problems. 4) Tested the following commands, using a 10MB table of PostgreSQL log data: postgres=# COPY marchlog TO '/tmp/marchlog1.csv' with csv header; COPY 81097 postgres=# COPY marchlog TO '/tmp/marchlog2.csv' with csv header force quote *; COPY 81097 postgres=# COPY marchlog TO '/tmp/marchlog3.csv' with csv header force quote process_id; COPY 81097 postgres=# COPY marchlog TO '/tmp/marchlog4.csv' with csv force quote *; COPY 81097 postgres=# COPY marchlog TO '/tmp/marchlog5.csv' with force quote *; ERROR: COPY force quote available only in CSV mode STATEMENT: COPY marchlog TO '/tmp/marchlog5.csv' with force quote *; ERROR: COPY force quote available only in CSV mode postgres=# COPY reloadlog FROM '/tmp/marchlog2.csv' with csv header; COPY 81097 postgres-# \copy marchlog TO '/tmp/marchlog5.csv' with csv force quote *; postgres-# Per discussion, I did not test FORCE QUOTE NOT NULL *. All output looked as expected. This patch did not seem to change eariler functionality, and seems to quote as specified. Unless there are other things we want to test (CLOBs?) I think the patch is probably ready for code review of the FORCE QUOTE * portion. -- Josh Berkus PostgreSQL Experts Inc. www.pgexperts.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
