Re: [PATCHES] pg_dump: fix crash on error
Neil Conway <[EMAIL PROTECTED]> writes: > Tom Lane wrote: >> [ scratches head... ] Did this code change recently? It's a tad hard >> to believe that such a thing could have gone unnoticed for long. > Seems the problem was introduced here: > http://developer.postgresql.org/cvsweb.cgi/pgsql/src/bin/pg_dump/pg_backup_db.c.diff?r1=1.58;r2=1.59 Oh dear, that makes it my fault :-( > (I could just revert the code to using PQdb(), but I don't think there's > a need to mention the database in the error message ourselves -- > PQerrorMessage() should include that information if appropriate.) I'd go with reverting it to using PQdb, myself. Looks like I got bit by the perennial mantra: premature optimization is the root of all evil. regards, tom lane ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] pg_dump: fix crash on error
Tom Lane wrote: I'd go with reverting it to using PQdb, myself. Fair enough; reverted to using PQdb(), and applied to HEAD and REL8_0_STABLE. -Neil ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] pg_dump: fix crash on error
Tom Lane wrote: [ scratches head... ] Did this code change recently? It's a tad hard to believe that such a thing could have gone unnoticed for long. Seems the problem was introduced here: http://developer.postgresql.org/cvsweb.cgi/pgsql/src/bin/pg_dump/pg_backup_db.c.diff?r1=1.58;r2=1.59 (I could just revert the code to using PQdb(), but I don't think there's a need to mention the database in the error message ourselves -- PQerrorMessage() should include that information if appropriate.) -Neil ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] pg_dump: fix crash on error
Neil Conway <[EMAIL PROTECTED]> writes: > If pg_dump fails to connect to Postgres, it attempts to print an error > message in ConnectDatabase(): > ... > But if no database is explicitly specified, `dbname' is NULL, and libc > is entitled to crash if you pass a NULL pointer to it for a %s > formatting sequence (it actually does crash on Solaris, for example -- > per report from Omar Kilani). [ scratches head... ] Did this code change recently? It's a tad hard to believe that such a thing could have gone unnoticed for long. 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
[PATCHES] pg_dump: fix crash on error
If pg_dump fails to connect to Postgres, it attempts to print an error message in ConnectDatabase(): /* check to see that the backend connection was successfully made */ if (PQstatus(AH->connection) == CONNECTION_BAD) die_horribly(AH, modulename, "connection to database \"%s\" failed: %s", dbname, PQerrorMessage(AH->connection)); But if no database is explicitly specified, `dbname' is NULL, and libc is entitled to crash if you pass a NULL pointer to it for a %s formatting sequence (it actually does crash on Solaris, for example -- per report from Omar Kilani). Attached is a patch that fixes this by just removing `dbname' from the error message, as PQerrorMessage() should provide enough information. Barring any objections, I'll apply this to HEAD and back branches within 24 hours. -Neil Index: src/bin/pg_dump/pg_backup_db.c === RCS file: /var/lib/cvs/pgsql/src/bin/pg_dump/pg_backup_db.c,v retrieving revision 1.63 diff -c -r1.63 pg_backup_db.c *** src/bin/pg_dump/pg_backup_db.c 1 Jul 2005 21:03:25 - 1.63 --- src/bin/pg_dump/pg_backup_db.c 27 Jul 2005 01:23:27 - *** *** 265,272 /* check to see that the backend connection was successfully made */ if (PQstatus(AH->connection) == CONNECTION_BAD) ! die_horribly(AH, modulename, "connection to database \"%s\" failed: %s", ! dbname, PQerrorMessage(AH->connection)); /* check for version mismatch */ _check_database_version(AH, ignoreVersion); --- 265,272 /* check to see that the backend connection was successfully made */ if (PQstatus(AH->connection) == CONNECTION_BAD) ! die_horribly(AH, modulename, "connection to database failed: %s", ! PQerrorMessage(AH->connection)); /* check for version mismatch */ _check_database_version(AH, ignoreVersion); ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] pg_dump fix
Patch applied. Thanks. --- Rod Taylor wrote: -- Start of PGP signed section. > Seems my check constraint change did break stuff. > > Alias the appropriate columns back to their original name. > > Fixed formatting of a few other places as I went along (indenting) > -- > Rod Taylor <[EMAIL PROTECTED]> > > PGP Key: http://www.rbt.ca/rbtpub.asc [ Attachment, skipping... ] -- End of PGP section, PGP failed! -- Bruce Momjian| http://candle.pha.pa.us [EMAIL PROTECTED] | (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 8: explain analyze is your friend
Re: [PATCHES] pg_dump fix
Your patch has been added to the PostgreSQL unapplied patches list at: http://momjian.postgresql.org/cgi-bin/pgpatches I will try to apply it within the next 48 hours. --- Rod Taylor wrote: -- Start of PGP signed section. > Seems my check constraint change did break stuff. > > Alias the appropriate columns back to their original name. > > Fixed formatting of a few other places as I went along (indenting) > -- > Rod Taylor <[EMAIL PROTECTED]> > > PGP Key: http://www.rbt.ca/rbtpub.asc [ Attachment, skipping... ] -- End of PGP section, PGP failed! -- Bruce Momjian| http://candle.pha.pa.us [EMAIL PROTECTED] | (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 9: the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
[PATCHES] pg_dump fix
Seems my check constraint change did break stuff.
Alias the appropriate columns back to their original name.
Fixed formatting of a few other places as I went along (indenting)
--
Rod Taylor <[EMAIL PROTECTED]>
PGP Key: http://www.rbt.ca/rbtpub.asc
Ite ndex: src/bin/pg_dump/pg_dump.c
===
RCS file: /home/rbt/work/postgresql/cvs/pgsql-server/src/bin/pg_dump/pg_dump.c,v
retrieving revision 1.335
diff -c -r1.335 pg_dump.c
*** src/bin/pg_dump/pg_dump.c 25 Jun 2003 04:08:19 - 1.335
--- src/bin/pg_dump/pg_dump.c 26 Jun 2003 13:56:55 -
***
*** 3304,3316
* Fetch and process CHECK constraints for the domain
*/
if (g_fout->remoteVersion >= 70400)
! appendPQExpBuffer(chkquery, "SELECT conname,"
"pg_catalog.pg_get_constraintdef(oid) AS consrc "
"FROM pg_catalog.pg_constraint "
"WHERE contypid =
'%s'::pg_catalog.oid",
tinfo->oid);
else
! appendPQExpBuffer(chkquery, "SELECT conname, 'CHECK (' || consrc ||
')'"
"FROM pg_catalog.pg_constraint "
"WHERE contypid =
'%s'::pg_catalog.oid",
tinfo->oid);
--- 3304,3316
* Fetch and process CHECK constraints for the domain
*/
if (g_fout->remoteVersion >= 70400)
! appendPQExpBuffer(chkquery, "SELECT conname, "
"pg_catalog.pg_get_constraintdef(oid) AS consrc "
"FROM pg_catalog.pg_constraint "
"WHERE contypid =
'%s'::pg_catalog.oid",
tinfo->oid);
else
! appendPQExpBuffer(chkquery, "SELECT conname, 'CHECK (' || consrc ||
')' AS consrc "
"FROM pg_catalog.pg_constraint "
"WHERE contypid =
'%s'::pg_catalog.oid",
tinfo->oid);
***
*** 5267,5274
if (g_fout->remoteVersion >= 70400)
appendPQExpBuffer(query, "SELECT conname, "
"
pg_catalog.pg_get_constraintdef(c1.oid) AS consrc "
! " from
pg_catalog.pg_constraint c1"
! " where conrelid =
'%s'::pg_catalog.oid "
" and contype =
'c' "
" and not exists "
" (select 1 from "
--- 5267,5274
if (g_fout->remoteVersion >= 70400)
appendPQExpBuffer(query, "SELECT conname, "
"
pg_catalog.pg_get_constraintdef(c1.oid) AS consrc "
! " from
pg_catalog.pg_constraint c1 "
! " where conrelid =
'%s'::pg_catalog.oid "
" and contype =
'c' "
" and not exists "
" (select 1 from "
***
*** 5286,5292
tbinfo->oid);
else if (g_fout->remoteVersion >= 70300)
appendPQExpBuffer(query, "SELECT conname, "
! " 'CHECK (' ||
consrc || ')'"
" from
pg_catalog.pg_constraint c1"
" where conrelid =
'%s'::pg_catalog.oid "
" and contype =
'c' "
--- 5286,5292
tbinfo->oid);
else if (g_fout->remoteVersion >= 70300)
appendPQExpBuffer(query, "SELECT conname, "
! " 'CHECK (' ||
consrc || ')' AS consrc"
" from
p
