Thread: create database doesn't work well in MULTIBYTE mode
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
> 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
> -----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
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
> 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
> $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
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
> > 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
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