Hi

I have a crash while creating regression database in pararell regression
test.
Seems it's due to the following change.

@@ -2638,7 +2705,14 @@
 					n->dbname = $3;
 					n->dbpath = $5;
 #ifdef MULTIBYTE
-                    n->encoding = $6;
+					if ($6 != NULL) {
+						n->encoding = pg_char_to_encoding($6);
+						if (n->encoding < 0) {
+							elog(ERROR, "Encoding name '%s' is invalid", $6);
+						}
+					} else {
+						n->encoding = GetTemplateEncoding();
+					}
 #else
 					n->encoding = 0;
 #endif

Why ?
$6 is an ival not an str.

Regards.

Hiroshi Inoue
Inoue@tpf.co.jp


Thu, 17 Feb 2000 09:42:24 +0900
"Hiroshi Inoue" <Inoue@tpf.co.jp>
create database doesn't work well in MULTIBYTE mode
> I have a crash while creating regression database in pararell regression
> test.
> Seems it's due to the following change.
> 
> @@ -2638,7 +2705,14 @@
>  					n->dbname = $3;
>  					n->dbpath = $5;
>  #ifdef MULTIBYTE
> -                    n->encoding = $6;
> +					if ($6 != NULL) {
> +						n->encoding = pg_char_to_encoding($6);
> +						if (n->encoding < 0) {
> +							elog(ERROR, "Encoding name '%s' is invalid", $6);
> +						}
> +					} else {
> +						n->encoding = GetTemplateEncoding();
> +					}
>  #else
>  					n->encoding = 0;
>  #endif
> 
> Why ?
> $6 is an ival not an str.

Not sure what to recommend here, but you clearly have found a problem. 
Try it as a string, and if that works, patch it.

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

Thu, 17 Feb 2000 00:40:32 -0500 (EST)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: [HACKERS] create database doesn't work well in MULTIBYTE mode
> -----Original Message-----
> From: Bruce Momjian [mailto:pgman@candle.pha.pa.us]
> Sent: Thursday, February 17, 2000 2:41 PM
>
> > I have a crash while creating regression database in pararell regression
> > test.
> > Seems it's due to the following change.
> >
> > @@ -2638,7 +2705,14 @@
> >  					n->dbname = $3;
> >  					n->dbpath = $5;
> >  #ifdef MULTIBYTE
> > -                    n->encoding = $6;
> > +					if ($6 != NULL) {
> > +						n->encoding =
> pg_char_to_encoding($6);
> > +						if (n->encoding < 0) {
> > +							elog(ERROR,
> "Encoding name '%s' is invalid", $6);
> > +						}
> > +					} else {
> > +						n->encoding =
> GetTemplateEncoding();
> > +					}
> >  #else
> >  					n->encoding = 0;
> >  #endif
> >
> > Why ?
> > $6 is an ival not an str.
>
> Not sure what to recommend here, but you clearly have found a problem.
> Try it as a string, and if that works, patch it.
>

$6 is already converted from string to ival in another place.
It seems to me that this change is unnecessary.
I don't understand why this was changed recently.

Regards.

Hiroshi Inoue
Inoue@tpf.co.jp


Thu, 17 Feb 2000 15:04:03 +0900
"Hiroshi Inoue" <Inoue@tpf.co.jp>
RE: [HACKERS] create database doesn't work well in MULTIBYTE mode
Good point. Thomas? ;)

As the one who wrote the seemingly correct code, I say revert this part.


On Thu, 17 Feb 2000, Hiroshi Inoue wrote:

> I have a crash while creating regression database in pararell
> regression test. Seems it's due to the following change.
> 
> @@ -2638,7 +2705,14 @@
>  					n->dbname = $3;
>  					n->dbpath = $5;
>  #ifdef MULTIBYTE
> -                    n->encoding = $6;
> +					if ($6 != NULL) {
> +						n->encoding = pg_char_to_encoding($6);
> +						if (n->encoding < 0) {
> +							elog(ERROR, "Encoding name '%s' is invalid", $6);
> +						}
> +					} else {
> +						n->encoding = GetTemplateEncoding();
> +					}
>  #else
>  					n->encoding = 0;
>  #endif
> 
> Why ?
> $6 is an ival not an str.
> 
> Regards.
> 
> Hiroshi Inoue
> Inoue@tpf.co.jp
> 
> 
> ************
> 
> 

-- 
Peter Eisentraut                  Sernanders vaeg 10:115
peter_e@gmx.net                   75262 Uppsala
http://yi.org/peter-e/            Sweden


Thu, 17 Feb 2000 12:48:57 +0100 (MET)
Peter Eisentraut <e99re41@DoCS.UU.SE>
Re: [HACKERS] create database doesn't work well in MULTIBYTE mode
> Good point. Thomas? ;)
> As the one who wrote the seemingly correct code, I say revert this part.

OK, so just a simple assignment to $6 is what is needed? I vaguely
remember a merging problem here, and obviously chose the wrong block
of code to retain. Amazing that the desired code is actually simpler
:)

                      - Thomas

-- 
Thomas Lockhart				lockhart@alumni.caltech.edu
South Pasadena, California

