Thread: create database doesn't work well in MULTIBYTE mode

create database doesn't work well in MULTIBYTE mode

From
"Hiroshi Inoue"
Date:
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



Re: [HACKERS] create database doesn't work well in MULTIBYTE mode

From
Bruce Momjian
Date:
> 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,
Pennsylvania19026
 


RE: [HACKERS] create database doesn't work well in MULTIBYTE mode

From
"Hiroshi Inoue"
Date:
> -----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



Re: [HACKERS] create database doesn't work well in MULTIBYTE mode

From
Peter Eisentraut
Date:
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



Re: [HACKERS] create database doesn't work well in MULTIBYTE mode

From
Thomas Lockhart
Date:
> 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


Re: [HACKERS] create database doesn't work well in MULTIBYTE mode

From
Thomas Lockhart
Date:
> $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


Re: [HACKERS] create database doesn't work well in MULTIBYTE mode

From
Peter Eisentraut
Date:
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




Re: [HACKERS] create database doesn't work well in MULTIBYTE mode

From
Thomas Lockhart
Date:
> > 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 MULTIBYTEn->encoding = $6;
#elsen->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


Re: [HACKERS] create database doesn't work well in MULTIBYTE mode

From
Peter Eisentraut
Date:
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