Re: [PATCHES] [PATCH] Add support for IS NULL to btree indexes
On Mon, Sep 19, 2005 at 04:33:27PM -0400, Tom Lane wrote: This is a bad idea, because it translates x IS NULL into x = NULL which is under no circumstances the same thing. It might coincidentally fail to malfunction for btree indexes, depending on the specifics of the = operator in use; but that doesn't make it right. (AFAICS, the proposed patch simply breaks for non-btree indexes.) Also, it cannot handle IS NOT NULL. That's point of the extra flag, to distinguish between the two cases. It works for all types, the actual operator in the operator class is never invoked (they're strict, the code can't call them with NULL even if it wanted to). ExecInitIndexScan has always set SK_ISNULL for null args, even though it never happened in practice. All it does is move to the point in the index where the NULLs begin and start returning tuples. It takes a previously impossible state and makes it return something useful. It doesn't handle IS NOT NULL, since I don't think that's worth indexing anyway, but perhaps for completeness. It the much harder case. A proper solution requires explicitly representing IS NULL/IS NOT NULL as distinct kinds of scankey. Well, this patch has a kind for IS NULL. It does have issues with other indexes, but there's no point working on that until there is a possibility of acceptance. The purpose was to demonstrate that it is trivially possible using what is available. -- Martijn van Oosterhout kleptog@svana.org http://svana.org/kleptog/ Patent. n. Genius is 5% inspiration and 95% perspiration. A patent is a tool for doing 5% of the work and then sitting around waiting for someone else to do the other 95% so you can sue them. pgp5vqkqJ5q4F.pgp Description: PGP signature
[PATCHES] Multiple -t options for pg_dump
Folks,
Please find enclosed a patch against CVS TIP that allows people to
specify more than one table via pg_dump -t.
Cheers,
D
--
David Fetter [EMAIL PROTECTED] http://fetter.org/
phone: +1 510 893 6100 mobile: +1 415 235 3778
Remember to vote!
Index: doc/src/sgml/ref/pg_dump.sgml
===
RCS file: /projects/cvsroot/pgsql/doc/src/sgml/ref/pg_dump.sgml,v
retrieving revision 1.80
diff -c -r1.80 pg_dump.sgml
*** doc/src/sgml/ref/pg_dump.sgml 25 Jul 2005 22:12:31 - 1.80
--- doc/src/sgml/ref/pg_dump.sgml 20 Sep 2005 15:49:33 -
***
*** 388,406
listitem
para
Dump data for replaceable class=parametertable/replaceable
! only. It is possible for there to be
! multiple tables with the same name in different schemas; if that
! is the case, all matching tables will be dumped. Specify both
! option--schema/ and option--table/ to select just one table.
/para
note
para
! In this mode, applicationpg_dump/application makes no
! attempt to dump any other database objects that the selected table
! may depend upon. Therefore, there is no guarantee
! that the results of a single-table dump can be successfully
! restored by themselves into a clean database.
/para
/note
/listitem
--- 388,408
listitem
para
Dump data for replaceable class=parametertable/replaceable
! only. This option can be repeated to get more than one table.
! It is possible for there to be multiple tables with the same name in
! different schemas. If that is the case, pg_dump will dump all
! matching tables. Specify both option--schema/ and
! option--table/ to select just the table or tables from
! that schema.
/para
note
para
! In this mode, applicationpg_dump/application makes no
! attempt to dump any other database objects that the selected
! tables may depend upon. Therefore, there is no guarantee that
! the results of a selected-table dump can be successfully
! restored by themselves into a clean database.
/para
/note
/listitem
Index: src/bin/pg_dump/pg_dump.c
===
RCS file: /projects/cvsroot/pgsql/src/bin/pg_dump/pg_dump.c,v
retrieving revision 1.420
diff -c -r1.420 pg_dump.c
*** src/bin/pg_dump/pg_dump.c 5 Sep 2005 23:50:48 - 1.420
--- src/bin/pg_dump/pg_dump.c 20 Sep 2005 15:49:40 -
***
*** 93,100
/* obsolete as of 7.3: */
static Oidg_last_builtin_oid; /* value of the last builtin oid */
! static char *selectTableName = NULL; /* name of a single table to dump */
! static char *selectSchemaName = NULL; /* name of a single schema to dump */
char g_opaque_type[10]; /* name for the opaque type */
--- 93,102
/* obsolete as of 7.3: */
static Oidg_last_builtin_oid; /* value of the last builtin oid */
! static char **selectTableNames = NULL;/* name(s) of specified
table(s) to dump */
! int tab_max = 0, tab_idx = 0;
! static char *selectSchemaName = NULL; /* name(s) of specified schema(ta) to
dump */
! int schem_max = 0, schem_idx = 0;
char g_opaque_type[10]; /* name for the opaque type */
***
*** 362,369
outputSuperuser = strdup(optarg);
break;
! case 't': /* Dump data for this
table only */
! selectTableName = strdup(optarg);
break;
case 'u':
--- 364,376
outputSuperuser = strdup(optarg);
break;
! case 't': /* Dump data for
th(is|ese) table(s) only */
! if (tab_idx == tab_max) {
! tab_max += 32;
! if ( (selectTableNames =
realloc(selectTableNames, tab_max*sizeof(char *))) == 0)
! exit(-1);
! }
! selectTableNames[tab_idx++] = strdup(optarg);
break;
case 'u':
***
*** 450,456
exit(1);
}
! if (selectTableName != NULL || selectSchemaName != NULL)
outputBlobs = false;
if (dumpInserts == true oids == true)
--- 457,463
exit(1);
}
! if (selectTableNames != NULL || selectSchemaName != NULL)
outputBlobs =
Re: [PATCHES] Multiple -t options for pg_dump
A few minor comments are below. This is for 8.2, right?
On Tue, 2005-20-09 at 08:51 -0700, David Fetter wrote:
/* obsolete as of 7.3: */
static Oidg_last_builtin_oid; /* value of the last builtin oid
*/
! static char **selectTableNames = NULL;/* name(s) of
specified table(s) to dump */
! int tab_max = 0, tab_idx = 0;
! static char *selectSchemaName = NULL; /* name(s) of specified
schema(ta) to dump */
! int schem_max = 0, schem_idx = 0;
char g_opaque_type[10]; /* name for the opaque type */
AFAICS the schema changes are unused. However, it would be worth
enhancing -n to allow it to be specified multiple times as well. And to
nit-pick, tab_max and tab_idx should be static. Also, within the PG
source the plural of schema is schemas.
! case 't': /* Dump data
for th(is|ese) table(s) only */
! if (tab_idx == tab_max) {
! tab_max += 32;
! if ( (selectTableNames =
realloc(selectTableNames, tab_max*sizeof(char *))) == 0)
! exit(-1);
! }
If you're going to check for calloc failure, you may as well check for
strdup failure as well. Also, exit(1) would be more consistent with the
rest of pg_dump. But personally I wouldn't bother checking for calloc
(or strdup) failure, as the rest of pg_dump doesn't.
*** 2414,2420
{
PGresult *res;
int ntups;
! int i;
PQExpBuffer query = createPQExpBuffer();
PQExpBuffer delqry = createPQExpBuffer();
PQExpBuffer lockquery = createPQExpBuffer();
--- 2427,2433
{
PGresult *res;
int ntups;
! int i,j;
PQExpBuffer query = createPQExpBuffer();
PQExpBuffer delqry = createPQExpBuffer();
PQExpBuffer lockquery = createPQExpBuffer();
You should move j's definition to the scope closest to its use.
--- 2706,2724
* simplistic since we don't fully check the combination of -n
and -t
* switches.)
*/
! if (selectTableNames != NULL)
{
! for (i = 0; i ntups; i++) {
! for (j = 0; j tab_idx; j++) {
! if (strcmp(tblinfo[i].dobj.name,
selectTableNames[j]) == 0)
! goto check_match;
! }
! }
/* Didn't find a match */
! check_match: if (i == ntups)
{
write_msg(NULL, specified table \%s\ does
not exist\n,
! selectTableNames[j]);
exit_nicely();
}
}
AFAICS the goto is unnecessary -- just break out of the loop when you
find a match, and test i == ntups outside the loop.
-Neil
---(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] Multiple -t options for pg_dump
On Tue, Sep 20, 2005 at 12:15:23PM -0400, Neil Conway wrote:
A few minor comments are below. This is for 8.2, right?
I am hoping to make a case for inclusion in 8.1. The code is
completely isolated in one command and does not affect anyone who
does not use the new features.
On Tue, 2005-20-09 at 08:51 -0700, David Fetter wrote:
/* obsolete as of 7.3: */
static Oidg_last_builtin_oid; /* value of the last builtin oid
*/
! static char **selectTableNames = NULL;/* name(s) of
specified table(s) to dump */
! int tab_max = 0, tab_idx = 0;
! static char *selectSchemaName = NULL; /* name(s) of specified
schema(ta) to dump */
! int schem_max = 0, schem_idx = 0;
char g_opaque_type[10]; /* name for the opaque type */
AFAICS the schema changes are unused. However, it would be worth
enhancing -n to allow it to be specified multiple times as well. And
to nit-pick, tab_max and tab_idx should be static. Also, within
the PG source the plural of schema is schemas.
OK, will fix when I get home from work.
! case 't': /* Dump data
for th(is|ese) table(s) only */
! if (tab_idx == tab_max) {
! tab_max += 32;
! if ( (selectTableNames =
realloc(selectTableNames, tab_max*sizeof(char *))) == 0)
! exit(-1);
! }
If you're going to check for calloc failure, you may as well check for
strdup failure as well. Also, exit(1) would be more consistent with the
rest of pg_dump. But personally I wouldn't bother checking for calloc
(or strdup) failure, as the rest of pg_dump doesn't.
OK :)
*** 2414,2420
{
PGresult *res;
int ntups;
! int i;
PQExpBuffer query = createPQExpBuffer();
PQExpBuffer delqry = createPQExpBuffer();
PQExpBuffer lockquery = createPQExpBuffer();
--- 2427,2433
{
PGresult *res;
int ntups;
! int i,j;
PQExpBuffer query = createPQExpBuffer();
PQExpBuffer delqry = createPQExpBuffer();
PQExpBuffer lockquery = createPQExpBuffer();
You should move j's definition to the scope closest to its use.
OK
--- 2706,2724
* simplistic since we don't fully check the combination of -n
and -t
* switches.)
*/
! if (selectTableNames != NULL)
{
! for (i = 0; i ntups; i++) {
! for (j = 0; j tab_idx; j++) {
! if (strcmp(tblinfo[i].dobj.name,
selectTableNames[j]) == 0)
! goto check_match;
! }
! }
/* Didn't find a match */
! check_match: if (i == ntups)
{
write_msg(NULL, specified table \%s\ does
not exist\n,
! selectTableNames[j]);
exit_nicely();
}
}
AFAICS the goto is unnecessary -- just break out of the loop when you
find a match, and test i == ntups outside the loop.
I tried that. How do I break out of both loops without a goto?
Cheers,
D
--
David Fetter [EMAIL PROTECTED] http://fetter.org/
phone: +1 510 893 6100 mobile: +1 415 235 3778
Remember to vote!
---(end of broadcast)---
TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] Multiple -t options for pg_dump
On Tue, 2005-20-09 at 11:33 -0700, David Fetter wrote:
I am hoping to make a case for inclusion in 8.1. The code is
completely isolated in one command and does not affect anyone who
does not use the new features.
On those grounds we could include a lot of new features during the 8.1
beta period. IMHO it is too late for new features at this point in the
release cycle.
AFAICS the goto is unnecessary -- just break out of the loop when you
find a match, and test i == ntups outside the loop.
I tried that. How do I break out of both loops without a goto?
Oh, yeah, I guess you need a goto, but you can at least avoid rechecking
the loop condition:
for (;;)
{
for (;;)
{
if (match_found)
goto done;
}
}
/* If we got here, no match */
error();
done: /* Otherwise we're good */
-Neil
---(end of broadcast)---
TIP 4: Have you searched our list archives?
http://archives.postgresql.org
Re: [PATCHES] [PORTS] Solaris - psql returns 0 instead of 1 for file not found.
I have applied the following patch to 8.1beta and 8.0.X to return the
proper failure value for a psql -f filename open failure.
The bug was that process_file() was returning false for failure, while
the call site expected MainLoop() return values, meaning false/0 was
success.
---
Clark, Andrew wrote:
Hi all,
I've found the when psql is used with the -f flag and the specified file
doesn't exist the rc value is 0.
$ uname -a
SunOS bld 5.8 Generic_108528-29 sun4u sparc SUNW,Sun-Fire-V440
$ psql -V
psql (PostgreSQL) 8.0.2
$ ls foo
foo: No such file or directory
$ psql -f foo
foo: No such file or directory
$ echo $?
0
However, in the man page for psql it says:
EXIT STATUS
psql returns 0 to the shell if it finished normally, 1 if a
fatal error of its own (out of memory, file not found)
occurs, ...
I'm assuming this is the same with other platforms, but I only use
PostgreSQL on Solaris.
Has this been fix in 8.0.3?
Cheers,
Andrew
--
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/bin/psql/command.c
===
RCS file: /cvsroot/pgsql/src/bin/psql/command.c,v
retrieving revision 1.152
diff -c -c -r1.152 command.c
*** src/bin/psql/command.c 14 Aug 2005 18:49:30 - 1.152
--- src/bin/psql/command.c 20 Sep 2005 18:43:41 -
***
*** 1314,1320
* process_file
*
* Read commands from filename and then them to the main processing loop
! * Handler for \i, but can be used for other things as well.
*/
int
process_file(char *filename)
--- 1314,1321
* process_file
*
* Read commands from filename and then them to the main processing loop
! * Handler for \i, but can be used for other things as well. Returns
! * MainLoop() error code.
*/
int
process_file(char *filename)
***
*** 1324,1330
char *oldfilename;
if (!filename)
! return false;
canonicalize_path(filename);
fd = fopen(filename, PG_BINARY_R);
--- 1325,1331
char *oldfilename;
if (!filename)
! return EXIT_FAILURE;
canonicalize_path(filename);
fd = fopen(filename, PG_BINARY_R);
***
*** 1332,1338
if (!fd)
{
psql_error(%s: %s\n, filename, strerror(errno));
! return false;
}
oldfilename = pset.inputfile;
--- 1333,1339
if (!fd)
{
psql_error(%s: %s\n, filename, strerror(errno));
! return EXIT_FAILURE;
}
oldfilename = pset.inputfile;
Index: src/bin/psql/startup.c
===
RCS file: /cvsroot/pgsql/src/bin/psql/startup.c,v
retrieving revision 1.122
diff -c -c -r1.122 startup.c
*** src/bin/psql/startup.c 5 Sep 2005 18:05:13 - 1.122
--- src/bin/psql/startup.c 20 Sep 2005 18:43:41 -
***
*** 690,698
sprintf(psqlrc, %s-%s, filename, PG_VERSION);
if (access(psqlrc, R_OK) == 0)
! process_file(psqlrc);
else if (access(filename, R_OK) == 0)
! process_file(filename);
free(psqlrc);
}
--- 690,698
sprintf(psqlrc, %s-%s, filename, PG_VERSION);
if (access(psqlrc, R_OK) == 0)
! (void)process_file(psqlrc);
else if (access(filename, R_OK) == 0)
! (void)process_file(filename);
free(psqlrc);
}
---(end of broadcast)---
TIP 4: Have you searched our list archives?
http://archives.postgresql.org
Re: [PATCHES] Multiple -t options for pg_dump
On Tue, Sep 20, 2005 at 11:33:39AM -0700, David Fetter wrote: On Tue, Sep 20, 2005 at 12:15:23PM -0400, Neil Conway wrote: A few minor comments are below. This is for 8.2, right? I am hoping to make a case for inclusion in 8.1. The code is completely isolated in one command and does not affect anyone who does not use the new features. On these grounds, this feature should have been included back when 8.1 started, because a patch to do this was posted when 8.0 was in beta ... In fact, IIRC, this is the third time a patch for this is posted, all of them in late beta :-( -- Alvaro Herrera Architect, http://www.EnterpriseDB.com 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 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] Bug in psql (on_error_rollback)
Good patch. You are right that the original code did not consider that
AUTOCOMMIT would change the transaction state seen by the
ON_ERROR_ROLLBACK check.
Fixed in CVS. ON_ERROR_ROLLBACK was not in 8.0.X so no need to backpatch.
---
Michael Paesold wrote:
There is a bug in psql for the new ON_ERROR_ROLLBACK feature. In AUTOCOMMIT
off mode it does not work correctly for the first statement.
This is how it works usually:
postgres=# \set AUTOCOMMIT off
postgres=# \set ON_ERROR_ROLLBACK interactive
postgres=# SELECT 1;
?column?
--
1
(1 row)
postgres=# SELECT a;
ERROR: column a does not exist
postgres=# SELECT 1;
?column?
--
1
(1 row)
postgres=# BEGIN;
WARNING: there is already a transaction in progress
BEGIN
postgres=# ROLLBACK;
ROLLBACK
For the first statement in a transaction after the implicit BEGIN it does
not work:
postgres=# ROLLBACK;
ROLLBACK
postgres=#
postgres=# SELECT a;
ERROR: column a does not exist
postgres=# SELECT 1;
ERROR: current transaction is aborted, commands ignored until end of
transaction block
With the attaced patch it works correctly even for the first statement.
postgres=# \set AUTOCOMMIT off
postgres=# \set ON_ERROR_ROLLBACK interactive
postgres=# SELECT a;
ERROR: column a does not exist
postgres=# SELECT 1;
?column?
--
1
(1 row)
postgres=# BEGIN;
WARNING: there is already a transaction in progress
BEGIN
postgres=# ABORT;
ROLLBACK
Please check the patch and apply to CVS tip.
I think it would be good to add regression tests for AUTOCOMMIT and
ON_ERROR_ROLLBACK and possibly others. There are currently no regression
tests specifically for psql features, but since the regression tests are
executed via psql, there would be no problem in creating a set of such
tests, right?. I could write some.
Best Regards,
Michael Paesold
[ Attachment, skipping... ]
---(end of broadcast)---
TIP 2: Don't 'kill -9' the postmaster
--
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/bin/psql/common.c
===
RCS file: /cvsroot/pgsql/src/bin/psql/common.c,v
retrieving revision 1.104
diff -c -c -r1.104 common.c
*** src/bin/psql/common.c 22 Jun 2005 21:14:30 - 1.104
--- src/bin/psql/common.c 20 Sep 2005 21:41:45 -
***
*** 1010,1022
return false;
}
PQclear(results);
}
! else if (transaction_status == PQTRANS_INTRANS
!(rollback_str = GetVariable(pset.vars,
ON_ERROR_ROLLBACK)) != NULL
!/* !off and !interactive is 'on' */
!pg_strcasecmp(rollback_str, off) != 0
!(pset.cur_cmd_interactive ||
! pg_strcasecmp(rollback_str, interactive) != 0))
{
if (on_error_rollback_warning == false pset.sversion 8)
{
--- 1010,1024
return false;
}
PQclear(results);
+ transaction_status = PQtransactionStatus(pset.db);
}
!
! if (transaction_status == PQTRANS_INTRANS
! (rollback_str = GetVariable(pset.vars, ON_ERROR_ROLLBACK)) !=
NULL
! /* !off and !interactive is 'on' */
! pg_strcasecmp(rollback_str, off) != 0
! (pset.cur_cmd_interactive ||
!pg_strcasecmp(rollback_str, interactive) != 0))
{
if (on_error_rollback_warning == false pset.sversion 8)
{
---(end of broadcast)---
TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] Multiple -t options for pg_dump
David Fetter [EMAIL PROTECTED] writes: I am hoping to make a case for inclusion in 8.1. We are months past feature freeze. Don't waste your breath. regards, tom lane ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
