Re: [PATCHES] [BUGS] My investigations of the postmaster Bus error
Hey Martin, I've been playing with the MIPS machine a little and still haven't found any _obvious_ cause for the problem. However I suspect that it may be related to unaligned memory access, which _I think_ results in a SIGBUS on MIPS. I haven't found any documentation on MIPS that would confirm this however. I'm not sure exactly how would this by worked around; it occurs to me that we'd have to change config_real to look like struct config_real { enum { struct config_generic gen; double dummy; } field1; /* these fields must be set correctly in initial value: */ /* (all but reset_val are constants) */ double *variable; ... } though I'm not sure and I haven't tested it. (Of course a working patch needs to change a few more places.) I'll do some more experiments and I'll let you know. [EMAIL PROTECTED]:/tmp/pgsql8.0.4$ gdb bin/postgres GNU gdb 6.3-debian Copyright 2004 Free Software Foundation, Inc. GDB is free software, covered by the GNU General Public License, and you are welcome to change it and/or distribute copies of it under certain conditions. Type show copying to see the conditions. There is absolutely no warranty for GDB. Type show warranty for details. This GDB was configured as mips-linux...Using host libthread_db library /lib/libthread_db.so.1. (gdb) set args -boot (gdb) run Starting program: /tmp/pgsql8.0.4/bin/postgres -boot Program received signal SIGBUS, Bus error. 0x00818c38 in InitializeGUCOptions () at guc.c:2360 2360*conf-variable = conf-reset_val; (gdb) bt #0 0x00818c38 in InitializeGUCOptions () at guc.c:2360 #1 0x004a8fc0 in BootstrapMain (argc=2, argv=0x10053998) at bootstrap.c:244 #2 0x005f4dc4 in main (argc=2, argv=0x10053998) at main.c:296 (gdb) print *conf $1 = {gen = {name = 0x8c4484 geqo_selection_bias, context = PGC_USERSET, group = QUERY_TUNING_GEQO, short_desc = 0x8c4498 GEQO: selective pressure within the population., long_desc = 0x0, flags = 0, vartype = PGC_REAL, status = 0, reset_source = PGC_S_DEFAULT, tentative_source = PGC_S_DEFAULT, source = PGC_S_DEFAULT, stack = 0x0}, variable = 0x100136d2, reset_val = 2, min = 1.5, max = 2, assign_hook = 0, show_hook = 0, tentative_val = 0} -- Alvaro Herrera http://www.amazon.com/gp/registry/CTMLCN8V17R4 La grandeza es una experiencia transitoria. Nunca es consistente. Depende en gran parte de la imaginaciĆ³n humana creadora de mitos (Irulan) ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] [BUGS] My investigations of the postmaster Bus error
Alvaro Herrera [EMAIL PROTECTED] writes: I've been playing with the MIPS machine a little and still haven't found any _obvious_ cause for the problem. However I suspect that it may be related to unaligned memory access, which _I think_ results in a SIGBUS on MIPS. I haven't found any documentation on MIPS that would confirm this however. I'm not sure exactly how would this by worked around; it occurs to me that we'd have to change config_real to look like I don't think so --- to believe that the GUC data structures aren't adequately aligned, you'd have to explain why PG doesn't crash on other architectures that require 8-byte alignment of doubles, eg HPPA. regards, tom lane ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] tuple count and v3 functions in psql for COPY
Volkan YAZICI wrote: I tried to prepare a patch for these TODO items: - Have COPY return the number of rows loaded/unloaded? - Update [pg_dump and] psql to use the new COPY libpq API. I'll apply this today or tomorrow, barring any objections. BTW, if we're using an int64 counter for the # of rows modified by COPY, I wonder if it's also worth converting INSERT's rowcount to use an int64. Any objections to doing that? -Neil ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] tuple count and v3 functions in psql for COPY
Volkan YAZICI [EMAIL PROTECTED] writes: I tried to prepare a patch for these TODO items: - Have COPY return the number of rows loaded/unloaded? - Update [pg_dump and] psql to use the new COPY libpq API. ! cstate-raw_buf_index = cstate-raw_buf_len = 0; ! cstate-raw_buf_index = cstate-raw_buf_len = cstate-processed = 0; Minor stylistic gripe: processed is unrelated to those two other variables and not even the same datatype. It'd be better to zero it in a separate statement. ! { ! uint64processed = DoCopy((CopyStmt *) parsetree); ! charbuf[21]; ! ! snprintf(buf, sizeof(buf), UINT64_FORMAT, processed); ! snprintf(completionTag, COMPLETION_TAG_BUFSIZE, ! COPY %s, buf); ! } This is ugly and unnecessary. Why not ! { ! uint64processed = DoCopy((CopyStmt *) parsetree); ! ! snprintf(completionTag, COMPLETION_TAG_BUFSIZE, ! COPY UINT64_FORMAT, processed); ! } Also, I think you've broken the psql-side handling of COPY IN. There's no check for I/O error on the input, and the test for terminator (\.\n) mistakenly assumes that the terminator could only appear at the start of the buffer, not to mention assuming that it couldn't span across two bufferloads. The check for \r is wrong too (counterexample: first input line exceeds 8K), but actually you should just dispense with that entirely, because if you're going to use PQputCopyEnd then it is not your responsibility to send the terminator. But the *real* problem with this coding is that it will fetch data past the \., which breaks files that include both COPY data and SQL. Thus for example this patch breaks pg_dump files. ! for (c = p; isdigit((int) *c); ++c) Wrong. Use (unsigned char). regards, tom lane ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [PATCHES] PLpgSQL: list of scalars as row for assign stmt, fore
On Wed, Dec 21, 2005 at 09:26:35AM +0800, Christopher Kings-Lynne wrote: x, y := r; That strikes me as a really bad idea. It weakens both syntax and semantic error checking, to accomplish how much? Could use PHP-style thingy: LIST(x, y) := r; Better still, the pg way: [ROW](x, y) := r; Cheers, D -- David Fetter [EMAIL PROTECTED] http://fetter.org/ phone: +1 415 235 3778 Remember to vote! ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [PATCHES] Disparity in search_path SHOW and SET
Greg Sabino Mullane wrote: [ There is text before PGP section. ] -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Well, sure, because you told it to. Why isn't the last parameter false? Thanks. I knew I was overlooking something. I've obviously been staring at the code too long. :) Still, would it make more sense for SHOW search_path to return this: $user,public Agreed. I have gotten confused on how to set $user in the past. I have developed the following patch that sets the default with the double quotes around it, and it works fine. The patch also contains updated documentation. I just never realized that dollar signs have to be double-quoted, but I it makes sense now that I see it: test= select lanname as $user from pg_language; ERROR: syntax error at or near $ at character 19 LINE 1: select lanname as $user from pg_language; ^ test= select lanname as $user from pg_language; $user -- internal c sql (3 rows) Are the quotes an improvement? search_path $user,public (1 row) test= set search_path = $user,public; SET -- Bruce Momjian| http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 Index: doc/src/sgml/config.sgml === RCS file: /cvsroot/pgsql/doc/src/sgml/config.sgml,v retrieving revision 1.39 diff -c -c -r1.39 config.sgml *** doc/src/sgml/config.sgml20 Dec 2005 02:30:35 - 1.39 --- doc/src/sgml/config.sgml22 Dec 2005 23:42:13 - *** *** 39,45 # This is a comment log_connections = yes log_destination = 'syslog' ! search_path = '$user, public' /programlisting One parameter is specified per line. The equal sign between name and value is optional. Whitespace is insignificant and blank lines are --- 39,45 # This is a comment log_connections = yes log_destination = 'syslog' ! search_path = '$user, public' /programlisting One parameter is specified per line. The equal sign between name and value is optional. Whitespace is insignificant and blank lines are *** *** 3117,3123 para The default value for this parameter is ! literal'$user, public'/literal (where the second part will be ignored if there is no schema named literalpublic/). This supports shared use of a database (where no users have private schemas, and all share use of literalpublic/), --- 3117,3123 para The default value for this parameter is ! literal'$user, public'/literal (where the second part will be ignored if there is no schema named literalpublic/). This supports shared use of a database (where no users have private schemas, and all share use of literalpublic/), Index: doc/src/sgml/ddl.sgml === RCS file: /cvsroot/pgsql/doc/src/sgml/ddl.sgml,v retrieving revision 1.50 diff -c -c -r1.50 ddl.sgml *** doc/src/sgml/ddl.sgml 4 Nov 2005 23:53:18 - 1.50 --- doc/src/sgml/ddl.sgml 22 Dec 2005 23:42:14 - *** *** 1650,1656 screen search_path -- ! $user,public /screen The first element specifies that a schema with the same name as the current user is to be searched. If no such schema exists, --- 1650,1656 screen search_path -- ! $user,public /screen The first element specifies that a schema with the same name as the current user is to be searched. If no such schema exists, Index: src/backend/utils/misc/guc.c === RCS file: /cvsroot/pgsql/src/backend/utils/misc/guc.c,v retrieving revision 1.302 diff -c -c -r1.302 guc.c *** src/backend/utils/misc/guc.c20 Dec 2005 02:30:36 - 1.302 --- src/backend/utils/misc/guc.c22 Dec 2005 23:42:17 - *** *** 1902,1908 GUC_LIST_INPUT | GUC_LIST_QUOTE }, namespace_search_path, ! $user,public, assign_search_path, NULL }, { --- 1902,1908 GUC_LIST_INPUT | GUC_LIST_QUOTE }, namespace_search_path, ! \$user\,public, assign_search_path, NULL }, { Index: src/backend/utils/misc/postgresql.conf.sample === RCS file: /cvsroot/pgsql/src/backend/utils/misc/postgresql.conf.sample,v retrieving revision 1.171 diff -c -c
Re: [PATCHES] Disparity in search_path SHOW and SET
Bruce Momjian pgman@candle.pha.pa.us writes: Agreed. I have gotten confused on how to set $user in the past. I have developed the following patch that sets the default with the double quotes around it, and it works fine. The patch also contains updated documentation. This is really entirely irrelevant to Greg's complaint. To respond to that, you'd have to modify the behavior of SHOW. Actually, it seems that this exposes a bug in the search_path code: if I wrote what you wrote, I'd really expect that it refers to a schema named exactly $user --- the quoting ought to suppress the substitution, one would think. Not sure how hard or easy that might be to implement though ... regards, tom lane ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] Disparity in search_path SHOW and SET
Tom Lane wrote: Bruce Momjian pgman@candle.pha.pa.us writes: Agreed. I have gotten confused on how to set $user in the past. I have developed the following patch that sets the default with the double quotes around it, and it works fine. The patch also contains updated documentation. This is really entirely irrelevant to Greg's complaint. To respond to that, you'd have to modify the behavior of SHOW. Uh, SHOW does show the quotes: test= show search_path; search_path $user,public (1 row) and that can be fed right into SET: test= set search_path = $user,public; SET I thought that was the goal. Actually, it seems that this exposes a bug in the search_path code: if I wrote what you wrote, I'd really expect that it refers to a schema named exactly $user --- the quoting ought to suppress the substitution, one would think. Not sure how hard or easy that might be to implement though ... I am unsure if the quotes are suppose to still allow dollar expansion. It does in shell scripts. Actually this is kind of unusual: test= set search_path = '$user', public; SET test= show search_path; search_path - $user, public (1 row) It converts the single quotes to double. -- Bruce Momjian| http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] Trouble building 8.1.1 on Tru64 UNIX 5.1
Albert Chin wrote: On Mon, Dec 19, 2005 at 06:34:38PM -0500, Tom Lane wrote: Albert Chin [EMAIL PROTECTED] writes: On Mon, Dec 19, 2005 at 05:59:12PM -0500, Tom Lane wrote: Perhaps a more relevant question is why ecpg/preproc is including that header. #include netdb.h with -D_REENTRANT includes it. preproc.c: #include postgres_fe.h #include c.h #include port.h #include netdb.h Well, port.h is certainly doing a fine job of polluting the namespace. Maybe we should pull out the stuff that depends on netdb.h and pwd.h into some other header that isn't going to get included so widely. ADD has the same problem. There's no way that we are going to be able to dodge every single symbol that any random system header on any random platform might define --- especially when you get into the less-well-standardized headers like these. We have to think smaller in terms of what headers we include everywhere. Well, we've built on most versions of Solaris, HP-UX, AIX, Tru64 UNIX, Redhat Linux, and IRIX and this is the only symbol conflict we ran into. So, it's not a big problem. I have converted the symbols that gave you problem (HEAD/ADD) to HEAD_P/ADD_P. Please test the patch and let me know if things work. Thanks. -- Bruce Momjian| http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 Index: src/backend/parser/gram.y === RCS file: /cvsroot/pgsql/src/backend/parser/gram.y,v retrieving revision 2.517 diff -c -c -r2.517 gram.y *** src/backend/parser/gram.y 11 Dec 2005 10:54:27 - 2.517 --- src/backend/parser/gram.y 23 Dec 2005 00:12:46 - *** *** 335,341 */ /* ordinary key words in alphabetical order */ ! %token keyword ABORT_P ABSOLUTE_P ACCESS ACTION ADD ADMIN AFTER AGGREGATE ALL ALSO ALTER ANALYSE ANALYZE AND ANY ARRAY AS ASC ASSERTION ASSIGNMENT ASYMMETRIC AT AUTHORIZATION --- 335,341 */ /* ordinary key words in alphabetical order */ ! %token keyword ABORT_P ABSOLUTE_P ACCESS ACTION ADD_P ADMIN AFTER AGGREGATE ALL ALSO ALTER ANALYSE ANALYZE AND ANY ARRAY AS ASC ASSERTION ASSIGNMENT ASYMMETRIC AT AUTHORIZATION *** *** 361,367 GLOBAL GRANT GRANTED GREATEST GROUP_P ! HANDLER HAVING HEADER HOLD HOUR_P IF_P ILIKE IMMEDIATE IMMUTABLE IMPLICIT_P IN_P INCLUDING INCREMENT INDEX INHERIT INHERITS INITIALLY INNER_P INOUT INPUT_P --- 361,367 GLOBAL GRANT GRANTED GREATEST GROUP_P ! HANDLER HAVING HEADER_P HOLD HOUR_P IF_P ILIKE IMMEDIATE IMMUTABLE IMPLICIT_P IN_P INCLUDING INCREMENT INDEX INHERIT INHERITS INITIALLY INNER_P INOUT INPUT_P *** *** 878,884 } ; ! add_drop: ADD { $$ = +1; } | DROP { $$ = -1; } ; --- 878,884 } ; ! add_drop: ADD_P { $$ = +1; } | DROP { $$ = -1; } ; *** *** 1300,1307 /* Subcommands that are for ALTER TABLE only */ alter_table_cmd: ! /* ALTER TABLE relation ADD [COLUMN] coldef */ ! ADD opt_column columnDef { AlterTableCmd *n = makeNode(AlterTableCmd); n-subtype = AT_AddColumn; --- 1300,1307 /* Subcommands that are for ALTER TABLE only */ alter_table_cmd: ! /* ALTER TABLE relation ADD_P [COLUMN] coldef */ ! ADD_P opt_column columnDef { AlterTableCmd *n = makeNode(AlterTableCmd); n-subtype = AT_AddColumn; *** *** 1373,1380 n-transform = $6; $$ = (Node *)n; } ! /* ALTER TABLE relation ADD CONSTRAINT ... */ ! | ADD TableConstraint { AlterTableCmd *n = makeNode(AlterTableCmd); n-subtype = AT_AddConstraint; --- 1373,1380
Re: [PATCHES] Disparity in search_path SHOW and SET
Bruce Momjian pgman@candle.pha.pa.us writes: Uh, SHOW does show the quotes: test= show search_path; search_path $user,public (1 row) Hmm ... you're right, it does, so the current default is actually a value that you can't get into the variable by a normal SET. Interesting. (We are doing the smart stuff during SET not SHOW, it appears.) regression=# show search_path ; search_path -- $user,public (1 row) regression=# set search_path = '$user',public; SET regression=# show search_path ; search_path - $user, public (1 row) Given that, I agree with changing the default string. It should look the same as a value that you could actually assign ... regards, tom lane ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] Disparity in search_path SHOW and SET
OK, applied. I have _not_ backpatched this. --- Tom Lane wrote: Bruce Momjian pgman@candle.pha.pa.us writes: Uh, SHOW does show the quotes: test= show search_path; search_path $user,public (1 row) Hmm ... you're right, it does, so the current default is actually a value that you can't get into the variable by a normal SET. Interesting. (We are doing the smart stuff during SET not SHOW, it appears.) regression=# show search_path ; search_path -- $user,public (1 row) regression=# set search_path = '$user',public; SET regression=# show search_path ; search_path - $user, public (1 row) Given that, I agree with changing the default string. It should look the same as a value that you could actually assign ... regards, tom lane ---(end of broadcast)--- TIP 6: explain analyze is your friend -- Bruce Momjian| http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] [BUGS] BUG #2120: Crash when doing UTF8-ISO_8859_8 encoding
Tom Lane wrote: Tatsuo Ishii [EMAIL PROTECTED] writes: It looks like somebody rearranged the pg_enc enum without bothering to fix the tables that are affected by this. I will look into this. Thank you. It might be worth adding a comment to pg_wchar.h listing all the places that need to be fixed when enum pg_enc changes. I have developed the following patch against CVS. Tatsuo, you can use it as a starting point. It adds a comment to encnames.c and reorders utf8_and_iso8859.c to match the existing order. I also added the missing entries at the bottom. I checked for pg_conv_map in the source code and only utf8_and_iso8859.c has that structure, so I assume it is the only one that also depends on the encnames.c ordering. I think the current implementaion in utf8_and_iso8859.c is fast but too fragile against rearranging of encoding id. I modify those functions in utf8_and_iso8859.c to do a linear search with encoding id. With this change developers feel free to rearrange encoding id, and this kind of problems will be gone forever. The only penalty is the time of searching 13 entries in the encoding map. We can do a quick sort but it will need sorted entry by encoding id and may cause similar problem in the future. So I'm not sure it's worth doing the quick sort. Propsed patch attached. Looking at 8.0.X, it has the matching order, so we are OK there, but it doesn't have the trailing entries. Tatsuo, are those needed? I think it's OK, since the last missing entry will never be visited. -- Tatsuo Ishii SRA OSS, Inc. Japan Index: src/backend/utils/mb/conversion_procs/utf8_and_iso8859/utf8_and_iso8859.c === RCS file: /cvsroot/pgsql/src/backend/utils/mb/conversion_procs/utf8_and_iso8859/utf8_and_iso8859.c,v retrieving revision 1.16 diff -u -r1.16 utf8_and_iso8859.c --- src/backend/utils/mb/conversion_procs/utf8_and_iso8859/utf8_and_iso8859.c 22 Nov 2005 18:17:26 - 1.16 +++ src/backend/utils/mb/conversion_procs/utf8_and_iso8859/utf8_and_iso8859.c 23 Dec 2005 01:43:38 - @@ -68,15 +68,6 @@ } pg_conv_map; static pg_conv_map maps[] = { - {PG_SQL_ASCII}, /* SQL/ASCII */ - {PG_EUC_JP},/* EUC for Japanese */ - {PG_EUC_CN},/* EUC for Chinese */ - {PG_EUC_KR},/* EUC for Korean */ - {PG_EUC_TW},/* EUC for Taiwan */ - {PG_JOHAB}, /* EUC for Korean JOHAB */ - {PG_UTF8}, /* Unicode UTF8 */ - {PG_MULE_INTERNAL}, /* Mule internal code */ - {PG_LATIN1},/* ISO-8859-1 Latin 1 */ {PG_LATIN2, LUmapISO8859_2, ULmapISO8859_2, sizeof(LUmapISO8859_2) / sizeof(pg_local_to_utf), sizeof(ULmapISO8859_2) / sizeof(pg_utf_to_local)}, /* ISO-8859-2 Latin 2 */ @@ -104,12 +95,6 @@ {PG_LATIN10, LUmapISO8859_16, ULmapISO8859_16, sizeof(LUmapISO8859_16) / sizeof(pg_local_to_utf), sizeof(ULmapISO8859_16) / sizeof(pg_utf_to_local)}, /* ISO-8859-16 Latin 10 */ - {PG_WIN1256}, /* windows-1256 */ - {PG_WIN1258}, /* Windows-1258 */ - {PG_WIN874},/* windows-874 */ - {PG_KOI8R}, /* KOI8-R */ - {PG_WIN1251}, /* windows-1251 */ - {PG_WIN866},/* (MS-DOS CP866) */ {PG_ISO_8859_5, LUmapISO8859_5, ULmapISO8859_5, sizeof(LUmapISO8859_5) / sizeof(pg_local_to_utf), sizeof(ULmapISO8859_5) / sizeof(pg_utf_to_local)}, /* ISO-8859-5 */ @@ -131,11 +116,23 @@ unsigned char *src = (unsigned char *) PG_GETARG_CSTRING(2); unsigned char *dest = (unsigned char *) PG_GETARG_CSTRING(3); int len = PG_GETARG_INT32(4); + int i; Assert(PG_GETARG_INT32(1) == PG_UTF8); Assert(len = 0); - LocalToUtf(src, dest, maps[encoding].map1, maps[encoding].size1, encoding, len); + for (i=0;isizeof(maps)/sizeof(pg_conv_map);i++) + { + if (encoding == maps[i].encoding) + { + LocalToUtf(src, dest, maps[i].map1, maps[i].size1, encoding, len); + PG_RETURN_VOID(); + } + } + + ereport(ERROR, + (errcode(ERRCODE_INTERNAL_ERROR), +errmsg(unexpected encoding id %d for ISO-8859 charsets, encoding))); PG_RETURN_VOID(); } @@ -147,11 +144,23 @@ unsigned char *src = (unsigned char *) PG_GETARG_CSTRING(2); unsigned char *dest = (unsigned char *) PG_GETARG_CSTRING(3); int len =
Re: [PATCHES] [BUGS] BUG #2120: Crash when doing UTF8-ISO_8859_8 encoding conversion
That is a nice solution --- instead of listing all the encodings, you listed just the ones that need to be used. The list is shorter and clearer. It seems like the right approach. Thanks. --- Tatsuo Ishii wrote: Tom Lane wrote: Tatsuo Ishii [EMAIL PROTECTED] writes: It looks like somebody rearranged the pg_enc enum without bothering to fix the tables that are affected by this. I will look into this. Thank you. It might be worth adding a comment to pg_wchar.h listing all the places that need to be fixed when enum pg_enc changes. I have developed the following patch against CVS. Tatsuo, you can use it as a starting point. It adds a comment to encnames.c and reorders utf8_and_iso8859.c to match the existing order. I also added the missing entries at the bottom. I checked for pg_conv_map in the source code and only utf8_and_iso8859.c has that structure, so I assume it is the only one that also depends on the encnames.c ordering. I think the current implementaion in utf8_and_iso8859.c is fast but too fragile against rearranging of encoding id. I modify those functions in utf8_and_iso8859.c to do a linear search with encoding id. With this change developers feel free to rearrange encoding id, and this kind of problems will be gone forever. The only penalty is the time of searching 13 entries in the encoding map. We can do a quick sort but it will need sorted entry by encoding id and may cause similar problem in the future. So I'm not sure it's worth doing the quick sort. Propsed patch attached. Looking at 8.0.X, it has the matching order, so we are OK there, but it doesn't have the trailing entries. Tatsuo, are those needed? I think it's OK, since the last missing entry will never be visited. -- Tatsuo Ishii SRA OSS, Inc. Japan Index: src/backend/utils/mb/conversion_procs/utf8_and_iso8859/utf8_and_iso8859.c === RCS file: /cvsroot/pgsql/src/backend/utils/mb/conversion_procs/utf8_and_iso8859/utf8_and_iso8859.c,v retrieving revision 1.16 diff -u -r1.16 utf8_and_iso8859.c --- src/backend/utils/mb/conversion_procs/utf8_and_iso8859/utf8_and_iso8859.c 22 Nov 2005 18:17:26 - 1.16 +++ src/backend/utils/mb/conversion_procs/utf8_and_iso8859/utf8_and_iso8859.c 23 Dec 2005 01:43:38 - @@ -68,15 +68,6 @@ } pg_conv_map; static pg_conv_map maps[] = { - {PG_SQL_ASCII}, /* SQL/ASCII */ - {PG_EUC_JP},/* EUC for Japanese */ - {PG_EUC_CN},/* EUC for Chinese */ - {PG_EUC_KR},/* EUC for Korean */ - {PG_EUC_TW},/* EUC for Taiwan */ - {PG_JOHAB}, /* EUC for Korean JOHAB */ - {PG_UTF8}, /* Unicode UTF8 */ - {PG_MULE_INTERNAL}, /* Mule internal code */ - {PG_LATIN1},/* ISO-8859-1 Latin 1 */ {PG_LATIN2, LUmapISO8859_2, ULmapISO8859_2, sizeof(LUmapISO8859_2) / sizeof(pg_local_to_utf), sizeof(ULmapISO8859_2) / sizeof(pg_utf_to_local)}, /* ISO-8859-2 Latin 2 */ @@ -104,12 +95,6 @@ {PG_LATIN10, LUmapISO8859_16, ULmapISO8859_16, sizeof(LUmapISO8859_16) / sizeof(pg_local_to_utf), sizeof(ULmapISO8859_16) / sizeof(pg_utf_to_local)}, /* ISO-8859-16 Latin 10 */ - {PG_WIN1256}, /* windows-1256 */ - {PG_WIN1258}, /* Windows-1258 */ - {PG_WIN874},/* windows-874 */ - {PG_KOI8R}, /* KOI8-R */ - {PG_WIN1251}, /* windows-1251 */ - {PG_WIN866},/* (MS-DOS CP866) */ {PG_ISO_8859_5, LUmapISO8859_5, ULmapISO8859_5, sizeof(LUmapISO8859_5) / sizeof(pg_local_to_utf), sizeof(ULmapISO8859_5) / sizeof(pg_utf_to_local)}, /* ISO-8859-5 */ @@ -131,11 +116,23 @@ unsigned char *src = (unsigned char *) PG_GETARG_CSTRING(2); unsigned char *dest = (unsigned char *) PG_GETARG_CSTRING(3); int len = PG_GETARG_INT32(4); + int i; Assert(PG_GETARG_INT32(1) == PG_UTF8); Assert(len = 0); - LocalToUtf(src, dest, maps[encoding].map1, maps[encoding].size1, encoding, len); + for (i=0;isizeof(maps)/sizeof(pg_conv_map);i++) + { + if (encoding == maps[i].encoding) + { + LocalToUtf(src, dest, maps[i].map1, maps[i].size1, encoding, len); + PG_RETURN_VOID(); + } + } + + ereport(ERROR, + (errcode(ERRCODE_INTERNAL_ERROR), +
Re: [PATCHES] Disparity in search_path SHOW and SET
Agreed. I have gotten confused on how to set $user in the past. I have developed the following patch that sets the default with the double quotes around it, and it works fine. The patch also contains updated documentation. Just be careful about pg_dump's special handling of search_path in user and db variables... Make sure you haven't broken it. Chris ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] [BUGS] BUG #2120: Crash when doing UTF8-ISO_8859_8
Ok, I will commit the patches. BTW, the example code sequence (ISO-8859-8) Sagi posted seems to have wrong one. select ''; WARNING: ignoring unconvertible ISO_8859_8 character 0x00d7 : : 0x00d7(\327) is not listed in our ISO-8858-8/UTF-8 conversion map. Is this OK or do we need to add the conversion for the code? What do you think, Sega? -- Tatsuo Ishii SRA OSS, Inc. Japan That is a nice solution --- instead of listing all the encodings, you listed just the ones that need to be used. The list is shorter and clearer. It seems like the right approach. Thanks. --- Tatsuo Ishii wrote: Tom Lane wrote: Tatsuo Ishii [EMAIL PROTECTED] writes: It looks like somebody rearranged the pg_enc enum without bothering to fix the tables that are affected by this. I will look into this. Thank you. It might be worth adding a comment to pg_wchar.h listing all the places that need to be fixed when enum pg_enc changes. I have developed the following patch against CVS. Tatsuo, you can use it as a starting point. It adds a comment to encnames.c and reorders utf8_and_iso8859.c to match the existing order. I also added the missing entries at the bottom. I checked for pg_conv_map in the source code and only utf8_and_iso8859.c has that structure, so I assume it is the only one that also depends on the encnames.c ordering. I think the current implementaion in utf8_and_iso8859.c is fast but too fragile against rearranging of encoding id. I modify those functions in utf8_and_iso8859.c to do a linear search with encoding id. With this change developers feel free to rearrange encoding id, and this kind of problems will be gone forever. The only penalty is the time of searching 13 entries in the encoding map. We can do a quick sort but it will need sorted entry by encoding id and may cause similar problem in the future. So I'm not sure it's worth doing the quick sort. Propsed patch attached. Looking at 8.0.X, it has the matching order, so we are OK there, but it doesn't have the trailing entries. Tatsuo, are those needed? I think it's OK, since the last missing entry will never be visited. -- Tatsuo Ishii SRA OSS, Inc. Japan Index: src/backend/utils/mb/conversion_procs/utf8_and_iso8859/utf8_and_iso8859.c === RCS file: /cvsroot/pgsql/src/backend/utils/mb/conversion_procs/utf8_and_iso8859/utf8_and_iso8859.c,v retrieving revision 1.16 diff -u -r1.16 utf8_and_iso8859.c --- src/backend/utils/mb/conversion_procs/utf8_and_iso8859/utf8_and_iso8859.c 22 Nov 2005 18:17:26 - 1.16 +++ src/backend/utils/mb/conversion_procs/utf8_and_iso8859/utf8_and_iso8859.c 23 Dec 2005 01:43:38 - @@ -68,15 +68,6 @@ } pg_conv_map; static pg_conv_map maps[] = { - {PG_SQL_ASCII}, /* SQL/ASCII */ - {PG_EUC_JP},/* EUC for Japanese */ - {PG_EUC_CN},/* EUC for Chinese */ - {PG_EUC_KR},/* EUC for Korean */ - {PG_EUC_TW},/* EUC for Taiwan */ - {PG_JOHAB}, /* EUC for Korean JOHAB */ - {PG_UTF8}, /* Unicode UTF8 */ - {PG_MULE_INTERNAL}, /* Mule internal code */ - {PG_LATIN1},/* ISO-8859-1 Latin 1 */ {PG_LATIN2, LUmapISO8859_2, ULmapISO8859_2, sizeof(LUmapISO8859_2) / sizeof(pg_local_to_utf), sizeof(ULmapISO8859_2) / sizeof(pg_utf_to_local)}, /* ISO-8859-2 Latin 2 */ @@ -104,12 +95,6 @@ {PG_LATIN10, LUmapISO8859_16, ULmapISO8859_16, sizeof(LUmapISO8859_16) / sizeof(pg_local_to_utf), sizeof(ULmapISO8859_16) / sizeof(pg_utf_to_local)}, /* ISO-8859-16 Latin 10 */ - {PG_WIN1256}, /* windows-1256 */ - {PG_WIN1258}, /* Windows-1258 */ - {PG_WIN874},/* windows-874 */ - {PG_KOI8R}, /* KOI8-R */ - {PG_WIN1251}, /* windows-1251 */ - {PG_WIN866},/* (MS-DOS CP866) */ {PG_ISO_8859_5, LUmapISO8859_5, ULmapISO8859_5, sizeof(LUmapISO8859_5) / sizeof(pg_local_to_utf), sizeof(ULmapISO8859_5) / sizeof(pg_utf_to_local)}, /* ISO-8859-5 */ @@ -131,11 +116,23 @@ unsigned char *src = (unsigned char *) PG_GETARG_CSTRING(2); unsigned char *dest = (unsigned char *) PG_GETARG_CSTRING(3); int len = PG_GETARG_INT32(4); + int i; Assert(PG_GETARG_INT32(1) == PG_UTF8); Assert(len = 0); - LocalToUtf(src, dest,
Re: [PATCHES] Disparity in search_path SHOW and SET
Christopher Kings-Lynne wrote: Agreed. I have gotten confused on how to set $user in the past. I have developed the following patch that sets the default with the double quotes around it, and it works fine. The patch also contains updated documentation. Just be careful about pg_dump's special handling of search_path in user and db variables... Make sure you haven't broken it. Uh, could you provide a test I can do? The code is already in CVS. -- Bruce Momjian| http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] [BUGS] BUG #2120: Crash when doing UTF8-ISO_8859_8 encoding
Tatsuo Ishii [EMAIL PROTECTED] writes: I think the current implementaion in utf8_and_iso8859.c is fast but too fragile against rearranging of encoding id. I modify those functions in utf8_and_iso8859.c to do a linear search with encoding id. That's not unreasonable, but I was wondering whether we could add some Assert() tests that would catch problems without imposing any extra cost in normal non-Assert builds. regards, tom lane ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster