Re: [HACKERS] Add ENCODING option to COPY

2011-02-08 Thread Peter Eisentraut
On tis, 2011-02-08 at 10:53 -0500, Tom Lane wrote: > Peter Eisentraut writes: > > On fre, 2011-02-04 at 10:47 -0500, Tom Lane wrote: > >> The reason that we use quotes in CREATE DATABASE is that encoding > >> names aren't assumed to be valid SQL identifiers. If this patch isn't > >> following the

Re: [HACKERS] Add ENCODING option to COPY

2011-02-08 Thread Hitoshi Harada
2011/2/9 Tom Lane : > Peter Eisentraut writes: >> On fre, 2011-02-04 at 10:47 -0500, Tom Lane wrote: >>> The reason that we use quotes in CREATE DATABASE is that encoding >>> names aren't assumed to be valid SQL identifiers.  If this patch isn't >>> following the CREATE DATABASE precedent, it's th

Re: [HACKERS] Add ENCODING option to COPY

2011-02-08 Thread Tom Lane
Peter Eisentraut writes: > On fre, 2011-02-04 at 10:47 -0500, Tom Lane wrote: >> The reason that we use quotes in CREATE DATABASE is that encoding >> names aren't assumed to be valid SQL identifiers. If this patch isn't >> following the CREATE DATABASE precedent, it's the patch that's wrong, >> n

Re: [HACKERS] Add ENCODING option to COPY

2011-02-08 Thread Peter Eisentraut
On fre, 2011-02-04 at 10:47 -0500, Tom Lane wrote: > The reason that we use quotes in CREATE DATABASE is that encoding > names aren't assumed to be valid SQL identifiers. If this patch isn't > following the CREATE DATABASE precedent, it's the patch that's wrong, > not CREATE DATABASE. Since encod

Re: [HACKERS] Add ENCODING option to COPY

2011-02-07 Thread Hitoshi Harada
2011/2/8 Robert Haas : > On Fri, Feb 4, 2011 at 1:54 PM, Hitoshi Harada wrote: >> 2011/2/5 Tom Lane : >>> Hitoshi Harada writes: 2011/2/5 Tom Lane : > Yeah, putting backend-only stuff into that header is a nonstarter. >>> Do you mean you think it' all right to define pg_cached_

Re: [HACKERS] Add ENCODING option to COPY

2011-02-07 Thread Robert Haas
On Fri, Feb 4, 2011 at 1:54 PM, Hitoshi Harada wrote: > 2011/2/5 Tom Lane : >> Hitoshi Harada writes: >>> 2011/2/5 Tom Lane : Yeah, putting backend-only stuff into that header is a nonstarter. >> >>> Do you mean you think it' all right to define >>> pg_cached_encoding_conversion() in pg_conv

Re: [HACKERS] Add ENCODING option to COPY

2011-02-04 Thread Hitoshi Harada
2011/2/5 Tom Lane : > Hitoshi Harada writes: >> 2011/2/5 Tom Lane : >>> Yeah, putting backend-only stuff into that header is a nonstarter. > >> Do you mean you think it' all right to define >> pg_cached_encoding_conversion() in pg_conversion_fn.h? > > That seems pretty random too.  I still think y

Re: [HACKERS] Add ENCODING option to COPY

2011-02-04 Thread Tom Lane
Hitoshi Harada writes: > 2011/2/5 Tom Lane : >> Yeah, putting backend-only stuff into that header is a nonstarter. > Do you mean you think it' all right to define > pg_cached_encoding_conversion() in pg_conversion_fn.h? That seems pretty random too. I still think you've designed this API badly

Re: [HACKERS] Add ENCODING option to COPY

2011-02-04 Thread Hitoshi Harada
2011/2/5 Tom Lane : > Hitoshi Harada writes: >> 2011/2/5 Tom Lane : >>> The reason that we use quotes in CREATE DATABASE is that encoding names >>> aren't assumed to be valid SQL identifiers.  If this patch isn't >>> following the CREATE DATABASE precedent, it's the patch that's wrong, >>> not CRE

Re: [HACKERS] Add ENCODING option to COPY

2011-02-04 Thread Tom Lane
Hitoshi Harada writes: > 2011/2/5 Tom Lane : >> The reason that we use quotes in CREATE DATABASE is that encoding names >> aren't assumed to be valid SQL identifiers.  If this patch isn't >> following the CREATE DATABASE precedent, it's the patch that's wrong, >> not CREATE DATABASE. > What about

Re: [HACKERS] Add ENCODING option to COPY

2011-02-04 Thread Hitoshi Harada
2011/2/5 Tom Lane : > Itagaki Takahiro writes: >> * Syntax: ENCODING encoding vs. ENCODING 'encoding' >> We don't have to quote encoding names in the patch, but we always need >> quotes for CREATE DATABASE WITH ENCODING. I think we should modify >> CREATE DATABASE to accept unquoted encoding names