Thu, 17 Feb 2000 14:43:35 +0000
Thomas Lockhart <lockhart@alumni.caltech.edu>
Re: [HACKERS] create database doesn't work well in MULTIBYTE mode
> $6 is already converted from string to ival in another place.
> It seems to me that this change is unnecessary.
> I don't understand why this was changed recently.

At the moment, if the code is compiled without MULTIBYTE enabled, it
will silently ignore any "ENCODING=" clause in a CREATE DATABASE
statement.

Wouldn't it be more appropriate to throw an elog(ERROR) in this case
(or perhaps an elog(WARN))? I've got the code ready to add in.
Comments?

                   - Thomas

-- 
Thomas Lockhart				lockhart@alumni.caltech.edu
South Pasadena, California

Thu, 17 Feb 2000 15:11:19 +0000
Thomas Lockhart <lockhart@alumni.caltech.edu>
Re: [HACKERS] create database doesn't work well in MULTIBYTE mode
On 2000-02-17, Thomas Lockhart mentioned:

> At the moment, if the code is compiled without MULTIBYTE enabled, it
> will silently ignore any "ENCODING=" clause in a CREATE DATABASE
> statement.

Huh?

template1=# create database foo with encoding='LATIN1';
ERROR:  Multi-byte support is not enabled

I believe that you have missed that a fair amount of work is being done in
the create_opt_encoding rule. Take a look.

-- 
Peter Eisentraut                  Sernanders vï¿½g 10:115
peter_e@gmx.net                   75262 Uppsala
http://yi.org/peter-e/            Sweden



Fri, 18 Feb 2000 01:20:30 +0100 (CET)
Peter Eisentraut <peter_e@gmx.net>
Re: [HACKERS] create database doesn't work well in MULTIBYTE mode
> > At the moment, if the code is compiled without MULTIBYTE enabled, it
> > will silently ignore any "ENCODING=" clause in a CREATE DATABASE
> > statement.
> template1=# create database foo with encoding='LATIN1';
> ERROR:  Multi-byte support is not enabled
> I believe that you have missed that a fair amount of work is being done in
> the create_opt_encoding rule. Take a look.

Ah, thanks. I was misled (why try actually testing it? I was reading
the source ;) by some crufty code above createdb_opt_encoding (some
tabs removed for readability):

...
#ifdef MULTIBYTE
	n->encoding = $6;
#else
	n->encoding = 0;
#endif
...

where in fact if MULTIBYTE is not enabled and $6 is non-empty, the $6
production never returns! I'm planning on fixing this up (yacc
willing) to *not* do anything special when MULTIBYTE is on or off, but
will instead embed all of this funny business down in
createdb_opt_encoding with the other stuff already there.

So, why does the createdb_opt_encoding ($6 above) bother trying to
return "-1" when MULTIBYTE is disabled, when that -1 is ignored and
replaced with a zero anyway? Is it acceptable to return -1, as the $6
production does, or should it really be returning the zero which is
passed along??

                 - Thomas

-- 
Thomas Lockhart				lockhart@alumni.caltech.edu
South Pasadena, California

Fri, 18 Feb 2000 04:49:29 +0000
Thomas Lockhart <lockhart@alumni.caltech.edu>
Re: [HACKERS] create database doesn't work well in MULTIBYTE mode
On Fri, 18 Feb 2000, Thomas Lockhart wrote:

> ...
> #ifdef MULTIBYTE
> 	n->encoding = $6;
> #else
> 	n->encoding = 0;
> #endif
> ...
> 
> where in fact if MULTIBYTE is not enabled and $6 is non-empty, the $6
> production never returns!

It will if you write ENCODING = DEFAULT.

Also, the rule you're looking at also covers the case CREATE DATABASE WITH
LOCATION (no ENCODING clause given). In that case, with MULTIBYTE on, $6
will be set to GetTemplateEncoding() in the create_opt_encoding: /*EMPTY*/
rule. With MULTIBYTE off, you must set encoding to 0, because that's the
default SQL_ASCII encoding, and the createdb() function (where this all
ends up), does no further interpretation on the encoding at all. Either
way you read it, I still think the previous code is completely correct as
it stands.

> So, why does the createdb_opt_encoding ($6 above) bother trying to
> return "-1" when MULTIBYTE is disabled, when that -1 is ignored and
> replaced with a zero anyway? Is it acceptable to return -1, as the $6
> production does, or should it really be returning the zero which is
> passed along??

Two reasons: First, it's better to have some rule than none at all, I
thunk. Second, if someone mucks with the code and somehow we have a -1
encoding the database, we know exactly what went wrong. If you feel so
inclined, you can change it to a zero, but after all the code works
perfectly.


-- 
Peter Eisentraut                  Sernanders vaeg 10:115
peter_e@gmx.net                   75262 Uppsala
http://yi.org/peter-e/            Sweden


Fri, 18 Feb 2000 15:47:05 +0100 (MET)
Peter Eisentraut <e99re41@DoCS.UU.SE>
Re: [HACKERS] create database doesn't work well in MULTIBYTE mode