[PATCHES] Feature: give pg_dump a WHERE clause expression
Greetings,
I have developed a fairly simple patch to the pg_dump utility. It is
against version 8.3.1 source code.
I have added a new parameter, -w/--where=EXPR
This lets you specify an expression that will be used in a WHERE
clause when the data is dumped.
I have implemented and tested that it works when generating either
COPY statements (the default), or INSERT statements (-d and -D). These
two modes of operation have two different sections of code that select
the data to be dumped.
Though this change could arguably be removed, when a -w/--where
expression is specified, it is also indicated in the comments of the
dump output so one viewing the dump can see that it was not necessarily
all of the data.
When -w/--where is not specified, the dump output is just as if this
patch had not been applied.
I've also updated the pg_dump.sgml file to add a description of this
new flag.
The code changes should also conform to the existing code style within
pg_dump.
The patch should be applied from the root of the source tree with a
-p1 option to the patch command.
Please give any feedback if the patch needs improvement
Thanks for a great DB!
diff -cr postgresql-8.3.1.orig/doc/src/sgml/ref/pg_dump.sgml postgresql-8.3.1/doc/src/sgml/ref/pg_dump.sgml
*** postgresql-8.3.1.orig/doc/src/sgml/ref/pg_dump.sgml 2008-05-31 17:48:38.0 -0500
--- postgresql-8.3.1/doc/src/sgml/ref/pg_dump.sgml 2008-06-01 00:23:32.0 -0500
***
*** 535,540
--- 535,562
/varlistentry
varlistentry
+ termoption-w replaceable class=parameterexpression/replaceable/option/term
+ termoption--where=replaceable class=parameterexpression/replaceable/option/term
+ listitem
+para
+ Dump only table data for which the given commandWHERE/command
+ clause expression is true. The given expression is used for all
+ tables that will be dumped. Therefore, an error will occur if the
+ expression refers to columns that do not exist in the table being
+ dumped.
+/para
+
+note
+ para
+ When giving the expression at the shell prompt, it must be correctly
+ quoted and escaped since otherwise it may be received as multiple
+ arguments to applicationpg_dump/application.
+ /para
+/note
+ /listitem
+ /varlistentry
+
+ varlistentry
termoption-x//term
termoption--no-privileges//term
termoption--no-acl//term
diff -cr postgresql-8.3.1.orig/src/bin/pg_dump/pg_dump.c postgresql-8.3.1/src/bin/pg_dump/pg_dump.c
*** postgresql-8.3.1.orig/src/bin/pg_dump/pg_dump.c 2008-05-31 17:48:25.0 -0500
--- postgresql-8.3.1/src/bin/pg_dump/pg_dump.c 2008-06-01 00:33:30.0 -0500
***
*** 76,81
--- 76,82
bool attrNames; /* put attr names into insert strings */
bool schemaOnly;
bool dataOnly;
+ char *whereClauseExpr;
bool aclsSkip;
/* subquery used to convert user ID (eg, datdba) to user name */
***
*** 191,197
static void dumpEncoding(Archive *AH);
static void dumpStdStrings(Archive *AH);
static const char *getAttrName(int attrnum, TableInfo *tblInfo);
! static const char *fmtCopyColumnList(const TableInfo *ti);
static void do_sql_command(PGconn *conn, const char *query);
static void check_sql_result(PGresult *res, PGconn *conn, const char *query,
ExecStatusType expected);
--- 192,198
static void dumpEncoding(Archive *AH);
static void dumpStdStrings(Archive *AH);
static const char *getAttrName(int attrnum, TableInfo *tblInfo);
! static const char *fmtCopyColumnList(const TableInfo *ti, bool addParens);
static void do_sql_command(PGconn *conn, const char *query);
static void check_sql_result(PGresult *res, PGconn *conn, const char *query,
ExecStatusType expected);
***
*** 231,236
--- 232,238
static struct option long_options[] = {
{data-only, no_argument, NULL, 'a'},
+ {where, no_argument, NULL, 'w'},
{blobs, no_argument, NULL, 'b'},
{clean, no_argument, NULL, 'c'},
{create, no_argument, NULL, 'C'},
***
*** 281,286
--- 283,289
strcpy(g_opaque_type, opaque);
dataOnly = schemaOnly = dumpInserts = attrNames = false;
+ whereClauseExpr = NULL;
progname = get_progname(argv[0]);
***
*** 302,308
}
}
! while ((c = getopt_long(argc, argv, abcCdDE:f:F:h:in:N:oOp:RsS:t:T:U:vWxX:Z:,
long_options, optindex)) != -1)
{
switch (c)
--- 305,311
}
}
! while ((c = getopt_long(argc, argv, abcCdDE:f:F:h:in:N:oOp:RsS:t:T:U:vw:WxX:Z:,
long_options, optindex)) != -1)
{
switch (c)
***
*** 403,408
--- 406,415
g_verbose = true;
break;
+ case 'w': /* where clause expression */
+ whereClauseExpr = strdup(optarg);
+ break;
+
case 'W':
Re: [PATCHES] Feature: give pg_dump a WHERE clause expression
Davy Durham [EMAIL PROTECTED] writes: I have added a new parameter, -w/--where=EXPR This lets you specify an expression that will be used in a WHERE clause when the data is dumped. This seems pretty poorly thought out. It can hardly work in a dump of more than one table, which means that there's not any real reason to use pg_dump at all. Just do a COPY (SELECT ...) TO somefile. regards, tom lane -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] partial header cleanup
Zdenek Kotala wrote: This replace xlog.h with xlogdefs.h in bufpage.h. All other changes are forgotten include somewhere. It reduce e.g. bloat to half in itup.h. But, There are still unresolved problems. htup should include bufpage.h, because it needs PageHeader size, but there is still unnecessary bufmgr.h include in bufpage which generates bloat. I agree with this patch -- in fact I had done the same before PGCon and then neglected it for some reason. (I think I was distracted trying to get the struct RelationData definition out of rel.h, but that did not turn out too well). I was thinking maybe we need a third buffer manager header file. One would have the current bufmgr.h, another would have the page stuff that does not know about bufmgr.h (so most of current bufpage.h), and the third one would be both plus the #define that needs both (which is currently in bufpage.h). I am not sure what kind of fallout that causes. Maybe that would help you too. We need to come up with a good name for that file however ... bufmgrpage.h seems ugly. -- Alvaro Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] partial header cleanup
Alvaro Herrera napsal(a): Zdenek Kotala wrote: This replace xlog.h with xlogdefs.h in bufpage.h. All other changes are forgotten include somewhere. It reduce e.g. bloat to half in itup.h. But, There are still unresolved problems. htup should include bufpage.h, because it needs PageHeader size, but there is still unnecessary bufmgr.h include in bufpage which generates bloat. I agree with this patch -- in fact I had done the same before PGCon and then neglected it for some reason. (I think I was distracted trying to get the struct RelationData definition out of rel.h, but that did not turn out too well). I think rel.h is atomic. There is no space to split it. I was thinking maybe we need a third buffer manager header file. One would have the current bufmgr.h, another would have the page stuff that does not know about bufmgr.h (so most of current bufpage.h), and the third one would be both plus the #define that needs both (which is currently in bufpage.h). I am not sure what kind of fallout that causes. Maybe that would help you too. We need to come up with a good name for that file however ... bufmgrpage.h seems ugly. I don't think that we need third header file. It seems to me, that only macros BufferGetPage requires both headers (Page and Buffer datatype). Other content seems to me independent. I'm thinking about renaming bufpage.h to page.h, but it is cosmetic change and IIRC CVS does not like file renaming. Zdenek -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Feature: give pg_dump a WHERE clause expression
On Sun, 2008-06-01 at 10:43 -0400, Tom Lane wrote: Davy Durham [EMAIL PROTECTED] writes: I have added a new parameter, -w/--where=EXPR This lets you specify an expression that will be used in a WHERE clause when the data is dumped. This seems pretty poorly thought out. It can hardly work in a dump of more than one table, which means that there's not any real reason to use pg_dump at all. Just do a COPY (SELECT ...) TO somefile. regards, tom lane Well, my primary reason for writing the patch was to have a standard SQL file using INSERT statements in order to load the some of a table's data into a database other than postgresql which does not support the COPY statement. I'll admit that the single where clause would often not be applicable across all tables in a database, but when pg_dump is told specific tables to dump (a nice existing feature of pg_dump for doing something specialized other than a simple entire database backup), then it can be useful. My particular case is that I have several tables that are simple event logs. Each table has a timestamp column. I'm periodically bringing these tables into sync on another database and I only want to pull rows newer than since the last sync.. So, a where-clause of.. 'ts $last_sync' ..works for me. However, I'm sure there are other uses too.. == Thinking Further == Beyond serving my own needs, I'm trying to generically extend the general idea that pg_dump already supports: 1) pg_dump can be made to dump an entire database 2) pg_dump can be made to dump only requested tables 3) [my addition] pg_dump can be made to dump only requested rows from requested tables However, it's no SO generic in that the where clause applies to all tables. So, if this patch is not acceptable as-is, what would you feel about this: I could enhance the -t/--table=NAME option to accept more than a simple NAME. Rather it could accept something in the form: --table=table_name:where-clause expression For example, pg_dump --table='foo:col1 10 AND f2 14' Currently, the user can specify -t/--table multiple times to have more than one table dumped. Or the user can use a pattern to a single -t option to request multiple tabes. This way, a user could specify a WHERE clause per table he has requested to dump. Granted, the WHERE clause may then apply to multiple tables if a pattern was used, but that may very well be desirable to the user. Unless you disagree, this is a more generic solution (than what my patch contains) to allowing the user of pg_dump to further refine what they wish to dump. Thoughts? Thanks for the feedback -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] [BUGS] BUG #4203: perform dblink() in begin/exception returns wrong SQLSTATE code
Joe Conway [EMAIL PROTECTED] writes: Here is my proposed patch -- as suggested for cvs tip only. A few comments: Don't use errstart/errfinish directly. A reasonable way to deal with the type of situation you have here is ereport(ERROR, (errcode(...), errmsg(...), det_msg ? errdetail(%s, det_msg) : 0, hint_msg ? errhint(%s, hint_msg) : 0, ...)); You can't expect the result of PQresultErrorField to still be valid after you've PQclear'd the PGresult. I think you'll need to pstrdup the non-null results first. Or maybe use a PG_TRY block to free the PGresult on the way out after the error escape ... but pstrdup is probably cleaner. This code doesn't cope with the possibility that no SQLSTATE is available (a distinct possibility for libpq-detected errors). You'll need to use some generic error code in that case. I'm tempted to suggest ERRCODE_CONNECTION_FAILURE, on the assumption that if it's libpq-detected then it's a connection problem. It would probably be useful to show the name of the dblink connection in the context. I'm thinking that you are getting well past what is reasonable to put in a macro. Time to use an out-of-line function. Don't use unable to... --- this is against the message style guide. could not is approved style. Also note the expectation that context entries be complete sentences. I haven't been around enough lately to be sure I understand the process these days. Should I be posting this to the wiki and waiting for the next commit fest, or just apply myself in a day or two assuming no objections? No, you can apply it yourself when you feel it's ready. The wiki queue is just to keep track of stuff that is submitted by non-committers or that a committer wants extra review of. regards, tom lane -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Feature: give pg_dump a WHERE clause expression
Davy Durham [EMAIL PROTECTED] writes: So, if this patch is not acceptable as-is, what would you feel about this: I could enhance the -t/--table=NAME option to accept more than a simple NAME. Rather it could accept something in the form: --table=table_name:where-clause expression Well, that would at least address the complaint that it doesn't scale to multiple tables, but the whole thing still seems like a frammish that will never see enough use to justify maintaining it. (BTW, what will you do with a table whose name contains a colon?) regards, tom lane -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Feature: give pg_dump a WHERE clause expression
Tom Lane wrote: Davy Durham [EMAIL PROTECTED] writes: So, if this patch is not acceptable as-is, what would you feel about this: I could enhance the -t/--table=NAME option to accept more than a simple NAME. Rather it could accept something in the form: --table=table_name:where-clause expression Well, that would at least address the complaint that it doesn't scale to multiple tables, but the whole thing still seems like a frammish that will never see enough use to justify maintaining it. (BTW, what will you do with a table whose name contains a colon?) ISTM this would be better off waiting until we turn large parts of pg_dump into a library, as has been often discussed, at which point it should be relatively simple to write a custom client to do what the OP wants. I agree that it does not at all belong in pg_dump. cheers andrew -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Feature: give pg_dump a WHERE clause expression
On Sun, Jun 01, 2008 at 04:13:34PM -0400, Andrew Dunstan wrote: Tom Lane wrote: Davy Durham [EMAIL PROTECTED] writes: So, if this patch is not acceptable as-is, what would you feel about this: I could enhance the -t/--table=NAME option to accept more than a simple NAME. Rather it could accept something in the form: --table=table_name:where-clause expression Well, that would at least address the complaint that it doesn't scale to multiple tables, but the whole thing still seems like a frammish that will never see enough use to justify maintaining it. (BTW, what will you do with a table whose name contains a colon?) ISTM this would be better off waiting until we turn large parts of pg_dump into a library, as has been often discussed, at which point it should be relatively simple to write a custom client to do what the OP wants. I agree that it does not at all belong in pg_dump. I can't imagine many of my clients ever writing another C program or even being willing to pay me to do so. While modularizing pg_dump is a fine idea, I don't think it addresses the same set of use cases and users as this proposal. -dg -- David Gould [EMAIL PROTECTED] 510 536 1443510 282 0869 If simplicity worked, the world would be overrun with insects. -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Feature: give pg_dump a WHERE clause expression
On Sun, 2008-06-01 at 15:47 -0400, Tom Lane wrote: Davy Durham [EMAIL PROTECTED] writes: So, if this patch is not acceptable as-is, what would you feel about this: I could enhance the -t/--table=NAME option to accept more than a simple NAME. Rather it could accept something in the form: --table=table_name:where-clause expression Well, that would at least address the complaint that it doesn't scale to multiple tables, but the whole thing still seems like a frammish that will never see enough use to justify maintaining it. The code is not all that much to maintain as it is, and making it an addition to an existing parameter instead of a new one may not increase the code size by much more at all. BTW- I looked it up, and mysqldump supports such an option as mine, but it too is global for all tables and cannot be specified per table AFAICT. (BTW, what will you do with a table whose name contains a colon?) I thought about that, but didn't know if tables could contain a colon or not, but I see that this is possible by enclosing the table name in double-quotes. I suppose they could escape the colon as I believe they may have to do if a table contains '*', or '?' ?? Is there another character that is more appropriate? Another option I just thought about was to leave the -w/--where flag in place, but it applies to all subsequent -t/--table flags.. So you could do something like: pg_dump -w 'expr1' -t tab1 -t tab2 -w 'expr2' -t tab3 So that the expr1 filters tab1 and tab2, and expr2 filters tab3.. This should be a simple modification to the existing patch to make the where clause tracked per table rather than merely a global variable. However there the becomes an importance on the order that args are given to pg_dump which you may object to. But then again, if order of the tables in the dump file matters, then the -t/--tables flags already indicate what order the tables will be dumped. (By pointing this out, I mean there may already be an importance of argument order in some circumstances) This also solves the ':' syntax problem you mentioned above. regards, tom lane -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Feature: give pg_dump a WHERE clause expression
daveg wrote: ISTM this would be better off waiting until we turn large parts of pg_dump into a library, as has been often discussed, at which point it should be relatively simple to write a custom client to do what the OP wants. I agree that it does not at all belong in pg_dump. I can't imagine many of my clients ever writing another C program or even being willing to pay me to do so. While modularizing pg_dump is a fine idea, I don't think it addresses the same set of use cases and users as this proposal. It's not clear to me that your use case is very compelling. Does your foreign database not support import via CSV or XML? Postgres can now produce both of these for any arbitrary query. cheers andrew -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Feature: give pg_dump a WHERE clause expression
On Sun, Jun 01, 2008 at 04:40:15PM -0400, Andrew Dunstan wrote: daveg wrote: I can't imagine many of my clients ever writing another C program or even being willing to pay me to do so. While modularizing pg_dump is a fine idea, I don't think it addresses the same set of use cases and users as this proposal. It's not clear to me that your use case is very compelling. Does your foreign database not support import via CSV or XML? Postgres can now produce both of these for any arbitrary query. The foreign database in question is postgresql. The feature that the proposed patch enables is to create pg_dump custom format archives for multiple tables with a predicate. No amount of csv or xml will do that. Contrived example: pg_dump -Fc --table=*._stats:where ts = now()::date -f todays_stats.pgd If I have not been successful in explaining this clearly, please reply privately to avoid cluttering the list. If you simply disagree about the usefulness of the feature, I'm fine with that. -dg -- David Gould [EMAIL PROTECTED] 510 536 1443510 282 0869 If simplicity worked, the world would be overrun with insects. -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
[PATCHES] Re: [BUGS] BUG #4203: perform dblink() in begin/exception returns wrong SQLSTATE code
Tom Lane wrote:
Joe Conway [EMAIL PROTECTED] writes:
Here is my proposed patch -- as suggested for cvs tip only.
A few comments:
[...great comments as usual...]
Thanks for the excellent feedback! I think the attached addresses it all.
I haven't been around enough lately to be sure I understand the process
these days. Should I be posting this to the wiki and waiting for the
next commit fest, or just apply myself in a day or two assuming no
objections?
No, you can apply it yourself when you feel it's ready. The wiki queue
is just to keep track of stuff that is submitted by non-committers or
that a committer wants extra review of.
Great. Assuming no other objections I'll commit in a day or three.
Joe
Index: dblink.c
===
RCS file: /opt/src/cvs/pgsql/contrib/dblink/dblink.c,v
retrieving revision 1.73
diff -c -r1.73 dblink.c
*** dblink.c 4 Apr 2008 17:02:56 - 1.73
--- dblink.c 1 Jun 2008 21:58:46 -
***
*** 94,99
--- 94,100
static Oid get_relid_from_relname(text *relname_text);
static char *generate_relation_name(Oid relid);
static void dblink_security_check(PGconn *conn, remoteConn *rconn);
+ static void dblink_res_error(const char *conname, PGresult *res, const char *dblink_context_msg, bool fail);
/* Global */
static remoteConn *pconn = NULL;
***
*** 125,158
} \
} while (0)
! #define DBLINK_RES_INTERNALERROR(p2) \
! do { \
! msg = pstrdup(PQerrorMessage(conn)); \
! if (res) \
! PQclear(res); \
! elog(ERROR, %s: %s, p2, msg); \
! } while (0)
!
! #define DBLINK_RES_ERROR(p2) \
do { \
! msg = pstrdup(PQerrorMessage(conn)); \
! if (res) \
! PQclear(res); \
! ereport(ERROR, \
! (errcode(ERRCODE_SYNTAX_ERROR), \
! errmsg(%s, p2), \
! errdetail(%s, msg))); \
} while (0)
! #define DBLINK_RES_ERROR_AS_NOTICE(p2) \
do { \
msg = pstrdup(PQerrorMessage(conn)); \
if (res) \
PQclear(res); \
! ereport(NOTICE, \
! (errcode(ERRCODE_SYNTAX_ERROR), \
! errmsg(%s, p2), \
! errdetail(%s, msg))); \
} while (0)
#define DBLINK_CONN_NOT_AVAIL \
--- 126,145
} \
} while (0)
! #define xpstrdup(var_c, var_) \
do { \
! if (var_ != NULL) \
! var_c = pstrdup(var_); \
! else \
! var_c = NULL; \
} while (0)
! #define DBLINK_RES_INTERNALERROR(p2) \
do { \
msg = pstrdup(PQerrorMessage(conn)); \
if (res) \
PQclear(res); \
! elog(ERROR, %s: %s, p2, msg); \
} while (0)
#define DBLINK_CONN_NOT_AVAIL \
***
*** 396,408
res = PQexec(conn, buf.data);
if (!res || PQresultStatus(res) != PGRES_COMMAND_OK)
{
! if (fail)
! DBLINK_RES_ERROR(sql error);
! else
! {
! DBLINK_RES_ERROR_AS_NOTICE(sql error);
PG_RETURN_TEXT_P(cstring_to_text(ERROR));
- }
}
PQclear(res);
--- 383,391
res = PQexec(conn, buf.data);
if (!res || PQresultStatus(res) != PGRES_COMMAND_OK)
{
! dblink_res_error(conname, res, could not open cursor, fail);
! if (!fail)
PG_RETURN_TEXT_P(cstring_to_text(ERROR));
}
PQclear(res);
***
*** 470,482
res = PQexec(conn, buf.data);
if (!res || PQresultStatus(res) != PGRES_COMMAND_OK)
{
! if (fail)
! DBLINK_RES_ERROR(sql error);
! else
! {
! DBLINK_RES_ERROR_AS_NOTICE(sql error);
PG_RETURN_TEXT_P(cstring_to_text(ERROR));
- }
}
PQclear(res);
--- 453,461
res = PQexec(conn, buf.data);
if (!res || PQresultStatus(res) != PGRES_COMMAND_OK)
{
! dblink_res_error(conname, res, could not close cursor, fail);
! if (!fail)
PG_RETURN_TEXT_P(cstring_to_text(ERROR));
}
PQclear(res);
***
*** 513,519
int call_cntr;
int max_calls;
AttInMetadata *attinmeta;
- char *msg;
PGresult *res = NULL;
MemoryContext oldcontext;
char *conname = NULL;
--- 492,497
***
*** 590,602
(PQresultStatus(res) != PGRES_COMMAND_OK
PQresultStatus(res) != PGRES_TUPLES_OK))
{
! if (fail)
! DBLINK_RES_ERROR(sql error);
! else
! {
! DBLINK_RES_ERROR_AS_NOTICE(sql error);
SRF_RETURN_DONE(funcctx);
- }
}
else if (PQresultStatus(res) == PGRES_COMMAND_OK)
{
--- 568,576
(PQresultStatus(res) != PGRES_COMMAND_OK
PQresultStatus(res) != PGRES_TUPLES_OK))
{
! dblink_res_error(conname, res, could not fetch from cursor, fail);
! if (!fail)
SRF_RETURN_DONE(funcctx);
}
else if (PQresultStatus(res) == PGRES_COMMAND_OK)
{
***
*** 846,856
(PQresultStatus(res) != PGRES_COMMAND_OK
PQresultStatus(res) != PGRES_TUPLES_OK))
{
! if (fail)
! DBLINK_RES_ERROR(sql error);
! else
{
- DBLINK_RES_ERROR_AS_NOTICE(sql error);
if (freeconn)
PQfinish(conn);
SRF_RETURN_DONE(funcctx);
--- 820,828
Re: [PATCHES] [BUGS] BUG #4203: perform dblink() in begin/exception returns wrong SQLSTATE code
Joe Conway [EMAIL PROTECTED] writes: [ improved patch ] Still a couple quibbles: + ereport(level, + (errcode(sqlstate), + errmsg(message_primary), This *must* be errmsg(%s, message_primary), else you have big problems with % in the text. Also, I think it's at least theoretically possible for message_primary to be null, in which case you'd better substitute unknown error or some such. You could avoid the ugly cast-away-const by making dblink_context_conname be const char *, no? Since dblink_res_error isn't going to return if fail = true, seems like you could skip the if (!fail) tests occurring after calls to it. regards, tom lane -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
[PATCHES] Re: [BUGS] BUG #4203: perform dblink() in begin/exception returns wrong SQLSTATE code
Tom Lane wrote:
Joe Conway [EMAIL PROTECTED] writes:
[ improved patch ]
Still a couple quibbles:
[ more good feedback ]
All valid complaints, and noticeably improved/simplified code as a
result. Third patch attached.
Joe
Index: dblink.c
===
RCS file: /opt/src/cvs/pgsql/contrib/dblink/dblink.c,v
retrieving revision 1.73
diff -c -r1.73 dblink.c
*** dblink.c 4 Apr 2008 17:02:56 - 1.73
--- dblink.c 1 Jun 2008 23:52:39 -
***
*** 94,99
--- 94,100
static Oid get_relid_from_relname(text *relname_text);
static char *generate_relation_name(Oid relid);
static void dblink_security_check(PGconn *conn, remoteConn *rconn);
+ static void dblink_res_error(const char *conname, PGresult *res, const char *dblink_context_msg, bool fail);
/* Global */
static remoteConn *pconn = NULL;
***
*** 125,158
} \
} while (0)
! #define DBLINK_RES_INTERNALERROR(p2) \
! do { \
! msg = pstrdup(PQerrorMessage(conn)); \
! if (res) \
! PQclear(res); \
! elog(ERROR, %s: %s, p2, msg); \
! } while (0)
!
! #define DBLINK_RES_ERROR(p2) \
do { \
! msg = pstrdup(PQerrorMessage(conn)); \
! if (res) \
! PQclear(res); \
! ereport(ERROR, \
! (errcode(ERRCODE_SYNTAX_ERROR), \
! errmsg(%s, p2), \
! errdetail(%s, msg))); \
} while (0)
! #define DBLINK_RES_ERROR_AS_NOTICE(p2) \
do { \
msg = pstrdup(PQerrorMessage(conn)); \
if (res) \
PQclear(res); \
! ereport(NOTICE, \
! (errcode(ERRCODE_SYNTAX_ERROR), \
! errmsg(%s, p2), \
! errdetail(%s, msg))); \
} while (0)
#define DBLINK_CONN_NOT_AVAIL \
--- 126,145
} \
} while (0)
! #define xpstrdup(var_c, var_) \
do { \
! if (var_ != NULL) \
! var_c = pstrdup(var_); \
! else \
! var_c = NULL; \
} while (0)
! #define DBLINK_RES_INTERNALERROR(p2) \
do { \
msg = pstrdup(PQerrorMessage(conn)); \
if (res) \
PQclear(res); \
! elog(ERROR, %s: %s, p2, msg); \
} while (0)
#define DBLINK_CONN_NOT_AVAIL \
***
*** 396,408
res = PQexec(conn, buf.data);
if (!res || PQresultStatus(res) != PGRES_COMMAND_OK)
{
! if (fail)
! DBLINK_RES_ERROR(sql error);
! else
! {
! DBLINK_RES_ERROR_AS_NOTICE(sql error);
! PG_RETURN_TEXT_P(cstring_to_text(ERROR));
! }
}
PQclear(res);
--- 383,390
res = PQexec(conn, buf.data);
if (!res || PQresultStatus(res) != PGRES_COMMAND_OK)
{
! dblink_res_error(conname, res, could not open cursor, fail);
! PG_RETURN_TEXT_P(cstring_to_text(ERROR));
}
PQclear(res);
***
*** 470,482
res = PQexec(conn, buf.data);
if (!res || PQresultStatus(res) != PGRES_COMMAND_OK)
{
! if (fail)
! DBLINK_RES_ERROR(sql error);
! else
! {
! DBLINK_RES_ERROR_AS_NOTICE(sql error);
! PG_RETURN_TEXT_P(cstring_to_text(ERROR));
! }
}
PQclear(res);
--- 452,459
res = PQexec(conn, buf.data);
if (!res || PQresultStatus(res) != PGRES_COMMAND_OK)
{
! dblink_res_error(conname, res, could not close cursor, fail);
! PG_RETURN_TEXT_P(cstring_to_text(ERROR));
}
PQclear(res);
***
*** 513,519
int call_cntr;
int max_calls;
AttInMetadata *attinmeta;
- char *msg;
PGresult *res = NULL;
MemoryContext oldcontext;
char *conname = NULL;
--- 490,495
***
*** 590,602
(PQresultStatus(res) != PGRES_COMMAND_OK
PQresultStatus(res) != PGRES_TUPLES_OK))
{
! if (fail)
! DBLINK_RES_ERROR(sql error);
! else
! {
! DBLINK_RES_ERROR_AS_NOTICE(sql error);
! SRF_RETURN_DONE(funcctx);
! }
}
else if (PQresultStatus(res) == PGRES_COMMAND_OK)
{
--- 566,573
(PQresultStatus(res) != PGRES_COMMAND_OK
PQresultStatus(res) != PGRES_TUPLES_OK))
{
! dblink_res_error(conname, res, could not fetch from cursor, fail);
! SRF_RETURN_DONE(funcctx);
}
else if (PQresultStatus(res) == PGRES_COMMAND_OK)
{
***
*** 846,860
(PQresultStatus(res) != PGRES_COMMAND_OK
PQresultStatus(res) != PGRES_TUPLES_OK))
{
! if (fail)
! DBLINK_RES_ERROR(sql error);
! else
! {
! DBLINK_RES_ERROR_AS_NOTICE(sql error);
! if (freeconn)
! PQfinish(conn);
! SRF_RETURN_DONE(funcctx);
! }
}
if (PQresultStatus(res) == PGRES_COMMAND_OK)
--- 817,826
(PQresultStatus(res) != PGRES_COMMAND_OK
PQresultStatus(res) != PGRES_TUPLES_OK))
{
! dblink_res_error(conname, res, could not execute query, fail);
! if (freeconn)
! PQfinish(conn);
! SRF_RETURN_DONE(funcctx);
}
if (PQresultStatus(res) == PGRES_COMMAND_OK)
***
*** 1180,1189
(PQresultStatus(res) != PGRES_COMMAND_OK
PQresultStatus(res) != PGRES_TUPLES_OK))
{
! if (fail)
! DBLINK_RES_ERROR(sql
Re: [PATCHES] Feature: give pg_dump a WHERE clause expression
* daveg ([EMAIL PROTECTED]) wrote: The feature that the proposed patch enables is to create pg_dump custom format archives for multiple tables with a predicate. No amount of csv or xml will do that. Contrived example: Uh, pg_dump's custom format really isn't particularly special, to be honest. Is there some reason you're interested in using it over, as was suggested, COPY/CSV/etc format? You could, of course, gzip the output of COPY (as pg_dump does) if you're concerned about space.. Thanks, Stephen signature.asc Description: Digital signature
Re: [PATCHES] Feature: give pg_dump a WHERE clause expression
On Sun, 2008-06-01 at 22:02 -0400, Stephen Frost wrote: * Davy Durham ([EMAIL PROTECTED]) wrote: To reiterate, it is not possible to use the COPY command to create standard SQL INSERT statements that can be conveniently loaded by another db. No? Erm, thankfully, PostgreSQL (what you're loading the data into?) can take more than just SQL INSERT statements. No, the database I'm loading into is not PostgreSQL, but that's okay. I think that was someone else. Of course, on the other hand, you *could* use COPY to create SQL INSERT statements through an appropriately crafted query. PG makes that reasonably straight-forward, actually. Stephen -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Feature: give pg_dump a WHERE clause expression
Davy Durham [EMAIL PROTECTED] writes: To reiterate, it is not possible to use the COPY command to create standard SQL INSERT statements that can be conveniently loaded by another db. No? Fair point, but the question here is about how useful this incremental feature really is compared to its incremental maintenance cost. It is *possible* to achieve what you want without any pg_dump changes: create a fresh table via CREATE TABLE tmp_table AS SELECT * FROM my_table WHERE ... and then pg_dump that. So the issue is how often does this problem come up and is it worth maintaining more code to make it a bit easier to deal with? My thought is probably not, and that seems to be the majority opinion so far. regards, tom lane -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Feature: give pg_dump a WHERE clause expression
On Sun, 2008-06-01 at 22:19 -0400, Stephen Frost wrote: or so. I suppose it might be interesting to consider an 'insert-format' output for COPY, which pg_dump could possibly be refactored to use when requested. It'd be nice if it was easier to have COPY support more formats but right now it's kind of a pain. That sounds also reasonable to me. It seems like a worthy format that COPY could output. And sure.. pg_dump could then be simplified a bit by using this. Beyond that, pg_dump would probably most easily let the user specify the very keyword (or full COPY statement parameters) that COPY would use as output format rather than implementing flags for everything COPY already supports. Perhaps support the existing -d/-D flags for backwards compatibility. This makes pg_dump more flexible AND COPY more flexible for use cases like mine. -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Feature: give pg_dump a WHERE clause expression
Davy Durham [EMAIL PROTECTED] writes: On Sun, 2008-06-01 at 22:19 -0400, Stephen Frost wrote: or so. I suppose it might be interesting to consider an 'insert-format' output for COPY, which pg_dump could possibly be refactored to use when requested. And sure.. pg_dump could then be simplified a bit by using this. Not really. pg_dump has to support old server versions, so you won't get any meaningful simplification by moving functionality out of pg_dump into the backend; at least not before you've forgotten having made the change :-( regards, tom lane -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
[PATCHES] Feature: pg_dump: ability to specify WHERE clause expression for -t/--table option
Okay,
Because I'm hardheaded as well as anxious to implement my own ideas
for the challenge of it :) ...
I've extended the the -t/--table option to optionally accept a WHERE
clause expression following the table pattern.
The user can, for example, run:
pg_dump -t tab1:col115 -t tab2:col125
and it will dump only tab1 WHERE col115 and tab2 WHERE col225
Since the -t/--table option already supports accepting wildcard table
names, the WHERE clause expression applies to all tables matching a
pattern.
The only backwards incompatibility is that if a table name pattern
contains a ':', then the pattern must be quoted otherwise the ':' that
is part of the pattern will be interpreted as a separator between a
pattern and an expression. If ':' is too common a character to be found
in table patterns, then it can easily be changed (in the pg_dump.sgml
file and in one place with a #define at the top of pg_dump.c).
As before, I have implemented and tested that it works when generating
either COPY statements (the default), or INSERT statements (-d and -D).
These two modes of operation have two different sections of code that
select the data to be dumped.
The code changes should be in conformance to the existing coding style
within pg_dump.
I've made minimal changes to the pg_dump.sgml file as well to affect
the man page.
The patch should be applied from the root of the source tree with a -p1
option to the patch command. The patch is against version 8.3.1 source
code.
Thanks *again* for a great DB,
Davy
==
P.S.
This submission is my final response to the previous thread titled:
Feature: give pg_dump a WHERE clause expression
Please forgive the submitted patch that I know may never get merged
in. I merely wanted the final result posted/archived somewhere public
in case any other lurkers were interested for their own purposes. But
if you're at all curious, try it out. You might like it :)
Thanks again for all your feedback.
diff -cr postgresql-8.3.1.orig/doc/src/sgml/ref/pg_dump.sgml postgresql-8.3.1/doc/src/sgml/ref/pg_dump.sgml
*** postgresql-8.3.1.orig/doc/src/sgml/ref/pg_dump.sgml 2008-05-31 17:48:38.0 -0500
--- postgresql-8.3.1/doc/src/sgml/ref/pg_dump.sgml 2008-06-01 23:37:32.0 -0500
***
*** 451,458
/varlistentry
varlistentry
! termoption-t replaceable class=parametertable/replaceable/option/term
! termoption--table=replaceable class=parametertable/replaceable/option/term
listitem
para
Dump only tables (or views or sequences) matching replaceable
--- 451,458
/varlistentry
varlistentry
! termoption-t replaceable class=parametertable[:expression]/replaceable/option/term
! termoption--table=replaceable class=parametertable[:expression]/replaceable/option/term
listitem
para
Dump only tables (or views or sequences) matching replaceable
***
*** 468,473
--- 468,482
/para
para
+ If ':' and an expression follow the given table pattern, then it
+ becomes a commandWHERE/command clause expression on the table(s)
+ being dumped by this parameter. When the expression contains cerain
+ characters or spaces, be careful also to quote the entire parameter
+ to prevent the shell from processing special characters or passing
+ the parameter as multiple arguments to commandpg_dump/command.
+/para
+
+para
The option-n/ and option-N/ switches have no effect when
option-t/ is used, because tables selected by option-t/ will
be dumped regardless of those switches, and non-table objects will not
diff -cr postgresql-8.3.1.orig/src/bin/pg_dump/common.c postgresql-8.3.1/src/bin/pg_dump/common.c
*** postgresql-8.3.1.orig/src/bin/pg_dump/common.c 2008-05-31 17:48:25.0 -0500
--- postgresql-8.3.1/src/bin/pg_dump/common.c 2008-06-01 20:16:01.0 -0500
***
*** 915,927
* Support for simple list operations
*/
! void
simple_oid_list_append(SimpleOidList *list, Oid val)
{
SimpleOidListCell *cell;
cell = (SimpleOidListCell *) pg_malloc(sizeof(SimpleOidListCell));
cell-next = NULL;
cell-val = val;
if (list-tail)
--- 915,928
* Support for simple list operations
*/
! SimpleOidListCell *
simple_oid_list_append(SimpleOidList *list, Oid val)
{
SimpleOidListCell *cell;
cell = (SimpleOidListCell *) pg_malloc(sizeof(SimpleOidListCell));
cell-next = NULL;
+ cell-userData = NULL;
cell-val = val;
if (list-tail)
***
*** 929,937
else
list-head = cell;
list-tail = cell;
}
! void
simple_string_list_append(SimpleStringList *list, const char *val)
{
SimpleStringListCell *cell;
--- 930,939
else
list-head = cell;
list-tail = cell;
+ return cell;
}
! SimpleStringListCell *
simple_string_list_append(SimpleStringList