Re: [HACKERS] Add ENCODING option to COPY

2011-02-04 Thread Tom Lane
Itagaki Takahiro writes: > * Syntax: ENCODING encoding vs. ENCODING 'encoding' > We don't have to quote encoding names in the patch, but we always need > quotes for CREATE DATABASE WITH ENCODING. I think we should modify > CREATE DATABASE to accept unquoted encoding names aside from the patch. Th

Re: [HACKERS] Add ENCODING option to COPY

2011-02-04 Thread Hitoshi Harada
2011/2/4 Itagaki Takahiro : > On Tue, Feb 1, 2011 at 13:08, Hitoshi Harada wrote: >>> The third patch is attached, modifying mb routines so that they can >>> receive conversion procedures as FmgrInof * and save the function >>> pointer in CopyState. >>> I tested it with encoding option and could n

Re: [HACKERS] Add ENCODING option to COPY

2011-02-03 Thread Itagaki Takahiro
On Tue, Feb 1, 2011 at 13:08, Hitoshi Harada wrote: >> The third patch is attached, modifying mb routines so that they can >> receive conversion procedures as FmgrInof * and save the function >> pointer in CopyState. >> I tested it with encoding option and could not see performance slowdown. > Hmm

Re: [HACKERS] Add ENCODING option to COPY

2011-01-31 Thread Hitoshi Harada
2011/2/1 Hitoshi Harada : > 2011/2/1 Tom Lane : >> Hitoshi Harada writes: >>> Finally I concluded the concern Itagaki-san raised can be solved by >>> adding code that restores client_encoding in copy_in_error_callback. >> >> It might happen to work today (or at least in the scenarios you tested),

Re: [HACKERS] Add ENCODING option to COPY

2011-01-31 Thread Hitoshi Harada
2011/2/1 Tom Lane : > Hitoshi Harada writes: >> Finally I concluded the concern Itagaki-san raised can be solved by >> adding code that restores client_encoding in copy_in_error_callback. > > It might happen to work today (or at least in the scenarios you tested), > but it seems fragile as can be.

Re: [HACKERS] Add ENCODING option to COPY

2011-01-31 Thread Tom Lane
Hitoshi Harada writes: > Finally I concluded the concern Itagaki-san raised can be solved by > adding code that restores client_encoding in copy_in_error_callback. That seems like an absolutely horrid idea. Error context callbacks should not have side-effects like that. They're not guaranteed t

Re: [HACKERS] Add ENCODING option to COPY

2011-01-31 Thread Hitoshi Harada
2011/1/31 Hitoshi Harada : > 2011/1/31 Robert Haas : >> On Tue, Jan 25, 2011 at 10:24 AM, Hitoshi Harada >> wrote: >>> I'll check the code more if we have better alternatives. >> >> Where are we with this? > > I'll post another version today. Here's the patch. Finally I concluded the concern It

Re: [HACKERS] Add ENCODING option to COPY

2011-01-30 Thread Hitoshi Harada
2011/1/31 Robert Haas : > On Tue, Jan 25, 2011 at 10:24 AM, Hitoshi Harada wrote: >> I'll check the code more if we have better alternatives. > > Where are we with this? I'll post another version today. Regards, -- Hitoshi Harada -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgres

Re: [HACKERS] Add ENCODING option to COPY

2011-01-30 Thread Robert Haas
On Tue, Jan 25, 2011 at 10:24 AM, Hitoshi Harada wrote: > I'll check the code more if we have better alternatives. Where are we with this? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresq

Re: [HACKERS] Add ENCODING option to COPY

2011-01-25 Thread Hitoshi Harada
2011/1/25 Itagaki Takahiro : > On Sat, Jan 15, 2011 at 02:25, Hitoshi Harada wrote: >> The patch overrides client_encoding by the added ENCODING option, and >> restores it as soon as copy is done. > > We cannot do that because error messages should be encoded in the original > encoding even during

Re: [HACKERS] Add ENCODING option to COPY

2011-01-24 Thread Itagaki Takahiro
On Sat, Jan 15, 2011 at 02:25, Hitoshi Harada wrote: > The patch overrides client_encoding by the added ENCODING option, and > restores it as soon as copy is done. We cannot do that because error messages should be encoded in the original encoding even during COPY commands with encoding option. E

[HACKERS] Add ENCODING option to COPY

2011-01-14 Thread Hitoshi Harada
Here's the patch to add ENCODING option to COPY command. The fundamental issue was explained months ago: http://archives.postgresql.org/message-id/AANLkTikCt6bHXZjO_oX+JS7+G=jaq7gvzpu0owjcs...@mail.gmail.com In short, client_encoding is not appropriate for copy operation so we should need the spe