Re: [HACKERS] deparsing utility commands
Hi, As a comment to the whole email below, I think the following approach is the best compromise we will find, allowing everybody to come up with their own format much as in the Logical Decoding plugins world. Of course, as in the Logical Decoding area, BDR and similar projects will implement their own representation, in BDR IIUC the DDL will get translated into a JSON based AST thing. Alvaro Herrera alvhe...@2ndquadrant.com writes: Actually here's another approach I like better: use a new pseudotype, pg_ddl_command, which internally is just a pointer to a CollectedCommand struct. We cannot give access to the pointer directly in SQL, so much like type internal or pg_node_tree the in/out functions should just error out. But the type can be returned as a column in pg_event_trigger_ddl_command. An extension can create a function that receives that type as argument, and return a different representation of the command; the third patch creates a function ddl_deparse_to_json() which does that. You can have as many extensions as you want, and your event triggers can use the column as many times as necessary. This removes the limitation of the previous approach that you could only have a single extension providing a CommandDeparse_hook function. This patch applies cleanly on top of current master. You need to install the extension with CREATE EXTENSION ddl_deparse after building it in contrib. Note: the extension is NOT to be committed to core, only the first two patches; that will give us more room to revisit the JSON representation more promptly. My intention is that the extension will be published elsewhere. +1 from me, Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] New Event Trigger: table_rewrite
Alvaro Herrera alvhe...@2ndquadrant.com writes: CLUSTER and VACUUM are not part of the supported commands anymore, so I think that we could replace that by the addition of a reference number in the cell of ALTER TABLE for the event table_rewrite and write at the bottom of the table a description of how this event behaves with ALTER TABLE. Note as well that might or might not is not really helpful for the user. That's precisely why we have an event trigger here, I think --- for some subcommands, it's not easy to determine whether a rewrite happens or not. (I think SET TYPE is the one). I don't think we want to document precisely under what condition a rewrite takes place. Yeah, the current documentation expands to the following sentence, as browsed in http://www.postgresql.org/docs/9.3/interactive/sql-altertable.html As an exception, if the USING clause does not change the column contents and the old type is either binary coercible to the new type or an unconstrained domain over the new type, a table rewrite is not needed, but any indexes on the affected columns must still be rebuilt. I don't think that “might or might not” is less helpful in the context of the Event Trigger, because the whole point is that the event is only triggered in case of a rewrite. Of course we could cross link the two paragraphs or something. 2) The examples of SQL queries provided are still in lower case in the docs, that's contrary to the rest of the docs where upper case is used for reserved keywords. Right, being consistent trumps personal preferences, changed in the attached. Yes please. nitpick Another thing in that sample code is not current_hour between 1 and 6. That reads strange to me. It should be equally correct to spell it as current_hour not between 1 and 6, which seems more natural. / True, fixed in the attached. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support diff --git a/doc/src/sgml/event-trigger.sgml b/doc/src/sgml/event-trigger.sgml index 6f71a27..0a80993 100644 --- a/doc/src/sgml/event-trigger.sgml +++ b/doc/src/sgml/event-trigger.sgml @@ -65,6 +65,16 @@ /para para +The literaltable_rewrite/ event occurs just before a table is going to +get rewritten by the commands literalALTER TABLE/literal. While other +control statements are available to rewrite a table, +like literalCLUSTER/literal and literalVACUUM/literal, +the literaltable_rewrite/ event is currently only triggered by +the literalALTER TABLE/literal command, which might or might not need +to rewrite the table. + /para + + para Event triggers (like other functions) cannot be executed in an aborted transaction. Thus, if a DDL command fails with an error, any associated literalddl_command_end/ triggers will not be executed. Conversely, @@ -120,6 +130,7 @@ entryliteralddl_command_start/literal/entry entryliteralddl_command_end/literal/entry entryliteralsql_drop/literal/entry +entryliteraltable_rewrite/literal/entry /row /thead tbody @@ -128,510 +139,595 @@ entry align=centerliteralX/literal/entry entry align=centerliteralX/literal/entry entry align=centerliteral-/literal/entry +entry align=centerliteral-/literal/entry /row row entry align=leftliteralALTER COLLATION/literal/entry entry align=centerliteralX/literal/entry entry align=centerliteralX/literal/entry entry align=centerliteral-/literal/entry +entry align=centerliteral-/literal/entry /row row entry align=leftliteralALTER CONVERSION/literal/entry entry align=centerliteralX/literal/entry entry align=centerliteralX/literal/entry entry align=centerliteral-/literal/entry +entry align=centerliteral-/literal/entry /row row entry align=leftliteralALTER DOMAIN/literal/entry entry align=centerliteralX/literal/entry entry align=centerliteralX/literal/entry entry align=centerliteral-/literal/entry +entry align=centerliteral-/literal/entry /row row entry align=leftliteralALTER EXTENSION/literal/entry entry align=centerliteralX/literal/entry entry align=centerliteralX/literal/entry entry align=centerliteral-/literal/entry +entry align=centerliteral-/literal/entry /row row entry align=leftliteralALTER FOREIGN DATA WRAPPER/literal/entry entry align=centerliteralX/literal/entry entry align=centerliteralX/literal/entry entry align=centerliteral-/literal/entry +entry align=centerliteral-/literal/entry /row row entry align=leftliteralALTER FOREIGN TABLE/literal/entry entry align=centerliteralX/literal/entry entry align=centerliteralX
Re: [HACKERS] New Event Trigger: table_rewrite
Alvaro Herrera alvhe...@2ndquadrant.com writes: Almost the whole of that function is conditions to bail out clustering the relation if things have changed since the relation list was collected. It seems wrong to invoke the event trigger in all those cases; it's going to fire spuriously. I think you should move the invocation of the event trigger at the end, just before rebuild_relation is called. Not sure where relative to the predicate lock stuff therein; probably before, so that we avoid doing that dance if the event trigger function decides to jump ship. Actually when you do a CLUSTER or a VACUUM FULL you know that the table is going to be rewritten on disk, because that's about the only purpose of the command. Given the complexity involved here, the new version of the patch (attached) has removed support for those statements. In ATRewriteTables, it seems wrong to call it after make_new_heap. If the event trigger function aborts, we end up with useless work done there; so I think it should be called before that. Also, why do you have the evt_table_rewrite_fired stuff? I think you should fire one event per table, no? Fixed in the attached version of the patch. The second ATRewriteTable call in ATRewriteTables does not actually rewrite the table; it only scans it to verify constraints. So I'm thinking you shouldn't call this event trigger there. Or, if we decide we want this, we probably also need something for the table scans in ALTER DOMAIN too. Fixed in the attached version of the patch. You still have the ANALYZE thing in docs, which now should be removed. Fixed in the attached version of the patch. -- Dimitri Fontaine06 63 07 10 78 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support diff --git a/doc/src/sgml/event-trigger.sgml b/doc/src/sgml/event-trigger.sgml index 6f71a27..78ec27b 100644 --- a/doc/src/sgml/event-trigger.sgml +++ b/doc/src/sgml/event-trigger.sgml @@ -65,6 +65,16 @@ /para para +The literaltable_rewrite/ event occurs just before a table is going to +get rewritten by the commands literalALTER TABLE/literal. While other +control statements are available to rewrite a table, +like literalCLUSTER/literal and literalVACUUM/literal, +the literaltable_rewrite/ event is currently only triggered by +the literalALTER TABLE/literal command, which might or might not need +to rewrite the table. + /para + + para Event triggers (like other functions) cannot be executed in an aborted transaction. Thus, if a DDL command fails with an error, any associated literalddl_command_end/ triggers will not be executed. Conversely, @@ -120,6 +130,7 @@ entryliteralddl_command_start/literal/entry entryliteralddl_command_end/literal/entry entryliteralsql_drop/literal/entry +entryliteraltable_rewrite/literal/entry /row /thead tbody @@ -128,510 +139,595 @@ entry align=centerliteralX/literal/entry entry align=centerliteralX/literal/entry entry align=centerliteral-/literal/entry +entry align=centerliteral-/literal/entry /row row entry align=leftliteralALTER COLLATION/literal/entry entry align=centerliteralX/literal/entry entry align=centerliteralX/literal/entry entry align=centerliteral-/literal/entry +entry align=centerliteral-/literal/entry /row row entry align=leftliteralALTER CONVERSION/literal/entry entry align=centerliteralX/literal/entry entry align=centerliteralX/literal/entry entry align=centerliteral-/literal/entry +entry align=centerliteral-/literal/entry /row row entry align=leftliteralALTER DOMAIN/literal/entry entry align=centerliteralX/literal/entry entry align=centerliteralX/literal/entry entry align=centerliteral-/literal/entry +entry align=centerliteral-/literal/entry /row row entry align=leftliteralALTER EXTENSION/literal/entry entry align=centerliteralX/literal/entry entry align=centerliteralX/literal/entry entry align=centerliteral-/literal/entry +entry align=centerliteral-/literal/entry /row row entry align=leftliteralALTER FOREIGN DATA WRAPPER/literal/entry entry align=centerliteralX/literal/entry entry align=centerliteralX/literal/entry entry align=centerliteral-/literal/entry +entry align=centerliteral-/literal/entry /row row entry align=leftliteralALTER FOREIGN TABLE/literal/entry entry align=centerliteralX/literal/entry entry align=centerliteralX/literal/entry entry align=centerliteralX/literal/entry +entry align=centerliteral-/literal/entry /row row entry align
Re: [HACKERS] New Event Trigger: table_rewrite
Hi, Michael Paquier michael.paqu...@gmail.com writes: 1) This patch is authorizing VACUUM and CLUSTER to use the event triggers ddl_command_start and ddl_command_end, but aren't those commands actually not DDLs but control commands? Reverted in the attached version 3 of the patch. 6) in_table_rewrite seems unnecessary. Removed in the attached version 3 of the patch. On Sun, Nov 16, 2014 at 5:51 AM, Simon Riggs si...@2ndquadrant.com wrote: 4) pg_event_trigger_table_rewrite_oid is able to return only one OID, which is the one of the table being rewritten, and it is limited to one OID because VACUUM, CLUSTER and ALTER TABLE can only run on one object at the same time in a single transaction. What about thinking that we may have in the future multiple objects rewritten in a single transaction, hence multiple OIDs could be fetched? Why would this API support something which the normal trigger API doesn't, just in case we support a feature that hadn't ever been proposed or discussed? Why can't such a change wait until that feature arrives? Agreed, unchanged in the attached. Robert Haas robertmh...@gmail.com writes: It seems pretty weird, also, that the event trigger will fire after we've taken AccessExclusiveLock when you cluster a particular relation, and before we've taken AccessExclusiveLock when you cluster database-wide. That's more or less an implementation artifact of the current code that we're exposing to the use for, really, no good reason. In the CLUSTER implementation we have only one call site for invoking the Event Trigger, in cluster_rel(). While it's true that in the single relation case, the relation is opened in cluster() then cluster_rel() is called, the opening is done with NoLock in cluster(): rel = heap_open(tableOid, NoLock); My understanding is that the relation locking only happens in cluster_rel() at this line: OldHeap = try_relation_open(tableOid, AccessExclusiveLock); Please help me through the cluster locking strategy here, I feel like I'm missing something obvious, as my conclusion from re-reading the code in lights of your comment is that your comment is not accurate with respect to the current state of the code. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support diff --git a/doc/src/sgml/event-trigger.sgml b/doc/src/sgml/event-trigger.sgml index 6f71a27..704a377 100644 --- a/doc/src/sgml/event-trigger.sgml +++ b/doc/src/sgml/event-trigger.sgml @@ -65,6 +65,12 @@ /para para +The literaltable_rewrite/ event occurs just before a table is going to +get rewritten by the commands literalALTER TABLE/literal, +literalCLUSTER/literal or literalVACUUM/literal. + /para + + para Event triggers (like other functions) cannot be executed in an aborted transaction. Thus, if a DDL command fails with an error, any associated literalddl_command_end/ triggers will not be executed. Conversely, @@ -120,518 +126,625 @@ entryliteralddl_command_start/literal/entry entryliteralddl_command_end/literal/entry entryliteralsql_drop/literal/entry +entryliteraltable_rewrite/literal/entry /row /thead tbody row +entry align=leftliteralANALYZE/literal/entry +entry align=centerliteralX/literal/entry +entry align=centerliteralX/literal/entry +entry align=centerliteral-/literal/entry +entry align=centerliteral-/literal/entry + /row + row entry align=leftliteralALTER AGGREGATE/literal/entry entry align=centerliteralX/literal/entry entry align=centerliteralX/literal/entry entry align=centerliteral-/literal/entry +entry align=centerliteral-/literal/entry /row row entry align=leftliteralALTER COLLATION/literal/entry entry align=centerliteralX/literal/entry entry align=centerliteralX/literal/entry entry align=centerliteral-/literal/entry +entry align=centerliteral-/literal/entry /row row entry align=leftliteralALTER CONVERSION/literal/entry entry align=centerliteralX/literal/entry entry align=centerliteralX/literal/entry entry align=centerliteral-/literal/entry +entry align=centerliteral-/literal/entry /row row entry align=leftliteralALTER DOMAIN/literal/entry entry align=centerliteralX/literal/entry entry align=centerliteralX/literal/entry entry align=centerliteral-/literal/entry +entry align=centerliteral-/literal/entry /row row entry align=leftliteralALTER EXTENSION/literal/entry entry align=centerliteralX/literal/entry entry align=centerliteralX/literal/entry entry align=centerliteral-/literal/entry +entry align=centerliteral-/literal/entry /row row
Re: [HACKERS] New Event Trigger: table_rewrite
Simon Riggs si...@2ndquadrant.com writes: It would be more useful to work on the applications of this 1. INSERT into a table * Action start time * Schema * Tablename * Number of blocks in table which would then allow you to do these things run an assessment report showing which tables would be rewritten. That should be done by the user, from within his Event Trigger code. For that to be possible, the previous patch was missing a way to expose the OID of the table being rewritten, I've now added support for that. 2. Get access to number of blocks, so you could limit rewrites only to smaller tables by putting a block limit in place. Also, I did expand the docs to fully cover your practical use case of a table_rewrite Event Trigger implementing such a table rewrite policy. 3. It might be even cooler to contemplate having pg_stat_activity publish an estimated end time. We'd probably need some kind of time_per_block parameter for each tablespace so we can estimate the time. That feels like another patch entirely. Regards, -- Dimitri Fontaine06 63 07 10 78 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support diff --git a/doc/src/sgml/event-trigger.sgml b/doc/src/sgml/event-trigger.sgml index 6f71a27..7eb3225 100644 --- a/doc/src/sgml/event-trigger.sgml +++ b/doc/src/sgml/event-trigger.sgml @@ -65,6 +65,12 @@ /para para +The literaltable_rewrite/ event occurs just before a table is going to +get rewritten by the commands literalALTER TABLE/literal, +literalCLUSTER/literal or literalVACUUM/literal. + /para + + para Event triggers (like other functions) cannot be executed in an aborted transaction. Thus, if a DDL command fails with an error, any associated literalddl_command_end/ triggers will not be executed. Conversely, @@ -120,518 +126,625 @@ entryliteralddl_command_start/literal/entry entryliteralddl_command_end/literal/entry entryliteralsql_drop/literal/entry +entryliteraltable_rewrite/literal/entry /row /thead tbody row +entry align=leftliteralANALYZE/literal/entry +entry align=centerliteralX/literal/entry +entry align=centerliteralX/literal/entry +entry align=centerliteral-/literal/entry +entry align=centerliteral-/literal/entry + /row + row entry align=leftliteralALTER AGGREGATE/literal/entry entry align=centerliteralX/literal/entry entry align=centerliteralX/literal/entry entry align=centerliteral-/literal/entry +entry align=centerliteral-/literal/entry /row row entry align=leftliteralALTER COLLATION/literal/entry entry align=centerliteralX/literal/entry entry align=centerliteralX/literal/entry entry align=centerliteral-/literal/entry +entry align=centerliteral-/literal/entry /row row entry align=leftliteralALTER CONVERSION/literal/entry entry align=centerliteralX/literal/entry entry align=centerliteralX/literal/entry entry align=centerliteral-/literal/entry +entry align=centerliteral-/literal/entry /row row entry align=leftliteralALTER DOMAIN/literal/entry entry align=centerliteralX/literal/entry entry align=centerliteralX/literal/entry entry align=centerliteral-/literal/entry +entry align=centerliteral-/literal/entry /row row entry align=leftliteralALTER EXTENSION/literal/entry entry align=centerliteralX/literal/entry entry align=centerliteralX/literal/entry entry align=centerliteral-/literal/entry +entry align=centerliteral-/literal/entry /row row entry align=leftliteralALTER FOREIGN DATA WRAPPER/literal/entry entry align=centerliteralX/literal/entry entry align=centerliteralX/literal/entry entry align=centerliteral-/literal/entry +entry align=centerliteral-/literal/entry /row row entry align=leftliteralALTER FOREIGN TABLE/literal/entry entry align=centerliteralX/literal/entry entry align=centerliteralX/literal/entry entry align=centerliteralX/literal/entry +entry align=centerliteral-/literal/entry /row row entry align=leftliteralALTER FUNCTION/literal/entry entry align=centerliteralX/literal/entry entry align=centerliteralX/literal/entry entry align=centerliteral-/literal/entry +entry align=centerliteral-/literal/entry /row row entry align=leftliteralALTER LANGUAGE/literal/entry entry align=centerliteralX/literal/entry entry align=centerliteralX/literal/entry entry align=centerliteral-/literal/entry +entry align=centerliteral-/literal/entry
Re: [HACKERS] New Event Trigger: table_rewrite
Dimitri Fontaine dimi...@2ndquadrant.fr writes: Please find attached to this email a patch to implement a new Event Trigger, fired on the the table_rewrite event. As attached, it's meant as a discussion enabler and only supports ALTER TABLE (and maybe not in all forms of it). It will need to grow support for VACUUM FULL and CLUSTER and more before getting commited. And here's already a new version of it, including support for ALTER TABLE, VACUUM and CLUSTER commands, and documentation. Still is a small patch: doc/src/sgml/event-trigger.sgml | 106 src/backend/commands/cluster.c | 14 ++- src/backend/commands/event_trigger.c| 106 +++- src/backend/commands/tablecmds.c| 53 -- src/backend/commands/vacuum.c | 3 +- src/backend/utils/cache/evtcache.c | 2 + src/include/commands/cluster.h | 4 +- src/include/commands/event_trigger.h| 1 + src/include/utils/evtcache.h| 3 +- src/test/regress/expected/event_trigger.out | 23 + src/test/regress/sql/event_trigger.sql | 24 + 11 files changed, 322 insertions(+), 17 deletions(-) -- Dimitri Fontaine06 63 07 10 78 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support diff --git a/doc/src/sgml/event-trigger.sgml b/doc/src/sgml/event-trigger.sgml index 6f71a27..08ae838 100644 --- a/doc/src/sgml/event-trigger.sgml +++ b/doc/src/sgml/event-trigger.sgml @@ -65,6 +65,12 @@ /para para +The literaltable_rewrite/ event occurs just before a table is going to +get rewritten by the commands literalALTER TABLE/literal, +literalCLUSTER/literal or literalVACUUM/literal. + /para + + para Event triggers (like other functions) cannot be executed in an aborted transaction. Thus, if a DDL command fails with an error, any associated literalddl_command_end/ triggers will not be executed. Conversely, @@ -120,6 +126,7 @@ entryliteralddl_command_start/literal/entry entryliteralddl_command_end/literal/entry entryliteralsql_drop/literal/entry +entryliteraltable_rewrite/literal/entry /row /thead tbody @@ -128,510 +135,609 @@ entry align=centerliteralX/literal/entry entry align=centerliteralX/literal/entry entry align=centerliteral-/literal/entry +entry align=centerliteral-/literal/entry /row row entry align=leftliteralALTER COLLATION/literal/entry entry align=centerliteralX/literal/entry entry align=centerliteralX/literal/entry entry align=centerliteral-/literal/entry +entry align=centerliteral-/literal/entry /row row entry align=leftliteralALTER CONVERSION/literal/entry entry align=centerliteralX/literal/entry entry align=centerliteralX/literal/entry entry align=centerliteral-/literal/entry +entry align=centerliteral-/literal/entry /row row entry align=leftliteralALTER DOMAIN/literal/entry entry align=centerliteralX/literal/entry entry align=centerliteralX/literal/entry entry align=centerliteral-/literal/entry +entry align=centerliteral-/literal/entry /row row entry align=leftliteralALTER EXTENSION/literal/entry entry align=centerliteralX/literal/entry entry align=centerliteralX/literal/entry entry align=centerliteral-/literal/entry +entry align=centerliteral-/literal/entry /row row entry align=leftliteralALTER FOREIGN DATA WRAPPER/literal/entry entry align=centerliteralX/literal/entry entry align=centerliteralX/literal/entry entry align=centerliteral-/literal/entry +entry align=centerliteral-/literal/entry /row row entry align=leftliteralALTER FOREIGN TABLE/literal/entry entry align=centerliteralX/literal/entry entry align=centerliteralX/literal/entry entry align=centerliteralX/literal/entry +entry align=centerliteral-/literal/entry /row row entry align=leftliteralALTER FUNCTION/literal/entry entry align=centerliteralX/literal/entry entry align=centerliteralX/literal/entry entry align=centerliteral-/literal/entry +entry align=centerliteral-/literal/entry /row row entry align=leftliteralALTER LANGUAGE/literal/entry entry align=centerliteralX/literal/entry entry align=centerliteralX/literal/entry entry align=centerliteral-/literal/entry +entry align=centerliteral-/literal/entry /row row entry align=leftliteralALTER OPERATOR/literal/entry entry align=centerliteralX/literal/entry entry align=centerliteralX
[HACKERS] New Event Trigger: table_rewrite
Hi fellow hackers, Please find attached to this email a patch to implement a new Event Trigger, fired on the the table_rewrite event. As attached, it's meant as a discussion enabler and only supports ALTER TABLE (and maybe not in all forms of it). It will need to grow support for VACUUM FULL and CLUSTER and more before getting commited. Also, I'd like to work on the AccessExclusiveLock Event Trigger next, but wanted this one, more simple, to get acceptance as the way to approach adding events that are not DDL centric. This time it's not about which command is running, it's about what the command is doing. src/backend/commands/event_trigger.c| 92 - src/backend/commands/tablecmds.c| 35 +++- src/backend/utils/cache/evtcache.c | 2 + src/include/commands/event_trigger.h| 1 + src/include/utils/evtcache.h| 3 +- src/test/regress/expected/event_trigger.out | 18 src/test/regress/sql/event_trigger.sql | 21 + 7 files changed, 166 insertions(+), 6 deletions(-) Regards, -- Dimitri Fontaine06 63 07 10 78 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support diff --git a/src/backend/commands/event_trigger.c b/src/backend/commands/event_trigger.c index 1b8c94b..9314da9 100644 --- a/src/backend/commands/event_trigger.c +++ b/src/backend/commands/event_trigger.c @@ -119,11 +119,14 @@ static void AlterEventTriggerOwner_internal(Relation rel, HeapTuple tup, Oid newOwnerId); static event_trigger_command_tag_check_result check_ddl_tag(const char *tag); +static event_trigger_command_tag_check_result check_table_rewrite_ddl_tag( + const char *tag); static void error_duplicate_filter_variable(const char *defname); static Datum filter_list_to_array(List *filterlist); static Oid insert_event_trigger_tuple(char *trigname, char *eventname, Oid evtOwner, Oid funcoid, List *tags); static void validate_ddl_tags(const char *filtervar, List *taglist); +static void validate_table_rewrite_tags(const char *filtervar, List *taglist); static void EventTriggerInvoke(List *fn_oid_list, EventTriggerData *trigdata); /* @@ -154,7 +157,8 @@ CreateEventTrigger(CreateEventTrigStmt *stmt) /* Validate event name. */ if (strcmp(stmt-eventname, ddl_command_start) != 0 strcmp(stmt-eventname, ddl_command_end) != 0 - strcmp(stmt-eventname, sql_drop) != 0) + strcmp(stmt-eventname, sql_drop) != 0 + strcmp(stmt-eventname, table_rewrite) != 0) ereport(ERROR, (errcode(ERRCODE_SYNTAX_ERROR), errmsg(unrecognized event name \%s\, @@ -183,6 +187,9 @@ CreateEventTrigger(CreateEventTrigStmt *stmt) strcmp(stmt-eventname, sql_drop) == 0) tags != NULL) validate_ddl_tags(tag, tags); + else if (strcmp(stmt-eventname, table_rewrite) == 0 + tags != NULL) + validate_table_rewrite_tags(tag, tags); /* * Give user a nice error message if an event trigger of the same name @@ -281,6 +288,40 @@ check_ddl_tag(const char *tag) } /* + * Validate DDL command tags. + */ +static void +validate_table_rewrite_tags(const char *filtervar, List *taglist) +{ + ListCell *lc; + + foreach(lc, taglist) + { + const char *tag = strVal(lfirst(lc)); + event_trigger_command_tag_check_result result; + + result = check_table_rewrite_ddl_tag(tag); + if (result == EVENT_TRIGGER_COMMAND_TAG_NOT_SUPPORTED) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + /* translator: %s represents an SQL statement name */ + errmsg(event triggers are not supported for %s, + tag))); + } +} + +static event_trigger_command_tag_check_result +check_table_rewrite_ddl_tag(const char *tag) +{ + if (pg_strcasecmp(tag, ALTER TABLE) == 0 || + pg_strcasecmp(tag, CLUSTER) == 0 || + pg_strcasecmp(tag, VACUUM) == 0) + return EVENT_TRIGGER_COMMAND_TAG_OK; + + return EVENT_TRIGGER_COMMAND_TAG_NOT_SUPPORTED; +} + +/* * Complain about a duplicate filter variable. */ static void @@ -838,6 +879,55 @@ EventTriggerSQLDrop(Node *parsetree) list_free(runlist); } + +/* + * Fire table_rewrite triggers. + */ +void +EventTriggerTableRewrite(Node *parsetree) +{ + List *runlist; + EventTriggerData trigdata; + + /* + * Event Triggers are completely disabled in standalone mode. There are + * (at least) two reasons for this: + * + * 1. A sufficiently broken event trigger might not only render the + * database unusable, but prevent disabling itself to fix the situation. + * In this scenario, restarting in standalone mode provides an escape + * hatch. + * + * 2. BuildEventTriggerCache relies on systable_beginscan_ordered, and + * therefore will malfunction if pg_event_trigger's indexes are damaged. + * To allow recovery from a damaged index, we need some operating mode + * wherein event triggers are disabled. (Or we could implement + * heapscan-and-sort logic for that case, but having disaster recovery + * scenarios depend on code that's
Re: [HACKERS] DDL Damage Assessment
Jim Nasby jim.na...@bluetreble.com writes: EXPLAIN ALTER TABLE I'm thinking it would be better to have something you could set at a session level, so you don't have to stick EXPLAIN in front of all your DDL. Yeah I'm coming into that camp too, and I think the Event Trigger idea gets us halfway there. Here's a detailed sketched of how it would work: 1. preparatory steps: install the Event Trigger create extension norewrite; 2. test run: psql -1 -f ddl.sql ERROR: Table Rewrite has been cancelled. 3. Well actually we need to run that thing in production BEGIN; ALTER EVENT TRIGGER norewrite DISABLE; \i ddl.sql ALTER EVENT TRIGGER norewrite ENABLE; COMMIT; Then it's also possible to have another Event Trigger that would automatically issue a LOCK table NOWAIT; command before any DDL against a table is run, in another extension: create extension ddl_lock_nowait; The same applies, if your production rollout is blocked repeatedly and you want to force it through at some point, it's possible to disable the event trigger within the DDL script/transaction. As for the dry-run idea, I don't think that's really necessary. I've never seen anyone serious that doesn't have a development environment, which is where you would simply deploy the real DDL using verbose mode and see what the underlying commands actually do. The major drawback of the Event Trigger idea is that the transaction is cancelled as soon as a Rewrite Event is fired when you have installed the protective trigger. It means that you won't see the next problem after the first one, so it's not a dry-run. But considering what you're saying here, it might well be enough. Regards, -- Dimitri Fontaine06 63 07 10 78 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] DDL Damage Assessment
Hi fellow hackers, I would like to work on a new feature allowing our users to assess the amount of trouble they will run into when running a DDL script on their production setups, *before* actually getting their services down. The main practical example I can offer here is the ALTER TABLE command. Recent releases are including very nice optimisations to it, so much so that it's becoming increasingly hard to answer some very basic questions: - what kind of locks will be taken? (exclusive, shared) - on what objects? (foreign keys, indexes, sequences, etc) - will the table have to be rewritten? the indexes? Of course the docs are answering parts of those, but in particular the table rewriting rules are complex enough that “accidental DBAs” will fail to predict if the target data type is binary coercible to the current one. Questions: 1. Do you agree that a systematic way to report what a DDL command (or script, or transaction) is going to do on your production database is a feature we should provide to our growing user base? 2. What do you think such a feature should look like? 3. Does it make sense to support the whole set of DDL commands from the get go (or ever) when most of them are only taking locks in their own pg_catalog entry anyway? Provided that we are able to converge towards a common enough answer to those questions, I propose to hack my way around and send patches to have it (the common answer) available in the next PostgreSQL release. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] DDL Damage Assessment
Alvaro Herrera alvhe...@2ndquadrant.com writes: - will the table have to be rewritten? the indexes? Please give my DDL deparsing patch a look. There is a portion there about deparsing ALTER TABLE specifically; what it does is save a list of subcommands, and for each of them we either report the OID of the object affected (for example in ADD CONSTRAINT), or a column number (for ALTER COLUMN RENAME, say). It sounds like you would like to have some extra details returned: for instance the does the whole of it require a table rewrite bit. It sounds like it can be trivially returned in the JSON Some years ago when working on the Event Trigger framework we did mention providing some interesting events, such as a TableRewrite Event. In between what you're saying here and what Harold and Peter Geoghegan are mentionning (basically that dealing with table rewrites is 90% of the need for them), it could be that the best way to have at it would be to add that Event in the Event Trigger mechanism. We could also add an AccessExclusiveLock Event that would fire just before actually taking the lock, allowing people to RAISE EXCEPTION in that case, or to maybe just do the LOCK … NOWAIT themselves in the trigger. For the locking parts, best would be to do the LOCK … NOWAIT dance for all the tables touched by the DDL migration script. The Event Trigger approach will not solve that, unfortunately. Regards, -- Dimitri Fontaine06 63 07 10 78 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] extension_control_path
Peter Eisentraut pete...@gmx.net writes: Aside from those details, it seems clear that any reasonably complete move-extensions-elsewhere feature will need some kind of build system support. I have various ideas on that and would gladly contribute some of them, but it's not going to happen within two weeks. +1 Note that I am currently working on such a build system, so feel free to send me off-list emails about your thoughs, I'm interested and could integrate them into what I'm building. At this point I suggest that we work toward the minimum viable product: the extension_control_path feature as originally proposed (plus the crash fixes), and let the field work out best practices. As you describe, you can work around all the other issues by patching various text files, but you currently cannot move the extension control file in any way, and that's a real deficiency. (I once experimented with bind mounts to work around that -- a real mess ;-) ) Please find attached the v2 version of the patch, including fixes for the crash and documentation aspects you've listed before. Regards, -- Dimitri Fontaine06 63 07 10 78 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support *** a/doc/src/sgml/config.sgml --- b/doc/src/sgml/config.sgml *** *** 6008,6013 dynamic_library_path = 'C:\tools\postgresql;H:\my_project\lib;$libdir' --- 6008,6068 /listitem /varlistentry + varlistentry id=guc-extension-control-path xreflabel=extension_control_path + termvarnameextension_control_path/varname (typestring/type)/term + indexterm +primaryvarnameextension_control_path/ configuration parameter/primary + /indexterm + indextermprimaryextension packaging// + listitem +para + The command commandCREATE EXTENSION/ searches for the extension + control file in order to install it. The value + for varnameextension_control_path/varname is used to search for + the literalname.control/literal files. +/para + +para + Note that unless using the literaldirectory/literal control file + parameter, the extension scripts and auxilliary files are searched in + the varnameextension_control_path/varname too. +/para + +para + The value for varnameextension_control_path/varname must be a list + of absolute directory paths separated by colons (or semi-colons on + Windows). If a list element starts with the special + string literal$extdir/literal, the + compiled-in productnamePostgreSQL/productname package extension + directory is substituted for literal$extdir/literal; this is where + the extensions provided by the standard + productnamePostgreSQL/productname distribution are installed. + (Use literalpg_config --extdir/literal to find out the name of + this directory.) For example: + programlisting + extension_control_path = '/usr/local/postgresql/extension:/home/my_project:$extdir' + /programlisting + or, in a Windows environment: + programlisting + extension_control_path = 'C:\tools\postgresql;H:\my_project\lib;$extdir' + /programlisting +/para + +para + The default value for this parameter is literal'$extdir'/literal. +/para + +para + This parameter can be changed at run time by superusers, but a + setting done that way will only persist until the end of the + client connection, so this method should be reserved for + development purposes. The recommended way to set this parameter + is in the filenamepostgresql.conf/filename configuration + file. +/para + /listitem + /varlistentry + varlistentry id=guc-gin-fuzzy-search-limit xreflabel=gin_fuzzy_search_limit termvarnamegin_fuzzy_search_limit/varname (typeinteger/type)/term indexterm *** a/src/backend/commands/extension.c --- b/src/backend/commands/extension.c *** *** 25,30 --- 25,31 #include dirent.h #include limits.h + #include sys/stat.h #include unistd.h #include access/htup_details.h *** *** 60,71 --- 61,76 bool creating_extension = false; Oid CurrentExtensionObject = InvalidOid; + /* GUC extension_control_path */ + char *Extension_control_path; + /* * Internal data structure to hold the results of parsing a control file */ typedef struct ExtensionControlFile { char *name; /* name of the extension */ + char *filename; /* full path of the extension control file */ char *directory; /* directory for script files */ char *default_version; /* default install target version, if any */ char *module_pathname; /* string to substitute for MODULE_PATHNAME */ *** *** 342,397 is_extension_script_filename(const
Re: [HACKERS] extension_control_path
Hi, Peter Eisentraut pete...@gmx.net writes: On 2/27/14, 6:04 AM, Dimitri Fontaine wrote: directory = 'local/hstore-new' module_pathname = '$directory/hstore' I think your previously proposed patch to add extension_control_path plus my suggestion to update existing de facto best practices to not include $libdir into the module path name (thus allowing the use of dynamic_library_path) will address all desired use cases just fine. My opinion is that we have two choices: refactoring the current API or incrementally improving it. In both cases we should make it possible for the packager to control where any individual module file is loaded from, with maybe options for the sysadmin to override the packager's choice. In your proposal, the control moves away from the developer, and that's a good thing, so you get a +1 from me. Just please make sure that it's still possible to use full absolute path for the module path name so that the packager can have control too. Moreover, going that way would reuse existing facilities and concepts, remove indirections and reduce overall complexity. This new proposal, on the other hand, would go the other way, introducing new concepts, adding more indirections, and increasing overall complexity, while actually achieving less. What the $directory proposal achieves is allowing a fully relocatable extension layout, where you just have to drop a directory anywhere in the file system and it just works (*). It just work and allows to easily control which module is loaded and without having to setup either LD_LIBRARY_PATH, ld.so.conf nor our own dynamic_library_path. * providing that said directory is part of extension_control_path, or that you copy or move the .control file to sharedir/extension. That said, I don't intend to be using it myself, so I won't try and save that patch in any ways. My position is that Stephen's concern is real and his idea simple enough while effective, so worth pursuing. I see an analogy here. What we are currently doing is similar to hardcoding absolute rpaths into all libraries. Your proposal is effectively to (1) add the $ORIGIN mechanism and (2) make people use chrpath when they want to install somewhere else. My proposal is to get rid of all rpaths and just set a search path. Yes, on technical level, this is less powerful, but it's simpler and gets the job done and is harder to misuse. What happens if you have more than one 'prefix.so' file in your path? A problem with features like these is that they get rarely used but offer infinite flexibility, so they are not used consistently and you can't rely on anything. This is already the case for the module_pathname setting in the control file. It has, AFAICT, no actual use, and because of that no one uses it, and because of that, there is no guarantee that extensions use it sensibly, and because of that no one can ever make sensible use of it in the future, because there is no guarantee that extensions have it set sensibly. In fact, I would propose deprecating module_pathname. The module_pathname facility allows the packager to decide where the library module file gets installed and the extension author not to concern himself with that choice. I agree that using $libdir as the extension developer isn't the right thing to do. Having to choose the installation path as a developer, either in the SQL script or in the control file, is not the right thing. Now, the practical answer I have to that point is to have the packager rewrite the control file as part of its build system. My vote goes against deprecating module_pathname, because I didn't see in your proposal any ways to offer the control back to the packager, only to the sysadmin, and I don't want to have the sysadmin involved if we can avoid it (as you say, too much flexibility is a killer). In practical term, though, given the current situation, the build system I'm woking on already has to edit the SQL scripts and control files anyways… Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] extension_control_path
Peter Eisentraut pete...@gmx.net writes: I think we should get rid of the module_pathname business, and extensions' SQL files should just refer to the base file name and rely on the dynamic library path to find the files. What would we lose if we did that? Control over *which* mylib.so file gets loaded for a specific sql script. That's the whole namespace issue Stephen is worried about. If you're testing the new version of an extension before installing it properly, then you will have the current and the new versions of the .so, with the exact same name, at different places. Note that when using base file name only, then you could also have a clash with a dynamic library of the same name installed on the system, even if not made to be loaded by PostgreSQL. Some extensions are using way too generic names. Hint: prefix.so. Regards, -- Dimitri http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] extension_control_path
Stephen Frost sfr...@snowman.net writes: # hstore extension comment = 'data type for storing sets of (key, value) pairs' default_version = '1.3' directory = 'local/hstore-new' module_pathname = '$directory/hstore' relocatable = true Interesting idea. I'm a *little* concerned that re-useing '$directory' there might confuse people into thking that any values in the control file could be substituted in a similar way though. Would there really be much difference between that and '$ctldir' or something? Well, using $directory makes the feature auto-documented and very easy to read even without the reference documentation handy. It's also a very known way to setup things in .ini files. Now, what other parameters would you possibly use that way, other than $directory? I can see a use for $default_version, but that's about it. Would you rather add support for $default_version in the patch, for all of the parameters just in case, for a different set of control parameters, or rename the $directory macro? My vote goes for adding $default_version only. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] extension_control_path
Stephen Frost sfr...@snowman.net writes: Yeah, default_version was the other one that looked like it might be possible to include, but folks might decide to try and use 'comment' in that way too. Basically, there's a chance that they'd want to use any string in there. Actually, I think that $default_value is the only other serious enough candidate that we should support, and I think we should support it both from the directory and module_pathname parameters. Also, it seems to me that while the $directory macro should still be found only at the beginning of the module_pathname value, the $default_value should be substituted from wherever it is found. Please find attached a v1 version of the patch implementing that. doc/src/sgml/extend.sgml | 18 src/backend/commands/extension.c | 79 +--- 2 files changed, 91 insertions(+), 6 deletions(-) Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support *** a/doc/src/sgml/extend.sgml --- b/doc/src/sgml/extend.sgml *** *** 412,417 --- 412,423 default behavior is equivalent to specifying literaldirectory = 'extension'/. /para +para + The macro literal$default_value/literal is supported for this + parameter. When used literal$default_value/literal is then + substituted with the literaldefault_value/literal control + parameter value. +/para /listitem /varlistentry *** *** 462,467 --- 468,485 FUNCTION/ commands for C-language functions, so that the script files do not need to hard-wire the name of the shared library. /para +para + The macro literal$default_value/literal is supported for this + parameter. When used literal$default_value/literal is then + substituted with the literaldefault_value/literal control + parameter value. +/para +para + The macro literal$directory/literal is also supported for this + parameter, only when found at the very start of the value for this + parameter. When used literal$directory/literal is then substituted + with the literaldirectory/literal control parameter value. +/para /listitem /varlistentry *** a/src/backend/commands/extension.c --- b/src/backend/commands/extension.c *** *** 369,374 get_extension_control_filename(const char *extname) --- 369,377 return result; } + /* + * In the control file, the directory entry supports $default_version macro. + */ static char * get_extension_script_directory(ExtensionControlFile *control) { *** *** 383,393 get_extension_script_directory(ExtensionControlFile *control) return get_extension_control_directory(); if (is_absolute_path(control-directory)) ! return pstrdup(control-directory); ! get_share_path(my_exec_path, sharepath); ! result = (char *) palloc(MAXPGPATH); ! snprintf(result, MAXPGPATH, %s/%s, sharepath, control-directory); return result; } --- 386,406 return get_extension_control_directory(); if (is_absolute_path(control-directory)) ! result = pstrdup(control-directory); ! else ! { ! get_share_path(my_exec_path, sharepath); ! result = (char *) palloc(MAXPGPATH); ! snprintf(result, MAXPGPATH, %s/%s, sharepath, control-directory); ! } ! /* see about replacing the $default_version macro if present. */ ! result = text_to_cstring( ! DatumGetTextPP( ! DirectFunctionCall3(replace_text, ! CStringGetTextDatum(result), ! CStringGetTextDatum($default_version), ! CStringGetTextDatum(control-default_version; return result; } *** *** 432,437 get_extension_script_filename(ExtensionControlFile *control, --- 445,499 return result; } + /* + * Substitute for any macros appearing in the given string. + * Result is always freshly palloc'd. + * + * Supported macros are: + * - $directory + * - $default_version + * + * The $directory macro must be used at the very start of the module_pathname. + */ + static char * + substitute_module_macros(const char *module_pathname, + const char *directory, + const char *default_version) + { + Datum t_result; + const char *sep_ptr; + + if (module_pathname == NULL) + return NULL; + + /* Currently, we only recognize $directory at the start of the string */ + if (module_pathname[0] != '$') + return pstrdup(module_pathname); + + if ((sep_ptr = first_dir_separator(module_pathname)) == NULL) + sep_ptr = module_pathname + strlen(module_pathname); + + /* Accept $libdir, just return module_pathname as is then */ + if (strlen($libdir) == sep_ptr - module_pathname + strncmp(module_pathname, $libdir, strlen($libdir)) == 0) + return pstrdup(module_pathname); + + if (strlen($directory
Re: [HACKERS] extension_control_path
Stephen Frost sfr...@snowman.net writes: I'm a bit confused here- above you '+1'd packagers/sysadmins, but then here you are saying that hackers will be setting it? Also, it strikes Well I was then talking about how it works today, as in PostgreSQL 9.1, 9.2 and 9.3, and most certainly 9.4 as we're not trying to change anything on that front. me as a terrible idea to ship absolute object file names (which I assume you mean to include path, given you say 'absolute') unless you're an I agree, that's why my current design also needs cooperation on the backend side of things, to implement what you're calling here relocation of the files. Now that I read your comments, we might be able to implement something really simple and have something in core… Please see attached patch, tested and documented. doc/src/sgml/extend.sgml | 7 ++ src/backend/commands/extension.c | 39 +++- 2 files changed, 45 insertions(+), 1 deletion(-) Presumably, that's what you'd want to set both the control path and the dynamic extension path to- a directory of control files and a directory of .so's, or perhaps one combined directory of both, for the simplest setup. If you're working with a directory-per-package, then wouldn't you want to have everything for that package in that package's directory and then only have to add all those directories to one place in postgresql.conf? That's a fair-enough observation, that targets a use case where you're using the feature without the extra software. I also note that it could simplify said software a little bit. What about allowing a control file like this: # hstore extension comment = 'data type for storing sets of (key, value) pairs' default_version = '1.3' directory = 'local/hstore-new' module_pathname = '$directory/hstore' relocatable = true The current way directory is parsed, relative pathnames are allowed and will be resolved in SHAREDIR, which is where we find the extension/ main directory, where currently live extension control files. With such a feature, we would allow module_pathname to reuse the same location as where we're going to find auxilliary control files and scripts. My questions about this are mostly covered above, but I did want to get clarification- is this going to be on a per-system basis, as in, when the package is installed through your tool, it's going to go figure out where the package got installed to and rewrite the control file? Seems like a lot of work if you're going to have to add that directory to the postgresql.conf path for the control file anyway to then *also* have to hack up the control file itself. Given module_pathname = '$directory/xxx' the extension is now fully relocatable and the tool doesn't need to put in any other effort than hacking the control file *at build time*. See the attached patch that implements the idea. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support *** a/doc/src/sgml/extend.sgml --- b/doc/src/sgml/extend.sgml *** *** 462,467 --- 462,474 FUNCTION/ commands for C-language functions, so that the script files do not need to hard-wire the name of the shared library. /para +para + The macro literal$directory/literal is supported when found at the + very start of the value of this parameter. When + used, literal$directory/literal is then substituted with + the literaldirectory/literal control parameter value by + PostgreSQL. +/para /listitem /varlistentry *** a/src/backend/commands/extension.c --- b/src/backend/commands/extension.c *** *** 432,437 get_extension_script_filename(ExtensionControlFile *control, --- 432,470 return result; } + /* + * Substitute for any macros appearing in the given string. + * Result is always freshly palloc'd. + */ + static char * + substitute_directory_macro(const char *directory, const char *module_pathname) + { + const char *sep_ptr; + + AssertArg(module_pathname != NULL); + + /* Currently, we only recognize $directory at the start of the string */ + if (module_pathname[0] != '$') + return pstrdup(module_pathname); + + if ((sep_ptr = first_dir_separator(module_pathname)) == NULL) + sep_ptr = module_pathname + strlen(module_pathname); + + /* Accept $libdir, just return module_pathname as is then */ + if (strlen($libdir) == sep_ptr - module_pathname + strncmp(module_pathname, $libdir, strlen($libdir)) == 0) + return pstrdup(module_pathname); + + if (strlen($directory) != sep_ptr - module_pathname || + strncmp(module_pathname, $directory, strlen($directory)) != 0) + ereport(ERROR, + (errcode(ERRCODE_INVALID_NAME), + errmsg(invalid macro module_pathname in: %s, + module_pathname))); + + return psprintf(%s%s, directory, sep_ptr); + } + /* * Parse contents
Re: [HACKERS] extension_control_path
Hi, Peter Eisentraut pete...@gmx.net writes: I'm massively in favor of this feature. (I had started writing it about three times myself.) Thanks! The problem I see, however, is that most extensions, by recommendation, set module_pathname = '$libdir/pgfoo', and so relocating the control files will still end up pointing to a not relocated library file. It's kind of true. Is the phrasing “typically” followed by an example really a recommendation though? I though it was more a detailed explanation of the way it works. We still have several other ways to tell PostgreSQL which lib to use for each and every LANGUAGE C function: - $libdir/soname - absolute/path - MODULE_PATHNAME - any/relative/path which is to be solved in dynamic_library_path Also, editing the AS '$libdir/foo' occurences from an SQL script is a quite very easy thing to do programmatically. We would need to remove that and then ask users to keep their dynamic_library_path in sync with extension_control_path. That's error prone, of course. I don't see any pressure in changing the way things currently work after adding this new GUC in. As you say, when extension_control_path is used then some extra work *might need* to be done in order to ensure that the right library is getting loaded. I mainly see that as a distribution/distributor problem tho. In order to address this properly, we need a new directory structure that keeps library files and control files together, similar to how Python, Ruby, etc. install things, and then just one path for everything. It might be true, be it reads to me like you're omiting the directory parameter from the control file: the scripts and auxilliary control files might be found anywhere else on the file system already. Again, my view is that if you want to do things in a non-standard way then you need to tweak the control file and maybe the script files. It's a distribution problem, and I'm solving it in an extra software layer. PostgreSQL is very flexible about where to organise extension files currently, *except* for the control file. This patch is providing the same level of flexibility to this part. Of course flexibility can be seen as creating a mess, but I don't think it's this patch nor PostgreSQL core to solve that mess. Now a few technical problems. Will see about fixing those later, by friday given my current schedule, thanks. Also, the documentation states that this controls the location of the control file, but it of course controls the location of the script files also. That should be made clearer. (It becomes clearer if we just have one path for everything. ;-) ) Again, we have directory = 'whatever' in the control file to control where to find the script files. I'm not sure your of course follows. Will still edit docs. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] extension_control_path
Stephen Frost sfr...@snowman.net writes: This is true and Debian puts the control/sql files into a different directory than the .so files, of course. Still, the issue here is how we find the .so files- the user *has* to tell us where the control file is, if it isn't in the default location, and the assumption (default?) is then that the .sql files are co-located with them. It's at that point when we get to the point of trying to figure out what $libdir is Ok you're migthy confused. The rules that PostgreSQL follows to know where to load the library from are not changed *at all* by this patch. In my book, it makes the whole topic irrelevant to the review. Futhermore, the rules in question come from two different elements: - the object file name in the AS clause, available *separately* for each and every function definition, to be found in the script files: src/backend/commands/functioncmds.c:744 * For a dynamically linked C language object, the form of the clause is * * AS object file name [, link symbol name ] - the dynamic_library_path GUC that helps interpreting the object file name when it's not absolute or when it contains $libdir as its first characters. If you want to change the rules and provide a way to resolve the object file name to use on a per-extension level, fee free to propose a patch. Yeah, but it seems to be pretty rarely used and the expectation is that the .sql files resides in the same directory. I think what we're looking for here, in some ways, is for that default for .so's to work out the same- except that right now, the users seem to all default to sticking in $libdir. It used to be a script.sql.in containing AS 'MODULE_PATHNAME', which would then be replaced with $libdir by pgxs.mk (the rule is still here in the file). Nowadays we have the replacement facility in the backend, driven by the module_pathname property in the extension's control file. Contrib modules are still using the AS 'MODULE_PATHNAME' spelling with the extension control file spelling module_pathname = '$libdir/xxx'. Nothing changes with this patch other than where to find the extension control file. How to resolve the object file name on the file system is still the distribution and local admin problem. That the controlling of where to find the dynamic libs is convoluted and involves other people than just the PostgreSQL backend packager might be seen as a problem or a great flexibility, in any case I don't see what it has to do with reviewing the extension_control_path patch. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] extension_control_path
Stephen Frost sfr...@snowman.net writes: I didn't suggest anywhere that the proposed patch changed the rules at all- instead I was trying to point out that by adding this functionality and *not* changing the way that lookup is done *is going to cause confusion*. I don't see any confusion about dynamic library name resolving added from the extension_control_path, I'm sorry. Simply because I don't expect people to use the facility without a third party software designed to fill-in the gap. You're saying that the backend should fill the gap, I'm saying that it should not. Or maybe within another patch entirely. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] extension_control_path
Stephen Frost sfr...@snowman.net writes: I find this role reversal to be quite bizarre. Who do you think should have a say about where to load the dynamic librairies from? hackers, packagers, system admins, dbas or users? Who do you think is currently editing the setup that decides where to load the dynamic librairies from, which is spread into SQL scripts, extension control file, postgresql.conf and pg_config --pkglibdir? What exactly are you calling bizarre in the idea that the PostgreSQL source code is maybe not the best way where to solve that problem from? Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] extension_control_path
Stephen Frost sfr...@snowman.net writes: * Dimitri Fontaine (dimi...@2ndquadrant.fr) wrote: Who do you think should have a say about where to load the dynamic librairies from? hackers, packagers, system admins, dbas or users? My gut feeling on this is packages and sysadmins. Do you see it +1 Who do you think is currently editing the setup that decides where to load the dynamic librairies from, which is spread into SQL scripts, extension control file, postgresql.conf and pg_config --pkglibdir? I agree that packagers and sysadmins will be setting this up initially, Not quite, because of the ability to ship absolute object file names in the SQL script and the extension control files, edited by hackers. The third party tool I'm talking about will have to edit those files at packaging time in order to get the control back to where you want it. but it strikes me as a bit silly to ask the sysadmins to go modify the control file path and then also have to modify the dynamic library load path when they're setting them to the same thing. Well, the point is that if you edit the control file, then you don't have to care about the dynamic_library_path at all, because you're going to setup absolute object file names (or location). Related to this, as I've asked repeatedly on this thread- what is the plan for dealing with namespace overlaps? As in, the admin happily goes in and sets dynamic_library_path to '$libdir:/path/to/new/hstore' and then tries to CREATE EXTENSION hstore; with the -contrib packages installed? My proposal is to edit the control file module_pathname property to the right absolute location within the new hstore binary packaging. That responsibility is then given to the new third party tool, aimed at both packagers and system admins. Part of the reason that I'm pushing for a change here is to try and address that problem. I'd appreciate some feedback on it. Within the way I see things, this problem just doesn't exist, by design. I was referring to the apparent role reversal between us, with me trying to get PG to do more and you pushing to have more in an external tool. It wasn't that long ago that our positions were swapped. Well you know, I actually read my emails and learn from them. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Add CREATE support to event triggers
Hi, Alvaro Herrera alvhe...@2ndquadrant.com writes: So here's a patch implementing the ideas expressed in this thread. There are two new SQL functions: I spent some time reviewing the patch and tried to focus on a higher level review, as I saw Andres already began with the lower level stuff. The main things to keep in mind here are: - this patch enables running Event Triggers anytime a new object is created, in a way that the user code is run once the object already made it through the catalogs; - the Event Trigger code has access to the full details about every created object, so it's not tied to a command but really the fact that an object just was created in the catalogs; (it's important with serial and primary key sub-commands) - facilities are provided so that it's possible to easily build an SQL command that if executed would create the exact same object again; - the facilities around passing the created object details and building a SQL command are made in such a way that it's trivially possible to hack away the captured objects properties before producing again a new SQL command. After careful study and thinking, it appears to me that the proposed patch addresses the whole range of features we expect here, and is both flexible enough for the users and easy enough to maintain. The event being fired once the objects are available in the catalogs makes it possible for the code providing the details in the JSON format to complete the parsetree with necessary information. Current state of the patch is not ready for commit yet, independant of code details some more high-level work needs to be done: - preliminary commit It might be a good idea to separate away some pre-requisites of this patch and commit them separately: the OID publishing parts and allowing an Event Trigger to get fired after CREATE but without the extra detailed JSON formated information might be good commits already, and later add the much needed details about what did happen. - document the JSON format I vote for going with the proposed format, because it actually allows to implement both the audit and replication features we want, with the capability of hacking schema, data types, SQL representation etc; and because I couldn't think of anything better than what's proposed here ;-) - other usual documentation I don't suppose I have to expand on what I mean here… - fill-in other commands Not all commands are supported in the submitted patch. I think once we have a clear documentation on the general JSON formating and how to use it as a user, we need to include support for all CREATE commands that we have. I see nothing against extending when this work has to bo done until after the CF, as long as it's fully done before beta. After all it's only about filling in minutia at this point. - review the JSON producing code It might be possible to use more of the internal supports for JSON now that the format is freezing. - regression tests The patch will need some. The simpler solution is to add a new regression test entry and exercise all the CREATE commands in there, in a specific schema, activating an event trigger that outputs the JSON detailed information each time (the snitch() example). Best would be to have some pretty indented output of the JSON to help with reviewing diffs, and I have to wonder about JSON object inner-ordering if we're going to do that. No other ideas on this topic from me. The JSON parsing is done in event_trigger.c. This code should probably live elsewhere, but I again hesitate to put it in json.c or jsonfuncs.c, at least until some discussion about its general applicability takes place. I see that as useful enough if it can be made to work without the special fmt fields somehow, with a nice default formatting ability. In particular, being able to build some intermediate object with json_agg then call the formating/expanding function on top of that might be quite useful. That said, I don't think we have enough time to attack this problem now, I think it would be wiser to address your immediate problem separately in your patch and clean it later (next release) with sharing code and infrastructure and offering a more generally useful tool. At least we will have some feedback about the Event Trigger specific context then. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] extension_control_path
Hi, Sergey Muraviov sergey.k.murav...@gmail.com writes: Now patch applies cleanly and works. :-) Cool ;-) But I have some notes: 1. There is an odd underscore character in functions find_in_extension_control_path and list_extension_control_paths: \extension_control__path\ Fixed in the new version of the patch, attached. 2. If we have several versions of one extension in different directories (which are listed in extension_control_path parameter) then we get strange output from pg_available_extensions and pg_available_extension_versions views (Information about extension, whose path is at the beginning of the list, is duplicated). And only one version of the extension can be created. Fixed. 3. It would be fine to see an extension control path in pg_available_extensions and pg_available_extension_versions views (in separate column or within of extension name). I think the on-disk location is an implementation detail and decided in the attached version not to change those system view definitions. 4. Perhaps the CREATE EXTENSION command should be improved to allow creation of the required version of the extension. So we can use different versions of extensions in different databases. Fixed in the attached. I also fixed ALTER EXTENSION UPDATE to search for udpate scripts in the same directory where the main control file is found, but I suspect this part requires more thinking. When we ALTER EXTENSION UPDATE we might now have several places where we find extname.control files, with possibly differents default_version properties. In the attached, we select the directory containing the control file where default_version matches the already installed extension version. That matches with a model where the new version of the extension changes the default_version in an auxiliary file. We might want to instead match on the default_version in the control file to match with the new version we are asked to upgrade to. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support *** a/doc/src/sgml/config.sgml --- b/doc/src/sgml/config.sgml *** *** 5773,5778 SET XML OPTION { DOCUMENT | CONTENT }; --- 5773,5827 variablelist + varlistentry id=guc-extension-control-path xreflabel=extension_control_path + termvarnameextension_control_path/varname (typestring/type)/term + indexterm +primaryvarnameextension_control_path/ configuration parameter/primary + /indexterm + indextermprimaryextension packaging// + listitem +para + The command commandCREATE EXTENSION/ searches for the extension + control file in order to install it. The value + for varnameextension_control_path/varname is used to search for + the literalname.control/literal files. +/para + +para + The value for varnameextension_control_path/varname must be a list + of absolute directory paths separated by colons (or semi-colons on + Windows). If a list element starts with the special + string literal$extdir/literal, the + compiled-in productnamePostgreSQL/productname package extension + directory is substituted for literal$extdir/literal; this is where + the extensions provided by the standard + productnamePostgreSQL/productname distribution are installed. + (Use literalpg_config --extdir/literal to find out the name of + this directory.) For example: + programlisting + extension_control_path = '/usr/local/postgresql/extension:/home/my_project:$extdir' + /programlisting + or, in a Windows environment: + programlisting + extension_control_path = 'C:\tools\postgresql;H:\my_project\lib;$extdir' + /programlisting +/para + +para + The default value for this parameter is literal'$extdir'/literal. +/para + +para + This parameter can be changed at run time by superusers, but a + setting done that way will only persist until the end of the + client connection, so this method should be reserved for + development purposes. The recommended way to set this parameter + is in the filenamepostgresql.conf/filename configuration + file. +/para + /listitem + /varlistentry + varlistentry id=guc-dynamic-library-path xreflabel=dynamic_library_path termvarnamedynamic_library_path/varname (typestring/type)/term indexterm *** a/src/backend/commands/extension.c --- b/src/backend/commands/extension.c *** *** 25,30 --- 25,31 #include dirent.h #include limits.h + #include sys/stat.h #include unistd.h #include access/htup_details.h *** *** 60,71 --- 61,76 bool creating_extension = false; Oid CurrentExtensionObject = InvalidOid; + /* GUC extension_control_path */ + char *Extension_control_path
Re: [HACKERS] extension_control_path
Magnus Hagander mag...@hagander.net writes: Using colon as the path separator is going to break on windows. The patch notices this and uses semicolon on Windows instead. Do we really want to go down that path - that means that everybody who writes any sorts of installation instructions including this will have to make them separate for different platforms. Shouldn't we just use semicolon on all platforms, for consistency? Well, I've been considering that what I found already in the backend to solve the same problem was a valid model to build against. Pick any reasonnable choice you want to, fix dynamic_library_path along the new lines or maybe ask me to, and then let's apply the same design to the new GUC doing about exactly the same thing? Tom Lane t...@sss.pgh.pa.us writes: Since I disagree with the goal of this patch in the first place, I'm Should we remove dynamic_library_path? If not, why do we keep it? Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] extension_control_path
Sergey Muraviov sergey.k.murav...@gmail.com writes: I can't apply the patch. Did you try using the `patch`(1) command? The PostgreSQL project policy is to not use the git format when sending patches to the mailing list, prefering the context diff format. So you need to resort to using the basic patch commands rather than the modern git tooling. See also: http://wiki.postgresql.org/wiki/Submitting_a_Patch Patches must be in a format which provides context (eg: Context Diff); 'normal' or 'plain' diff formats are not acceptable. The following email might be useful for you: http://www.postgresql.org/message-id/CAOR=d=0q0dal0bnztsddnwpgm5ejkxuykj7m+qsqbr728eo...@mail.gmail.com Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] extension_control_path
Tom Lane t...@sss.pgh.pa.us writes: Why is that a good idea? It's certainly not going to simplify DBAs' lives, more the reverse. (This dump won't reload. Uh, where did you get that extension from? Ummm...) The latest users for the feature are the Red Hat team working on Open Shift where they want to have co-existing per-user PostgreSQL clusters on a machine, each with its own set of extensions. Having extension_control_path also allows to install extension files in a place not owned by root. Lastly, as a developer, you might enjoy being able to have your own non-system-global place to install extensions, as Andres did explain on this list not too long ago. Assuming that there is some need for loading extensions from nonstandard places, would it be better to just allow a filename specification in CREATE EXTENSION? (I don't know the answer, since the use-case isn't apparent to me in the first place, but it seems worth asking.) In the extension_control_path idea, we still are adressing needs where the people managing the OS and the database are distinct sets. The GUC allows the system admins to setup PostgreSQL the way they want, then the database guy doesn't need to know anything about that at CREATE EXTENSION time. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] extension_control_path
Tom Lane t...@sss.pgh.pa.us writes: Dimitri Fontaine dimi...@2ndquadrant.fr writes: Tom Lane t...@sss.pgh.pa.us writes: Why is that a good idea? It's certainly not going to simplify DBAs' lives, more the reverse. (This dump won't reload. Uh, where did you get that extension from? Ummm...) The latest users for the feature are the Red Hat team working on Open Shift where they want to have co-existing per-user PostgreSQL clusters on a machine, each with its own set of extensions. Um ... own set of installed extensions doesn't need to mean own set of available extensions, any more than those clusters need to have their own Postgres executables. If the clusters *do* have their own executables, eg because they're different PG versions, then they can certainly also have their own $SHAREDIR trees too. So this example is totally without value for your case. They have several clusters as in `initdb` running standard packaged binaries, each user having its own set of processes running with only his privileges. So when applying your idea (well, my understanding of it), they would be happy with a $SHAREDIR per initdb. Having extension_control_path also allows to install extension files in a place not owned by root. As far as the control files go, there's nothing saying that $SHAREDIR/extension has to be root-owned. If there are .so's involved, I do not believe the Red Hat crew is asking you to support loading .so's from non-root-owned dirs, because that'd be against their own corporate security policies. (But in any case, where we find the control and SQL files need not have anything to do with where the .so's are.) But you can have a single $SHAREDIR per set of executables, right? Please read the following email to know what they asked for and how they do operate OpenShift: http://www.postgresql.org/message-id/341087492.2585530.1376776393038.javamail.r...@redhat.com Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fixing pg_basebackup with tablespaces found in $PGDATA
Magnus Hagander mag...@hagander.net writes: Applied a fairly heavily edited version of this one. I also backpatched it to 9.1 and up. Thanks a lot! Did some reviewing and re-testing here, I like using DataDir and IS_DIR_SEP better than what I did, of course ;-) Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fixing pg_basebackup with tablespaces found in $PGDATA
Magnus Hagander mag...@hagander.net writes: We can't get away with just comparing the relative part of the pathname. Because it will fail if there is another path with exactly the same length, containing the tablespace. Actually… yeah. I think we might want to store a value in the tablespaceinfo struct indicating whether it's actually inside PGDATA (since we have the full path at that point), and then skip it based on that instead. Or store and pass the value of getcwd() perhaps. I think it's best to stuff in the tablespaceinfo struct either NIL or the relative path of the tablespace when found in $PGDATA, as done in the attached. I've attached a slightly updated patch - I changed around a bit of logic order and updated some comments during my review. And added error-checking. Thanks! I started again from your version for v3. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support *** a/src/backend/replication/basebackup.c --- b/src/backend/replication/basebackup.c *** *** 45,51 typedef struct } basebackup_options; ! static int64 sendDir(char *path, int basepathlen, bool sizeonly); static int64 sendTablespace(char *path, bool sizeonly); static bool sendFile(char *readfilename, char *tarfilename, struct stat * statbuf, bool missing_ok); --- 45,51 } basebackup_options; ! static int64 sendDir(char *path, int basepathlen, bool sizeonly, List *tablespaces); static int64 sendTablespace(char *path, bool sizeonly); static bool sendFile(char *readfilename, char *tarfilename, struct stat * statbuf, bool missing_ok); *** *** 72,77 typedef struct --- 72,78 { char *oid; char *path; + char *rpath; /* relative path within PGDATA, or nil. */ int64 size; } tablespaceinfo; *** *** 100,105 perform_base_backup(basebackup_options *opt, DIR *tblspcdir) --- 101,119 XLogRecPtr endptr; TimeLineID endtli; char *labelfile; + char cwd[MAXPGPATH]; + int rootpathlen; + + /* + * We need to compute rootpathlen to allow for skipping tablespaces + * installed within PGDATA. + */ + if (!getcwd(cwd, MAXPGPATH)) + ereport(ERROR, + (errcode_for_file_access(), + errmsg(could not determine current directory: %m))); + + rootpathlen = strlen(cwd); backup_started_in_recovery = RecoveryInProgress(); *** *** 119,124 perform_base_backup(basebackup_options *opt, DIR *tblspcdir) --- 133,139 { char fullpath[MAXPGPATH]; char linkpath[MAXPGPATH]; + char *relpath = NULL; int rllen; /* Skip special stuff */ *** *** 145,153 perform_base_backup(basebackup_options *opt, DIR *tblspcdir) --- 160,178 } linkpath[rllen] = '\0'; + /* + * Relpath is the relative path of the tablespace linkpath when + * the realname is found within PGDATA, or NULL. + */ + if (rllen rootpathlen + strncmp(linkpath, cwd, rootpathlen) == 0 + linkpath[rootpathlen] == '/') + relpath = linkpath + rootpathlen + 1; + ti = palloc(sizeof(tablespaceinfo)); ti-oid = pstrdup(de-d_name); ti-path = pstrdup(linkpath); + ti-rpath = relpath ? pstrdup(relpath) : NULL; ti-size = opt-progress ? sendTablespace(fullpath, true) : -1; tablespaces = lappend(tablespaces, ti); #else *** *** 165,171 perform_base_backup(basebackup_options *opt, DIR *tblspcdir) /* Add a node for the base directory at the end */ ti = palloc0(sizeof(tablespaceinfo)); ! ti-size = opt-progress ? sendDir(., 1, true) : -1; tablespaces = lappend(tablespaces, ti); /* Send tablespace header */ --- 190,196 /* Add a node for the base directory at the end */ ti = palloc0(sizeof(tablespaceinfo)); ! ti-size = opt-progress ? sendDir(., 1, true, tablespaces) : -1; tablespaces = lappend(tablespaces, ti); /* Send tablespace header */ *** *** 191,197 perform_base_backup(basebackup_options *opt, DIR *tblspcdir) sendFileWithContent(BACKUP_LABEL_FILE, labelfile); /* ... then the bulk of the files ... */ ! sendDir(., 1, false); /* ... and pg_control after everything else. */ if (lstat(XLOG_CONTROL_FILE, statbuf) != 0) --- 216,222 sendFileWithContent(BACKUP_LABEL_FILE, labelfile); /* ... then the bulk of the files ... */ ! sendDir(., 1, false, tablespaces); /* ... and pg_control after everything else. */ if (lstat(XLOG_CONTROL_FILE, statbuf) != 0) *** *** 778,785 sendTablespace(char *path, bool sizeonly) _tarWriteHeader(TABLESPACE_VERSION_DIRECTORY, NULL, statbuf); size = 512; /* Size of the header just added */ ! /* Send all the files in the tablespace version directory */ ! size += sendDir(pathbuf, strlen(path), sizeonly); return size; } --- 803,815
[HACKERS] Fixing pg_basebackup with tablespaces found in $PGDATA
Hi, As much as I've seen people frown upon $subject, it still happens in the wild, and Magnus seems to agree that the current failure mode of our pg_basebackup tool when confronted to the situation is a bug. So here's a fix, attached. To reproduce, mkdir -p $PGDATA/tbs/foo then CREATE TABLESPACE there, and then pg_basebackup your server. If doing so from the same server, as I did, then pick the tar format, as here: pg_basebackup -Ft -z -c fast -v -X fetch -D /tmp/backup Then use tar to see that the base backup contains the whole content of your foo tablespace, and if you did create another tablespace within $PGDATA/pg_tblspc (which is the other common way to trigger that issue) then add it to what you want to see: tar tzvf /tmp/backup/base.tar.gz pg_tblspc tbs/foo pg_tblspc/bar Note that empty directories are expected, so tar should output their entries. Those directories are where you need to be restoring the tablespace tarballs. When using pg_basebackup in plain mode, the error is that you get a copy of all your tablespaces first, then the main PGDATA is copied over and as the destination directories already do exists (and not empty) the whole backup fails there. The bug should be fixed against all revisions of pg_basebackup, though I didn't try to apply this very patch on all target branches. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support *** a/src/backend/replication/basebackup.c --- b/src/backend/replication/basebackup.c *** *** 45,51 typedef struct } basebackup_options; ! static int64 sendDir(char *path, int basepathlen, bool sizeonly); static int64 sendTablespace(char *path, bool sizeonly); static bool sendFile(char *readfilename, char *tarfilename, struct stat * statbuf, bool missing_ok); --- 45,52 } basebackup_options; ! static int64 sendDir(char *path, int basepathlen, int rootpathlen, ! bool sizeonly, List *tablespaces); static int64 sendTablespace(char *path, bool sizeonly); static bool sendFile(char *readfilename, char *tarfilename, struct stat * statbuf, bool missing_ok); *** *** 100,105 perform_base_backup(basebackup_options *opt, DIR *tblspcdir) --- 101,114 XLogRecPtr endptr; TimeLineID endtli; char *labelfile; + char cwd[MAXPGPATH]; + int rootpathlen; + + /* we need to compute rootpathlen to allow for skipping tablespaces + * installed within PGDATA + */ + getcwd(cwd, MAXPGPATH); + rootpathlen = strlen(cwd) + 1; backup_started_in_recovery = RecoveryInProgress(); *** *** 165,171 perform_base_backup(basebackup_options *opt, DIR *tblspcdir) /* Add a node for the base directory at the end */ ti = palloc0(sizeof(tablespaceinfo)); ! ti-size = opt-progress ? sendDir(., 1, true) : -1; tablespaces = lappend(tablespaces, ti); /* Send tablespace header */ --- 174,181 /* Add a node for the base directory at the end */ ti = palloc0(sizeof(tablespaceinfo)); ! ti-size = opt-progress ? ! sendDir(., 1, rootpathlen, true, tablespaces) : -1; tablespaces = lappend(tablespaces, ti); /* Send tablespace header */ *** *** 191,197 perform_base_backup(basebackup_options *opt, DIR *tblspcdir) sendFileWithContent(BACKUP_LABEL_FILE, labelfile); /* ... then the bulk of the files ... */ ! sendDir(., 1, false); /* ... and pg_control after everything else. */ if (lstat(XLOG_CONTROL_FILE, statbuf) != 0) --- 201,207 sendFileWithContent(BACKUP_LABEL_FILE, labelfile); /* ... then the bulk of the files ... */ ! sendDir(., 1, rootpathlen, false, tablespaces); /* ... and pg_control after everything else. */ if (lstat(XLOG_CONTROL_FILE, statbuf) != 0) *** *** 779,785 sendTablespace(char *path, bool sizeonly) size = 512; /* Size of the header just added */ /* Send all the files in the tablespace version directory */ ! size += sendDir(pathbuf, strlen(path), sizeonly); return size; } --- 789,795 size = 512; /* Size of the header just added */ /* Send all the files in the tablespace version directory */ ! size += sendDir(pathbuf, strlen(path), 0, sizeonly, NIL); return size; } *** *** 788,796 sendTablespace(char *path, bool sizeonly) * Include all files from the given directory in the output tar stream. If * 'sizeonly' is true, we just calculate a total length and return it, without * actually sending anything. */ static int64 ! sendDir(char *path, int basepathlen, bool sizeonly) { DIR *dir; struct dirent *de; --- 798,810 * Include all files from the given directory in the output tar stream. If * 'sizeonly' is true, we just calculate a total length and return it, without * actually sending anything. + * + * Omit any directory listed in tablepaces, so
Re: [HACKERS] SQL objects UNITs
Stephen Frost sfr...@snowman.net writes: That said, I'm starting to wonder about a few different options that might be handy- having the extension be dumpable (or maybe an option to pg_dump to dump them from the DB, or not), and perhaps an option to have the version # included in the dump (or an option to exclude it, such as when run by pg_upgrade..?). Perhaps similar things for pg_restore. In any case, this is certainly the way I had been hoping the discussion would go.. http://www.postgresql.org/message-id/18778.1354753...@sss.pgh.pa.us -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
SQL objects UNITs (was: [HACKERS] Extension Templates S03E11)
, as they are basically the only known extensions following the same delivery rules as the PostgreSQL core product itself. Almost any other extension existing today builds support for all the PostgreSQL releases in each version of it, meaning that the pecularities of `pg_dump` and `pg_restore` are not going to apply to a `UNIT` in the same way at all. Basically with building `UNIT` we realise with hindsight that we failed to build a proper `EXTENSION` system, and we send that message to our users. -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extension Templates S03E11
Tom Lane t...@sss.pgh.pa.us writes: I keep telling you this, and it keeps not sinking in. How can you say that? I've been spending a couple of years on designing and implementing and arguing for a complete feature set where dealing with modules is avoided at all costs. The problem we have now is that I'm being told that the current feature is rejected if it includes anything about modules, and not interesting enough if it's not dealing with modules. I tried my best to make it so that nothing in-core changes wrt modules, yet finding out-of-core solutions to still cope with that. It's a failure, ok. I think we need a conclusion on this thread: Extension specs are frozen. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extension Templates S03E11
Tom Lane t...@sss.pgh.pa.us writes: Right. I think a lot of the tension comes from people being unconvinced that the existing extension feature is an ideal model for this sort of use-case. Extensions were mainly designed around the notion of a .so The effort here is all about extending the Extension Use Case, yes. OTOH, for a set of pure-SQL objects, it's not necessary that there be a canonical text file somewhere, and we have in principle complete knowledge of the objects' semantics as well as the ability to dump-and-restore into newer PG versions. So it's not at all clear that we should just adopt the existing model with the smallest possible changes --- which AFAICS is basically what this proposal is. Maybe that's the way to go, but we should consider alternatives, and in particular I think there is much more reason to allow inside-the-database mutation of the SQL objects. My thinking is that if we invent a new mechanism for extensions that are not managed like contribs, we will find out that only contribs are going to be using extensions. Given the options of either growing extensions into being able to cope with more than a single model or building an entirely new system having most of the same feature set than Extensions, I'm pushing for the option where we build on top of what we have already. I think the name Extension Templates is horrible because it misleads all of us on this list into thinking the proposed feature is completely something other than what it is. I don't have a better name offhand, but that's got to change before it becomes a feature. Given your previous para, I wonder if library or package would work better. I agree that template isn't le mot juste. We can't use “package” because it means something very different in direct competition. I have other propositions, but they are only relevant if we choose not to improve Extensions… right? Regards, -- Dimitri Fontaine06 63 07 10 78 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extension Templates S03E11
Stephen Frost sfr...@snowman.net writes: My thinking is that if we invent a new mechanism for extensions that are not managed like contribs, we will find out that only contribs are going to be using extensions. That's only accurate if the new mechanism supports .so's, which seems unlikely to be the case. Really? Look at dynamic_library_path, then at a classic CREATE FUNCTION command that maps into a C provided symbol: CREATE OR REPLACE FUNCTION prefix_range_in(cstring) RETURNS prefix_range AS '$libdir/prefix' LANGUAGE C IMMUTABLE STRICT; A packaging or distribution software will have no problem removing the '$libdir/' part of the magic AS string here. Once removed, prefix.so will be loaded from anywhere on the file system paths listed into the dynamic_library_path GUC. So now, you don't need anymore to have file system write privileges into a central place owned by root, it can be anywhere else, and the backend hooks, when properly setup, will be able to benefit from that. The missing bits are where to find the extension control files and scripts. The only reason why the current proposal mention *nothing* about how to deal with modules (.so etc) is because each and every time a mention is made about that problem, the answer from Tom is “rejected, full stop”. What I think we'd end up with is a split between extensions, which would be things containing .so's, and libraries or what-have-you, which would be more-or-less everything else. That kind of a break-down strikes me as perfectly reasonable. Only if it's the best we can do. There would also be flexability in that an author might choose to use an extension even in cases where it's not strictly necessary to do so, for whatever reason they want. Note that of course you can still install proper OS packages when we ship with support for Extension Templates. I'd like to see extensions improved. I don't feel like the proposed 'extension templates' is the way to do that because I don't think it really solves anything and it adds a layer that strikes me as wholly unnecessary. You still didn't propose any other way to have at it, where it's already my fourth detailed proposal. I did spend time on designing what I think you're trying to say hand-wavely in that exchange, and I don't quite like the result, as I see now way for it not to entirely deprecate extensions. Maybe the proper answer is that we should actually confine extensions to being the way to install contribs and nothing else, and deprecate them for cases where you don't have an OS level package. It seems really strange to build a facility with such a generic name as “extension” only to resist changing any of it, then stop using it at first opportunity. Also, I'm not sure about the consequences in terms of user trust if we build something new to solve a use case that looks so much the same. However, as I understand it from the various discussions on this topic outside of this list, the immediate concern is the need for a multi-OS extension distribution network with support for binaries, .so's and .dll's and whatever else, to make installing extensions easier for developers on various platforms. I'm all for someone building that and dealing with the issues associated with that, but building a client for it in core, either in a way where a backend would reach out and download the files or accepting binary .so's through the frontend protocol, isn't the first step in that and I very much doubt it would ever make sense. That's exactly the reason why the first piece of that proposal has absolutely nothing to do with building said client, and is all about how NOT to have to build it in core *ever*. If you don't like what I'm building because it's not solving the problem you want to solve… well don't use what I'm building, right? Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extension Templates S03E11
using `ldd` or the like, so it should be possible to implement the same thing in the software packaging and distribution layer that we keep talking about to complement that feature. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Reference to parent query from ANY sublink
Tom Lane t...@sss.pgh.pa.us writes: Yeah, I found myself wishing for an EXPLAIN option that would show that. It's not hard to do ... how about the attached? +1 I chose to print grouping keys for both Agg and Group nodes, and to show them unconditionally. There's some case maybe for only including them in verbose mode, but since sort keys are shown unconditionally, it seemed more consistent this way. +1 Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Completing PL support for Event Triggers
Peter Eisentraut pete...@gmx.net writes: I think you are mistaken. My patch includes all changes between your v1 and v2 patch. I mistakenly remembered that we did remove all the is_event_trigger business from the plperl patch too, when it's not the case. Sorry about this confusion. My vote is for “ready for commit” then. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extension Templates S03E11
Hi, Stephen Frost sfr...@snowman.net writes: * Jeff Davis (pg...@j-davis.com) wrote: What is stopping Extension Templates, as proposed, from being this special extension creation mode? What would be a better design? The extra catalog tables which store SQL scripts in text columns is one of my main objections to the as-proposed Extension Templates. I view those scripts as a poor man's definition of database objects which are defined properly in the catalog already. I have a very hard time to understand this objection. PL/SQL functions are just a SQL script stored as-is in the catalogs. That applies the same way to any other PL language too, with scripts stored as-is in the catalogs in different languages. Even views are stored in a textual way in the catalogs, albeit in a specific pre-processed format, it's still a text blob that could pass for a script in a backend specific language, parsed by the rewriter. So while I hear your objection to the script in catalog idea Stephen, I think we should move forward. We don't have the luxury of only applying patches where no compromise has to be made, where everyone is fully happy with the solution we find as a community. The other big issue is that there isn't an easy way to see how we could open up the ability to create extensions to non-superusers with this approach. The main proposal here is to only allow the owner of a template to install it as an extension. For superusers, we can implement the needed SET ROLE command automatically in the CREATE EXTENSION command. Is there another security issue that this “same role” approach is not solving? I don't think so. It seems like the porting issue is just a matter of finding someone to write a tool to reliably translate packages from PGXN into a form suitable to be sent using SQL commands; which we would need anyway for this special mode. I already mentionned that's on my roadmap, part of the vision I'm trying to implement here. My goal is to deliver the full solution for 9.4, and this Extension Templates facility is the missing in-core bits of it. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extension Templates S03E11
Robert Haas robertmh...@gmail.com writes: You've got that backwards. We do have the luxury of rejecting new features until people are generally satisfied that the basic design is right. There's no overlord decreeing that this must be in 9.4. I strongly agree. PostgreSQL has succeeded because we try not to do things at all until we're sure we know how to do them right. I still agree to the principle, or I wouldn't even try. Not in details, because the current design passed all the usual criteria a year ago. http://www.postgresql.org/message-id/6466.1354817...@sss.pgh.pa.us I can certainly understand Dimitri's frustration, in that he's written several versions of this patch and none have been accepted. But what The design was accepted, last year. It took a year to review it, which is fair enough, only to find new problems again. Circles at their best. You just said on another thread that perfect is the enemy of good. What about applying the same line of thoughts to this patch? Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Completing PL support for Event Triggers
Peter Eisentraut pete...@gmx.net writes: I think this is wrong, and the reason it crashes if you remove it is that you need to call increment_prodesc_refcount(prodesc), like in the other handlers. Thanks, looks like this indeed. Attached is my final patch. Let me know if it's OK for you. It looks like you started with the v1 of the plperl patch rather than the v2, where the only difference is in only using is_trigger or using both is_trigger and is_event_trigger. Your version currently uses both where I though we chose using is_trigger only. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Add transforms feature
Peter Eisentraut pete...@gmx.net writes: Here is an idea. Add a GUC that basically says something like use_transforms = on|off. You can then attach that to individual functions, which is the right granularity, because only the function knows whether its code expects transforms or not. But you can use the full power of GUC to configure it any way you want. +1 The only thing this doesn't give you is per-argument granularity, but I think the use cases for that are slim, and we don't have a good existing mechanism to attach arbitrary attributes to function arguments. +1 Actually, I'd take this two steps further. First, make this parameter per-language, so something like plpython.use_transforms. Then it's up to the language implementation how they want to deal with this. A future new language could just ignore the whole issue and require transforms from the start. I'm not sure about this level of granularity, but why not. Second, depending on the choice of the language, this parameter could take three values: ignore | if available | require. That would allow users to set various kinds of strictness, for example if they want to be alerted that a language cannot deal with a particular type. My understanding is that it always can deal with any particular type if you consider text based input/output, right? Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extension Templates S03E11
Robert Haas robertmh...@gmail.com writes: Can we separate this feature out? It's an issue with extensions today, and I'm eager to make some progress after the explosion of differing opinions today. +1 for separating that part out. I thought it was separated, at some point. http://www.postgresql.org/message-id/caltqxtetvi-exhdbspuey3trthug51eswuod8cur2t+rxtg...@mail.gmail.com http://www.postgresql.org/message-id/m2r4k8jpfl@2ndquadrant.fr The only way for to bugfix all the reported problems had been to have regression testing… and it's become a necessary dependency of the extension templates patch, so I just included it in. My interdependent git branches development fu seems to have totally disappeared after the main extension patch that needed 7 of thoses… I'd need to look exactly what's being proposed in more detail. What I did propose is a new GUC default_full_version: + termvarnamedefault_full_version/varname (typestring/type)/term + listitem +para + This option allows an extension author to avoid shiping all versions + of all scripts when shipping an extension. When a version is requested + and the matching script does not exist on disk, + set replaceabledefault_full_version/replaceable to the first + script you still ship and PostgreSQL will apply the intermediate + upgrade script as per the commandALTER EXTENSION UPDATE/command + command. +/para +para + For example, say you did provide the extension literalpair/literal + version literal1.0/literal and are now providing the + version literal1.1/literal. If you want both current and new users + to be able to install the new version, you can provide both the + scripts literalpair--1.0--1.1.sql/literal + and literalpair--1.1.sql/literal, adding to the already + existing literalpair--1.0.sql/literal. +/para +para + When specifying literaldefault_version/literal + and literaldefault_full_version = 1.0/literal you can instead only + provide only the scripts literalpair--1.0.sql/literal + and literalpair-1.0--1.1.sql/literal. The commandCREATE + EXTENSION pair;/command will then automatically use the afore + mentionned scripts to install version 1.0 then update it to 1.1. +/para + /listitem What Jeff is proposing is to simplify that down and have PostgreSQL auto discover the upgrade cycle when the version asked for isn't directly available with a creation script. We would keep the behavior depicted here, just in a fully automated way. Working on a separate patch for that, then. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extension Templates S03E11
Stephen Frost sfr...@snowman.net writes: That's not really 'solved' unless you feel we can depend on that create extension from URL to work at pg_restore time... I wouldn't have guessed that people would accept that, but I've already been wrong about such things in this thread once. Basically, with the extra software I want to build out-of-core, what you have is an externally maintained repository and the scripts are downloaded at CREATE EXTENSION time. With the Extension Template, you then have a solid cache you can rely on at pg_restore time. -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extension Templates S03E11
Robert Haas robertmh...@gmail.com writes: In more normal cases, however, the system can (and probably should) figure out what was intended by choosing the *shortest* path to get to the intended version. For example, if someone ships 1.0, 1.0--1.1, 1.1, and 1.1--1.2, the system should choose to run 1.1 and then 1.1--1.2, not 1.0 and then 1.0--1.1 and then 1.1--1.2. But that can be automatic: only if there are two paths of equal length (as in the example in the previous paragraph) do we need help from the user to figure out what to do. Yeah. We should also consider the possibility of a user trying to deliberately install and older release. For example, if the user has 1.0, 1.0--1.1, 1.1, 1.1--1.2, and 1.2--1.0 (a downgrade script) with default_full_version = 1.2, an attempt to install 1.0 should run just the 1.0 script, NOT 1.2 and then 1.2--1.0. In what I did, if you want version 1.0 and we have a script --1.0.sql around, then we just use that script, never kicking the path chooser. The path chooser at CREATE EXTENSION time is only execised when we don't have a direct script to support that specific version you're asking. Putting all that together, I'm inclined to suggest that what we really need is a LIST of version numbers, rather than just one. If there one path to the version we're installing is shorter than any other, we choose that, period. If there are multiple paths of equal length, we That's what Jeff did propose, yes. break the tie by choosing which version number appears first in the aforementioned list. If that still doesn't break the tie, either because none of the starting points are mentioned in that list or because there are multiple equal-length paths starting in the same place, we give up and emit an error. Jeff also did mention about tiebreakers without entering into any level of details. We won't be able to just use default_version as the tiebreaker list here, because of the following example: default_version = 1.2, 1.0 create extension foo version '1.1'; With such a setup it would prefer 1.2--1.1 to 1.0--1.1, which doesn't look like what we want. Instead, we want default_version = 1.2 create_from_version_candidates = 1.0 create extension foo version '1.1'; Then the tie breaker is the 1.0 in create_from_version_candidates so we would run foo--1.0.sql and then foo--1.0--1.1.sql. Comments? Baring objections, I'm going to prepare a new branch to support developping that behavior against only file based extensions, and submit a spin-off patch to the current CF entry. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extension Templates S03E11
Stephen Frost sfr...@snowman.net writes: I understand that you once proposed that and it was shot down but I think we need to move past that now that we've seen what the alternative is.. That isn't to say anything about the code or about you specifically, but, for my part, I really don't like nor see the value of sticking script blobs into the catalog as some kind of representation of database objects. Well yeah, I'm having quite a hard time to withdraw that proposal, which is the fourth one in three years, and that had been proposed to me on this very mailing list, and got the infamous community buy-in about a year ago. It's always a hard time when you're being told that the main constraints you had to work with suddenly are no more, because after all this work, we realize that imposing those constraints actually made no sense. I understand that it can happen, it still really sucks when it does. delusionnal paragraph, censored for lack of humour (incl. sarcasm) Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] RFC: programmable file format for postgresql.conf
Peter Eisentraut pete...@gmx.net writes: On 12/1/13, 2:24 PM, Álvaro Hernández Tortosa wrote: IMHO, defining a new syntax for the postgreql.conf file format, that is suitable for writing and parsing, or using an already existing, well-known, programmatic syntax, could offer a solution for all the problems/limitations above. That's the problem, there isn't one, is there? The closest you'd get is the INI syntax, but that's like CSV, with many variations. And most client libraries for this will likely drop all comments when they read and write a file, so this doesn't address that issue. I've been using INI alot in pgloader previously, and I can't tell you how happy I am to be away from it now. I would argue that plenty of programmatic syntax and well known options do exist for a configuration format. From Emacs Lisp and Guile to Python, including Lua. You will tell me that it's too programmatic for what you think is a configuration file, I would argue that it's the best choice Emacs (and many other pieces of software) made. Also if the programmatic part of the idea looks fine to someone who never used the lisp syntax, just realise that there's nothing simpler to parse nor “better known” (after all, it's been in wild use already for more than 50 years). Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extension Templates S03E11
Heikki Linnakangas hlinnakan...@vmware.com writes: Right. I think Jeff was thinking of a catalog representation for extensions that haven't been installed yet, but are available in the system and could be installed with CREATE EXTENSION foo. I wouldn't mind having a catalog like that. Even without any of this extension template stuff, it would be handy to have a view that lists all the extensions available in the filesystem. http://www.postgresql.org/docs/9.1/static/view-pg-available-extensions.html http://www.postgresql.org/docs/9.1/static/view-pg-available-extension-versions.html There should be no difference between file-based extensions and catalog-based extensions. It's just two different ways to install the same extension. The extension author doesn't need to care about that, it's the DBA that decides which method to use to install it. Agreed. I'm going to object loudly to any proposal that doesn't meet that criteria. Please be kind enough to poin me where my current patch is drifting away from that criteria. What you're proposing here is what I think I have been implementing. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extension Templates S03E11
Stephen Frost sfr...@snowman.net writes: Having a versioning notion (and whatever other meta data we, or an extension author, feels is useful) for what are otherwise simple containers (aka the schematic we already have..) makes sense and it would be great to provide support around that, but not this duplication of object definitions. I don't like duplication either, we've just been failing to find any alternative with pg_restore support for the last 3 years. *That doesn't make this approach the right one*. If anything, I'm afraid we've ended up building ourselves a rube goldberg machine because of this constant struggle to fit a square peg into a round hole. This duplication you're talking about only applies to CREATE EXTENSION. I don't know of any ways to implement ALTER EXTENSION … UPDATE … behaviour without a separate set of scripts to apply in a certain order depending on the current and target versions of the extension. If you know how to enable a DBA to update a set of objects in a database only with information already found in the database, and in a way that this information is actually *not* an SQL script, I'm all ears. That's basically what we already do with schemas today and hence is pretty darn close to what I'm proposing. Perhaps it'd be a way to simply version schemas themselves- heck, with that, we could even provide that oft-asked-for schema delta tool in-core by being able to deduce the differences between schema at version X and schema at version Y. Given that at any moment you have a single version of the schema installed, I don't know how you're supposed to be able to do that? Maybe you mean by tracking the changes at update time? Well that at least would be a good incentive to have Command String access in event triggers, I guess. That would work beautifully, and of course you would have to do that again manually at pg_restore time after CREATE DATABASE and before pg_restore, or you would need to change the fact that extensions objects are not part of your pg_dump scripts, or you would have to name your new thing something else than an extension. We would need a way to dump and restore this, of course. Which is available in the current patch, of course. Having a management system for sets of objects is a *great* idea- and one which we already have through schemas. What we don't have is any kind of versioning system built-in or other metadata about it, nor do we have good tooling which leverages such a versioning or similar system. Exactly. How can we implement ALTER OBJECT … UPDATE TO VERSION without having access to some SQL scripts? The current patch offers a way to manage those scripts and apply them, with the idea that the people managing the scripts (extension authors) and the people applying them (DBAs) are not going to be the same people, and that it's then possible to have to apply more than a single script for a single UPDATE command. I really just don't see this as being either particularly useful nor feasible within a reasonable amount of effort. Shared libraries are really the perview of the OS packaging system. If you want to build some tool which is external to PG but helps facilitate the building and installing of shared libraries, but doesn't use the OS packaging system (and, instead, attempts to duplicate it) then go for it, but don't expect to ship or install that through the PG backend. I'll give you that implementing Event Triggers just to be able to build what you're talking about on top of it and out of core might not be called “a reasonable amount of effort.” The problem found here is that if a non privileged user installs an extension template named “pgcyrpto” then the superuser installs what he believes is the extension “pgcrypto”, the malicious unprivileged user now is running his own code (extension install script) as a superuser. For my part, the problem here is this notion of extension templates in the PG catalog and this is just one symptom of how that's really not a good approach. The only reason for that being the case is that you suppose that root on the file system is more trustworthy as an entity than postgres on the file system or any superuser in the PostgreSQL service. As soon as you question that, then you might come to realise the only difference in between file-system templates and catalog templates is our ability to deal with the problem, rather than the problem itself. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extension Templates S03E11
Stephen Frost sfr...@snowman.net writes: What is the next step to allow an extension pulled down from pgxn to be installed, unchanged, into a given database? An extension packaging system. Unchanged is not a goal, and not possible even today. PGXN is a *source based* packaging system. You can't just install what's in PGXN on the server's file system then CREATE EXTENSION, you have this extra step called the “build”. Whether you're targetting a file system template or a catalog template, PGXN is not a complete solution, you still need to build the extension. As I already mentionned in this thread, that's even true for SQL only extensions today, have a look at this example: http://api.pgxn.org/src/mimeo/mimeo-1.0.1/ http://api.pgxn.org/src/mimeo/mimeo-1.0.1/Makefile So even as of today, given file based extension templates and PGXN, there's something missing. You can find different client tools to help you there, such as pgxn_client and pex: http://pgxnclient.projects.pgfoundry.org/ https://github.com/petere/pex What I want to build is an “extension distribution” software that knows how to prepare anything from PGXN (and other places) so that it's fully ready for being used in the database. Then the main client would run as a CREATE EXTENSION ddl_command_start Event Trigger and would fetch the prepared extension for you and make it available, then leaving the main command operate as intended. Which is what I think the pex extension is doing, and that's not coincidental, but it runs the build step on the PostgreSQL server itself and needs to have a non-trivial set of file-system privileges to be doing so, and even needs to get root privileges with sudo for some of its operations. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extension Templates S03E11
Stephen Frost sfr...@snowman.net writes: Clearly we need the information from the extension package (the scripts which are on the PG server's filesystem today, but need not be in the future) but that doesn't mean we need to keep those text blobs in the catalog. So, I guess it would have been good to hear about that about a year ago: http://www.postgresql.org/message-id/13481.1354743...@sss.pgh.pa.us http://www.postgresql.org/message-id/6466.1354817...@sss.pgh.pa.us We could have CREATE TEMPLATE FOR EXTENSION store the scripts into some files in PGDATA instead of the catalogs, but really I don't see the point. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extension Templates S03E11
Stephen Frost sfr...@snowman.net writes: Fine- so we need a step that goes from 'source' to 'built'. I don't see that step being done in or by a PG backend process. Adding a new option which can take a pgxn source and build a script from it which can be run against PG via libpq is what I'd be going for- but that script *just installs (or perhaps upgrades) the extension.* There's no need for that script, or various upgrade/downgrade/whatever scripts, to be sucked wholesale into the PG catalog. As you said previously, we can't ask extension authors to control what version of their extension is installed on which database, so we need a way to cooperate with the backend in order to know how to operate the update. We can't just pull data out of the backend to do that, not until we've been pushing the list of available versions and update scripts that we have to be able to run the update. That's were I though about pushing the whole thing down to the catalogs and have the backend take control from there. What I want to build is an “extension distribution” software that knows how to prepare anything from PGXN (and other places) so that it's fully ready for being used in the database. Then the main client would run as a CREATE EXTENSION ddl_command_start Event Trigger and would fetch the prepared extension for you and make it available, then leaving the main command operate as intended. I really don't think that's a good approach. What's your alternative? Goals are: - using the update abilities of the extension mechanism - no access to the server's file system needed - pg_restore does the right thing I went for the whole set of extension abilities in my patch, you're pushing hard for me to reduce that goal so I only included the ability to manage version upgrades here. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extension Templates S03E11
Heikki Linnakangas hlinnakan...@vmware.com writes: I fear we're wandering off the point again. So let me repeat: It must be possible to install the same extension the way you do today, and using the new mechanism. The way you do today is running make install or apt-get install or something else to write files in the right place on the file system, usually with root privileges. The new mechanism tries to avoid using the file system *completely*. Sorry. I don't understand what you mean other that “I don't want this patch because I don't understand what it is about”. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extension Templates S03E11
Tom Lane t...@sss.pgh.pa.us writes: Stephen Frost sfr...@snowman.net writes: * Dimitri Fontaine (dimi...@2ndquadrant.fr) wrote: Then as soon as we are able to CREATE EXTENSION mystuff; without ever pre-installing files on the file system as root, then we would like to be able to do just that even with binary modules. I really just don't see this as being either particularly useful nor feasible within a reasonable amount of effort. Shared libraries are really the perview of the OS packaging system. Yes, exactly. What's more, you're going to face huge push-back from vendors who are concerned about security (which is most of them). Last time I talked with vendors, they were working in the Open Shift team at Red Hat, and they actually asked me to offer them the ability you're refusing, to let them enable a better security model. The way they use cgroups and SELinux means that they want to be able to load shared binaries from system user places. If there were such a feature, it would end up disabled, one way or another, in a large fraction of installations. That would make it impractical to use anyway for most extension authors. I don't think it's good project policy to fragment the user base that way. That point about fragmentation is a concern I share. I'm on board with the notion of an all-in-the-database extension mechanism for extensions that consist solely of SQL objects. But not for ones that need a .so somewhere. Thanks for restating your position. The current patch offers a feature that only works with SQL objects, it's currently completely useless as soon as there's a .so involved. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extension Templates S03E11
Stephen Frost sfr...@snowman.net writes: On the other hand, I can appreciate the concern that we don't really want a dump/restore to include the extension definition when it's already on the filesystem. That said, it amazes me that we don't include the version # of the extension in pg_dump's 'CREATE EXTENSION' command.. How is that not a problem? Including the version number would be a problem. When you install PostgreSQL 9.1, you only have hstore 1.0. When you install PostgreSQL 9.2, you only have hstore 1.1. When you install PostgreSQL 9.3, you only have hstore 1.2. http://git.postgresql.org/gitweb/?p=postgresql.git;a=blob;f=contrib/hstore/hstore.control;hb=refs/heads/REL9_1_STABLE http://git.postgresql.org/gitweb/?p=postgresql.git;a=blob;f=contrib/hstore/hstore.control;hb=refs/heads/REL9_2_STABLE http://git.postgresql.org/gitweb/?p=postgresql.git;a=blob;f=contrib/hstore/hstore.control;hb=refs/heads/REL9_3_STABLE We should maybe add the extension's version number in our documentation pages, such as the following: http://www.postgresql.org/docs/9.3/interactive/hstore.html So when you pg_dump | pg_restore from 9.1 into 9.3, if pg_dump were to be nitpicky about the version of hstore with the command CREATE EXTENSION hstore VERSION '1.0'; What would happen is that pg_restore would fail. That's just the way we maintain contribs. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extension Templates S03E11
Stephen Frost sfr...@snowman.net writes: I don't like the idea of having a pg_dump/restore mechanism that intentionally tries to go out and install the latest version of whatever extension was installed in the old DB by downloading it from PGXN, building it, and then installing it... Is that what people are expecting here? The whole idea of having Extension Templates in catalogs is exactly to prevent what you're describing here from happening. Whatever the templates you downloaded to get to the version you now have in your database for extension “foo” are going to used again by pg_restore at CREATE EXTENSION time. The extension depending on its in-catalog templates ensures that model of operations. You can copy/paste some extension examples from the regression tests and pg_dump -Fc | pg_restore -l to see the details, or something. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extension Templates S03E11
Greg Stark st...@mit.edu writes: On Mon, Dec 2, 2013 at 6:30 PM, Robert Haas robertmh...@gmail.com wrote: OK, I'll bite. I've been trying to stay out of this thread, but I really *don't* understand what this patch is about. Extensions, as Thanks! they exist today, are installed from the filesystem and their contents are not dumped. You're trying to create a new kind of extension which is installed from the system catalogs (instead of the file system) and is dumped. Why should anyone want that? To benefit from ALTER EXTENSION … UPDATE … and \dx. And technically the extension is not dumped, its templates are. It seems that part of the answer is that people would like to be able to install extensions via libpq. You could almost write a client-side tool for that today just by using adminpack to write the files to the server, but you'd trip over the fact that files written by adminpack must be in either the data directory or the log directory. But we could fix that easily enough. Trick question: when you've implemented said client and used it for a couple of (in-house) extensions, what do you think should happen at pg_restore time? Hint: in a properly designed ops model, pg_restore happens each and every day when the unattended cron job “rebases” the QA or testing environments from the production PITR backups, of course. Just tossing an idea out there. What if you could install an extension by specifying not a local file name but a URL. Obviously there's a security issue but for example we could allow only https URLs with verified domain names that are in a list of approved domain names specified by a GUC. That's something I want to build. This time, not in core. The model I've been thinking about involves an EVENT TRIGGER that is fired at ddl_command_start for CREATE EXTENSION and prepares an EXTENSION TEMPLATE before the command has a chance to check what's available and install the current default version of it. Also runs at ALTER EXTENSION … UPDATE …, of course, providing the upgrade scripts on the fly. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extension Templates S03E11
Stephen Frost sfr...@snowman.net writes: * David E. Wheeler (da...@justatheory.com) wrote: This is true today, but only because PostgreSQL provides the infrastructure for building and installing extensions that entails `make make install`. If Postgres provided some other method of building and installing extensions, you could start using it right away on PGXN. The *only* requirement for PGXN distributions, really, is a META.json file describing the extension. Thanks, that's a pretty interesting point.. I like the idea that we could provide a new make target which could build an 'inline extension' (or what-have-you) which could then be distributed and used by users either directly or with some client-side tool. +1 -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extension Templates S03E11
Stephen Frost sfr...@snowman.net writes: What I've been trying to point out is that there's absolutely zero need for the 'extension template' part of this to make a pg_restore work for an entirely-in-the-catalog extension. I realize that's how you've done it with this patch set but that doesn't make it necessary. If it's an extension, it's filtered out of pg_dump, so it's not part of your pg_restore. Full Stop. This point has been debated and there has been a very clear conclusion a year ago. What am I missing here? -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extension Templates S03E11
Robert Haas robertmh...@gmail.com writes: Speaking only for myself, I think the thing I most disliked about that proposal was the syntax. I'd rather see each extension member dumped separately, and then later dump the extension itself as CREATE EXTENSION ... WITH NO CONTENTS or similar followed by ALTER EXTENSION ... ADD item for each member. That would provide a way of handling dependency loops, which Dimitri's proposed syntax did not, and just in general seems more elegant. But it's not perfect: for example, I could have fixed the syntax quite easily, within 9.3 time frame. there's no clean way to handle the situation where the extension is present in the filesystem on the old database but not the new, or visca versa, and I don't think anyone's proposed *any* really clean way of handling that yet. Well, my memories is that I did propose thinking about upgrade paths mixing template sources, and Tom objected quite strongly to doing that. IIRC the line of thoughs is that it's indeed a very complex problem to solve, and renaming the extension when you switch your distribution model might be all you need. Same with incompatible major versions, when there's no integrated upgrade path possible. Fundamentally, I think this is a pretty hard problem. The OS-level equivalent of extensions is something like RPMs or .deb files, and I can't help but observe that those are only used for system-wide installations, not per-user installs. I think the reason we're having If you want to dive into system level unprivileged package management, have a look at that: http://www.gnu.org/software/guix/ a hard time coming up with a satisfactory way of making this work is that an extension as installed from SQL using libpq is a pretty different beast from an extension as installed via the filesystem, and bending the existing mechanism to make that work is somewhat painful no matter how you do it. The argument was made then, and with some validity, that we just shouldn't make the same mechanism serve both purposes. What I now understand (that I think I probably didn't fully understand back then) is that part of the point here is to enable installation of extensions without requiring local filesystem access; using a completely different mechanism would defeat that goal. But I'm still not altogether happy with where that's landed us. I'd like to better understanding what is so wrong about the current design in terms that I'm not feeling like we did address a year ago. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extension Templates S03E11
Stephen Frost sfr...@snowman.net writes: I'm not convinced we really need to solve that problem, but one way to solve it 'cleanly' would be to seperate the two types of extensions, perhaps by calling them by different names or inventing a namespace for extensions. My understanding is that this line of thoughts gave us Extension Templates which are templates, not extensions. I think we're falling into the trap of thinking that whatever this user-installable-collection-of-objects thing is, it must be considered PG 'extensions'. While I agree that some of the things we do for extensions should also be done with these collections of objects (eg: having versions and other meta-data for them), I'm starting to think that's the small side of this whole equation and duplicating that meta-data store for these collections would be easier than trying to shoehorn them into the existing notion of 'extensions'. My main question when thinking that way is: - how to update from a version to another one? The point about extensions is that we separate the author who maintains the upgrade scripts from the DBA who operates the upgrades. It seems to me it's important to keep that property. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extension Templates S03E11
Jeff Davis pg...@j-davis.com writes: I don't see why we are trying to accommodate a case where the author doesn't offer enough full SQL scripts and offers broken downgrade scripts; or why that case is different from offering broken upgrade scripts. That's fair enough I guess. I will work on automating the choice of the first full script to use then, for next patch version. Thanks, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extension Templates S03E11
Jeff Davis pg...@j-davis.com writes: So maybe we should have “SECURITY DEFINER” and “SECURITY INVOKER” extension templates, the default being “SECURITY DEFINER”? That doesn't seem to answer Heikki's stated concern, because a malicious non-superuser would just declare the trojan extension to be SECURITY INVOKER. It does answer if only superusers are allowed to install SECURITY INVOKER templates, which I forgot to add in the previous email. Or at least my understanding is that it could work that way. As I see it, the problem is more about namespacing than anything else. It's analogous to a shell which includes the current directory in the $PATH -- a malicious user can just name an executable ls and trick root into executing it. The solution for a shell has nothing to do with setuid; so I'm reluctant to base our solution on SECURITY DEFINER. I prefer a solution that prevents the kind of name collisions that would trick a privileged user. My strawman idea was to just say that an extension template created by a non-superuser could only be instantiated by that same user. Yes that's a simpler model. And simpler is better when talking security. The only drawback of that is to forbid the superuser from executing a command. That would be new in PostgreSQL I think. We can work around that with automating the SET ROLE to the template owner when a superuser is creating the extension. That's what led me to the SECURITY DEFINER proposition. Either of those solution are fine to me, with or without the automated SET ROLE when a superuser is installing an extension from a template owned by a non-superuser. Tell me your preference, I'll work on the code. Also consider multi-tenancy installations. Certainly, you don't want any database owner to be able to review PL code from any other database owner in the same cluster when each database owner is another customer. That could be solved by permissions, as well, right? I still think about extensions as being a per-database thing, and that the current security policy makes if a per-major-version thing when the extension contains a module (.so). Also, the dynamic_library_path already allows us to make binary extensions a per-database object again, baring incompatibilities that would manifest themselves as run-time errors… So I strongly vote against making the Extension Templates a set of shared catalogs. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extension Templates S03E11
Jeff Davis pg...@j-davis.com writes: I think that Stephen was just talking about the naming. I would have expected the names to be something like xtmpl (which is the shortest abbreviation that came to my mind) rather than tpl, for instance. Use of template is a bit ambiguous. To be honest I'm not following along with the complaint. What object would you liked to be named xtmpl rather than what it is called now? If you're refering to the catalog column names such as tplname and tplowner and the like, the tradition AFAICS is to find a trigram prefix for those entries… and that's what I did. However, I find the full version quite awkward still. I don't think it's purely a documentation issue -- the default full version concept itself is a bit awkward. Yes. I coulnd't find a better name, and I would quite like that we do. There are two ideas behind that feature: - When we released hstore 1.1 we had a choice of either providing still hstore--1.0.sql and hstore--1.1.sql or only the latter, deprecating the 1.0 version for the next release. The default_major_version feature allows to only ship hstore-1.0.sql and still offer full support for hstore 1.0 and 1.1. Note that it's problematic given the current implementation of pg_upgrade, where when migrating from 9.1 to 9.2 (IIRC that's when hstore got to 1.1) you can result in either still having hstore 1.0 in your database if you used pg_upgrade or have hstore 1.1 already if you used pg_dump and pg_restore. Note also that if you install 9.2 then by a technically constrained choice of policy in the project, then you cannot continue using hstore 1.0. So the goal is to simplify extension authors management of sql install and upgrade scripts while giving more options to the users of extension wrt the version they want to be using. - With Extension Templates, the extension author can be providing scripts foo--1.0.sql and foo--1.0--1.1.sql and run the upgrade with ALTER EXTENSION foo UPDATE TO '1.1'; Now what happens at pg_restore time? We only have the 1.0 and 1.0--1.1 scripts, yet we want to be installing foo version 1.1. So we need the default_major_version capabilities, whatever the name we choose. Hence my inclusion of that feature in the Extension Template patch. If I understand correctly, it goes something like this: When a user does: CREATE EXTENSION foo VERSION '3.0'; and there are templates for 1.0, 2.0, and 2.1, and upgrades from 1.0-2.0, 2.0-2.1, and 2.1-3.0, then there are three possible actions that might be taken: 1. Install 1.0 and run three upgrades 2. Install 2.0 and run two upgrade 3. Install 2.1 and run one upgrade With PostgreSQL versions 9.1, 9.2 and 9.3, given the scripts you're listing, the command you propose will just fail. Full stop. ERROR. The second one (in an else branch) shouldn't happen, assuming default_full_version points at a proper full version, right? Will review, thanks. It seems like it's for pg_dump, so it can avoid outputting the extension templates and just say VERSION 'x.y' without worrying about which version it needs to start from. Exactly. We need pg_dump to be smart enough for handling the case as soon as we have Extension Templates, and we already have said smarts in the backend code. They were just not applied at CREATE EXTENSION time before this patch. That seems like a legitimate purpose, but I think we can come up with something that's a little easier on users and easier to document (and name). Perhaps just find the shortest upgrade path to the version requested (using some arbitrary but deterministic tiebreaker)? The danger is that the shorter path could well include a downgrade, and nobody tests for downgrading paths IME. So I keep thinking we should ask the extension author to give us a starting point. Now, if you have a better name for it than “default_full_version”, I'm all ears! Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extension Templates S03E11
Jeff Davis pg...@j-davis.com writes: When a superuser CREATE EXTENSION against a template that has been provided by a non-privileged user, automatically SET ROLE to that user before doing so, avoiding escalation privileges. That proposal is worded like a special case for superusers, and I don't see why. If the security model is that an extension script is run with as the template owner, then we should just do that universally. If not, making a special case for superusers undermines the security of powerful-but-not-superuser roles. I like that idea yes. I haven't looked in detail at the security issues here... is this the result of a consensus or are there still differing opinions? AFAIK past reviewers came up with the privilege escalation use case and said we'd better have that feature a superuser only one. It's playing safe, but I wish we could find another solution. We already have a model for executing functions, and those are black boxes of code as well. If we deviate too much from that, I think we're inviting problems. So maybe we should have “SECURITY DEFINER” and “SECURITY INVOKER” extension templates, the default being “SECURITY DEFINER”? Aside: why do file-based templates shadow catalog-based templates? Shouldn't we just throw an error if both are available at CREATE EXTENSION time? That sounds good too. We need to ERROR out at UPDATE time too of course. Also, I notice that the extension templates are not in shared catalogs; was that discussed? Yes it was. The current model for extensions is to be per-database, but it's limited by the way we deal with modules (.so), for security reasons that encompass the per-database model. Also consider multi-tenancy installations. Certainly, you don't want any database owner to be able to review PL code from any other database owner in the same cluster when each database owner is another customer. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extension Templates S03E11
Jeff Davis pg...@j-davis.com writes: 1. During the initial development of extensions, there was discussion about ordered version numbers and dependencies on the version (e.g. requires foo = 2.1). Outside the scope of this patch, of course, but is that something that we can still do later? Or are we building infrastructure that will make that nearly impossible in a release or two? The best answer I can think of as of today has been proposed on list already in a patch names finer extension dependencies, where I failed miserably to explain my ideas correctly to -hackers. Here are the archive links to the opening of the thread and the latest known version of the patch: http://www.postgresql.org/message-id/m2hb0y2bh3@hi-media.com http://www.postgresql.org/message-id/871uoa4dal@hi-media-techno.com Meanwhile, at pgcon 2013 unconference about pg_upgrade, with Bruce and Peter we had a chat about this problem, coming from a different parallel that might allow for a better explaning of what is it all about: Distribution binary packages nowadays are tracking symbol level dependencies in between binaries (shared objects and programs). To do so they use some dynamic capabilities (objdump and the like, IIUC). In the PostgreSQL we just can't do that automatically, mainly because of EXECUTE support in PL functions and DO blocks. We are not even able to track inter-function dependencies because of that, so that you're allowed to DROP a function that you actually depend on. The “features” proposal where an extension “provides” a set of at least one feature (its own name) is a way for the extension author to list a subset of the symbols provided in his extension so that other extensions are able to depend on some of them. Then this set of “features” can be edited from a version to the next, adding new “features” and removing existing ones. With “feature” dependency tracking, we are able to prevent extension updates when the new version deprecates features that we know are still in the dependency graph: # ALTER EXTENSION foo UPDATE TO '2.0' ERROR: extension bar depends on feature baz provided by foo version '1.1' My plan is to be able to someday explain what I was trying to do in a way that allows for the patch to be considered again, and I have some faith in that plan because I remember Peter suddenly understanding what this was all about when in Ottawa earlier this year. It should be quite trivial to update the patch I had then to also support Extension Templates, basically adding a new column in the catalogs, grammar support in the control properties, and making sure that the value actually goes from the template into the extension catalogs at CREATE or UPDATE time. Mind you, if the Extension Templates patch fate is solvable this Commit Fest, and provided that the “extension features” appears to be a good idea for someone else than just me, I would happily be proposing an updated version of it for next Commit Fest, in time fro 9.4. It was a very simple patch. Of course, given Extension Templates, I would be adding some regression tests to said patch ;-) 2. People will want to start using this feature to control and version their schema. Do you have comments about that? Should we eventually try to get extensions to support that use case (if they don't already), or should that be a completely different feature? The tension between extensions and database schema is pretty simple: an Extension content is not part of pg_dump. With the idea of Extension Templates, you now have two kinds of templates: file system based templates, excluded from the dumps and managed separately (typically managed as an OS package), and catalog templates, parts of the dumps, fully managed by PostgreSQL. Both ways, at pg_restore time the extension is built again from the templates, wherever we find those. The term “extension” covers for a single set of behaviours. So my current thinking is that Extension Templates as currently developped will not be a solution to version controling your database schema. I have not been thinking about how to make that happen other than the following two points: - yes I would like PostgreSQL to offer database versioning capabilities; - no I don't think Extensions can be made into providing support for that set of features. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extension Templates S03E11
Stephen Frost sfr...@snowman.net writes: Without that, all of the information about a given extension is already in the database in our dependency system. As you pointed out, there was That's not entirely true. We would still be missing some information from the extension control file. previously a notion of inline templates. I'm not sure that particular patch is exactly where we want to go, but I absolutely do not like this idea that we have a template on a per-database level which does nothing but duplicate most of the information we *already have*, since you have to assume that if the extension template (which is per-database) exists then the extension has also been created in the database. That's a classic bootstrap problem. If you consider that the extension is already installed, then you don't need to know how to install it. The main feature that the patch provides is installation path for an extension that doesn't involve the server's file system. Having a versioning notion (and whatever other meta data we, or an extension author, feels is useful) for what are otherwise simple containers (aka the schematic we already have..) makes sense and it would be great to provide support around that, but not this duplication of object definitions. I don't like duplication either, we've just been failing to find any alternative with pg_restore support for the last 3 years. If you want the simplest possible patch that would enable you bypassing the file system, here's what I would be proposing: have a special flag allowing CREATE EXTENSION to just prepare pg_extension catalog entries. Then create your objects as usual, and use ALTER EXTENSION … ADD … to register them against the existing extension. That would work beautifully, and of course you would have to do that again manually at pg_restore time after CREATE DATABASE and before pg_restore, or you would need to change the fact that extensions objects are not part of your pg_dump scripts, or you would have to name your new thing something else than an extension. Also, please note that I did propose that design when working on the first patch series for extension (8.4 and 9.0 eras), or at least some variant where the control properties came in from some command rather than from a file, and it was rejected because the CREATE EXTENSION bootstrapping was then judged too complex, and it was not clear how extension authors were going to maintain their scripts. The current extension model is simple enough to reason about. A script must be provided in a template and is executed at CREATE EXTENSION time or at ALTER EXTENSION UPDATE time, and pg_dump only contains the CREATE EXTENSION command, so that pg_restore has to find the template again. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Status of FDW pushdowns
Tom Lane t...@sss.pgh.pa.us writes: I'm not real sure what this'd buy us that wouldn't be done as well or better by creating a view on the remote side. (IOW, there's nothing that says that the remote object backing a foreign table can't be a view.) Agreed, for those remote sides that know what a view is. -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Status of FDW pushdowns
Alvaro Herrera alvhe...@2ndquadrant.com writes: Seems to me that if you want to read remote tables without creating a foreign table, you could define them locally using something like the WITH syntax and then use them normally in the rest of the query. I guess what's needed here is a kind of barrier that allows pushing a whole arbitrary subquery (with joins and quals and whatnot) down to the remote side. My current thinking about how to solve that would be to add a notion of FOREIGN VIEW in our system, which would basically implement that barrier and send the view definition on the remote, with known quals values as constants, or something like that. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Status of FDW pushdowns
Shigeru Hanada shigeru.han...@gmail.com writes: I'm sorry but I don't see the point here. Do you mean that user executes CREATE FOREIGN VIEW in advance and uses the view in a Yes that's what I mean. I think it's nice to support executing ad-hoc remote query written in the syntax which is valid only on remote data source through FDW, and at the moment dblink interface seems feasible for that purpose. I guess the view query would have to be validated by the FDW, which would just receive a text. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Status of FDW pushdowns
Atri Sharma atri.j...@gmail.com writes: This would work,but how can we do it for FDWs which do not parse SQL? Am I missing something here? Worst case: CREATE FOREIGN VIEW foo AS $$ whatever syntax is accepted on the other side $$; Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Status of FDW pushdowns
Atri Sharma atri.j...@gmail.com writes: Can we add a function to the FDW API to define a SQL to foreign server side conversion? I think that's not tenable. Even if you limit the discussion to the postgres_fdw, some queries against past version will stop working against new version (keywords changes, catalogs, default settings, etc). I don't think you want to embed a full parser of every supported FOREIGN version of PostgreSQL inside the postgres_fdw code, so I think the text of the view needs to be an opaque string. -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extension Templates S03E11
Hi, Heikki Linnakangas hlinnakan...@vmware.com writes: I still don't like this. What I suggested back in December was to have a simple mechanism to upload an extension zip file to the server via libpq (http://www.postgresql.org/message-id/50bf80a6.20...@vmware.com). The idea developed from that into the concept of extension templates, but the original idea was lost somewhere along the way. And I will quote Andres' answer to your same proposal: http://www.postgresql.org/message-id/20121205172747.gc27...@awork2.anarazel.de So having a mode for pg_dump that actually makes dumps that are usable for recovering after a disaster seems sensible to me. Otherwise you need to redeploy from the VCS or whatever, which isn't really what you want when restoring a database backup. Comparing the situation to the one where you have extensions provided by the packaging system or by /contrib or whatever doesn't seem to be all that valid to me. If you continue reading the thread from back then, the conclusion was to drop the patch I was then proposing and instead work on the one we are currently reviewing. Back in December, when I agreed that upload zip file via libpq might be useful, Tom suggested that we call control+sql file a template, and the installed entity an extension. So far so good. Now comes the patch, and the extension template no longer means a control+sql file. It means an entity that's installed in the database that contains the same information as a control+sql file, but in a new format. In fact, what *do* you call the control+sql file that lies on the filesystem? Not a template, apparently. It's historical. To make it possible to start with some extension patch in the 9.0 development cycle, it's been decided to only target the contrib style extensions. Thanks to that we add something in 9.1. In practice, the patch currently under review makes it so that both the file system based model and the catalog based model behave the same (as much as possible and sensible, and thanks to lots of reviewing efforts from Markus Wanner), so we could be refering to the file system based model as “template”. I want to be able to download extension.zip from pgxn.org, and then install it on a server. I want to be able to install it the traditional way, by unzipping it to the filesystem, or via libpq by using this new feature. I do *not* want to rewrite the extension using a new CREATE TEMPLATE FOR EXTENSION syntax to do the latter. I want to be able to install the *same* zip file using either method. I would like to be able to support that, and the theory is attractive. In practice, it's not that simple. PGXN implements a source based distribution model, and most extensions over there are following the same model, where the needed SQL files are derived from sources at build time, using make(1). See the following examples, the first one includes a C source file and the second one is all PL stuff: http://pgxn.org/dist/first_last_agg/ http://api.pgxn.org/src/first_last_agg/first_last_agg-0.1.2/ sql/$(EXTENSION)--$(EXTVERSION).sql: sql/$(EXTENSION).sql cp $ $@ http://pgxn.org/dist/mimeo/1.0.1/ http://api.pgxn.org/src/mimeo/mimeo-1.0.1/ sql/$(EXTENSION)--$(EXTVERSION).sql: sql/tables/*.sql sql/functions/*.sql cat $^ $@ So, to support uploading PGXN zip files directly within the backend, now the backend must be in a position to unpack the archive and build the extension, then it must know where the build artefacts are going to be found or it needs to `make install` in a known prefix and follow our current conventions to find the files. As I said to David Wheeler when he did build PGXN, I don't think that a source level distribution is going to help us dealing with production deployments. So, while I understand where you're coming from, please tell me what are your answers for those two design questions about the Extension template idea: - what should happen at pg_restore time? - do you really want the extension build infrastructure in core? My current thinking is to build the missing infrastructure as a contrib module that will know how to divert CREATE EXTENSION with an Event Trigger and apply the necessary magics at that time, and fill in the Extension Templates for you. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Add transforms feature
Hi, Allow me to temporarily skip important questions that you asked so that we can focus on the main problem here. As soon as we decide how to handle any kind of selectivity for the transforms, then I'm back to answering the other things. Peter Eisentraut pete...@gmx.net writes: Let's review that, as there as been some confusion about that. The SQL standard syntax is Thanks for that. I see that you reused parts of the concept and keywords and implemented something specific to PostgreSQL from there. As there's no collision within the command sets, I think that problem is cleared. I had proposed disallowing installing a transform that would affect existing functions. That was rejected or deemed unnecessary. You can't have it both ways. ;-) Well I'm not so sure, that's the point. A mechanism allowing for the transform to only be used in some functions but not others might be useful. The simplest such mechanism I can think of is modeled against the PL/Java classpath facility as specified in the SQL standard: you attach a classpath per schema. Anything that's a problem per-database would also be a problem per schema. But a smaller problem, and as problem get smaller they get easier to reason about and fix too. In the example given by Robert Haas in the same thread, you could install extension A in schema A using the transforms for hstore and plperl and extension B in schema B not using the same transforms. I'm not saying using the schema that way is awesome, just that we have solid precedent in the standard and in pljava, and it looks easy enough to implement (we already have search_path invalidations IIRC). This is a transition problem. Nobody is required to install the transforms into their existing databases. They probably shouldn't. I disagree. How many people actually use hstore with PL/Perl or PL/Python now? Probably not many, because it's weird. I like to think about how this works for new development: Here is my extension type, here is how it interfaces with languages. Once you have established that, you don't want to have to repeat that every time you write a function. That's error prone and cumbersome. And anything that's set per schema or higher is a dependency tracking and caching mess. The problem is installing a set of extensions where some of them are already using the new transform feature and some of them are not. We need a way to cater with that, I think. Also, extension types should work the same as built-in types. Eventually, I'd like to rip out the hard-coded data type support in PL/Python and replace it with built-in transforms. Even if we don't actually do it, conceptually it should be possible. Now if we require I like that idea a lot. I don't see how you can make it work by default, as like Robert I think the transition phase you're talking about will never end. USING TRANSFORM FOR int, bytea every time, we'd have taken a big step back. Effectively, we already have built-in transforms in PL/Python. For core types only. We have added a few more over the years. It's been a bit of a pain from time to time. At least, with this feature we'd be moving this decision into user space and give people a way to fix things. (Incidentally, if you add a lot of transforms, you are probably dealing with a strongly typed language. And a strongly typed language is more likely to cleanly catch type errors resulting from changes in the transforms.) I think use space will want to be able to use code written using different sets of transforms for the same set of types, rather than being forced into upgrading their whole code at once. It reminds me of the python 2 to python 3 upgrade path. This is not solved yet, with libs that are python2 only and others that are python3 only, and OS that would ship some but not others. We already have the problem that shipping contrib by default is frowned upon at some places, then they miss out of plenty of awesome extensions. Transforms with no granularity is only going to make it worse, I think. So we have to choose the granularity: - per database (current patch), - per schema (standard has precedents with pljava, - per extension (forcing users into using extensions, not good), - per function (hard to maintain), - something else. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extension Templates S03E11
Tom Lane t...@sss.pgh.pa.us writes: As I've said multiple times before, this is an absolute nonstarter. FWIW, I was explaining the model that I didn't want to follow. Thanks for approving, even if that's not a surprise as the model I did follow is the one we agreed on a year ago. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extension Templates S03E11
Stephen Frost sfr...@snowman.net writes: These wouldn't be PG extensions really though, which it seems folks are pretty hung up on. There would also be no support for binary or untrusted components, which is a bit frustrating, as you'd like to be able to support those if you're a superuser. Trying to build both into one extension template structure, or what-have-you, seems to be causing us to fail to make any progress on either use-case. So, plenty of points to address separately here: - why do we want extensions to manage PL user code? - what about managing extensions with modules from the protocol directly, if we have Extension Templates ? - is it possible to open the feature to non-superusers ? Let's have at it. # Why do we want extensions to manage PL user code? Extensions allows to manage any number of database objects as a single entity with a very clear and simple enough life cycle management tooling: create extension … alter extension … update to … drop extension … [cascade] \dx \dx+ pg_available_extensions() pg_available_extension_versions() Plus, you can handle inter-extension dependencies, albeit currently in a very limited way, but that's still something. Now that we have those tools, I want to be able to use them when dealing with a bunch of PL code because I didn't find a better option in PostgreSQL for dealing with such code. It could well be that the “package” thing that I couldn't find tonight in the SQL standard documents are better suited to manage user code, but after having had to write management code and use extensions, I know I want to deal with PL user code the same way I deal with extensions. Of course, it's already possible to do so, packaging the user code into a set of files that you then need to install at the right place on the filesystem (a place owned by root in most cases), so I already did have the pleasure to use extensions for user code. And I also had to write all the packaging myself, then ship it to several nodes, etc. I think that opening Extensions to be easy to use without requiring root level access to the database server's filesystem would be a great idea, even if it would take, I don't know, say, 3 years of development to get there. # Extension Templates and Binary Modules Then as soon as we are able to CREATE EXTENSION mystuff; without ever pre-installing files on the file system as root, then we would like to be able to do just that even with binary modules. The current documentation of the dynamic_library_path GUC makes me believe it's possible already without patching the backend. We will need to patch the extension's CREATE FUNCTION statements not to hardcode $libdir in there of course. That could be done automatically by some separate distribution packaging software. Some cases would still need more work before being supported within that frame: - Hot Standby - Modules that depend on other libs being installed. # Extension Templates and Superusers The problem found here is that if a non privileged user installs an extension template named “pgcyrpto” then the superuser installs what he believes is the extension “pgcrypto”, the malicious unprivileged user now is running his own code (extension install script) as a superuser. The current patch will prioritize file-based templates when an extension is provided both from the file system and the catalogs, so for the situation to happen you need to have forgotten to install the proper system package. Still. I've been asked (if memory serves) to then limit the Extension Templates feature to superuser, which is a very bad thing™. What I now think we should do is only grant superusers the privileges to install an extension from a template they own or is owned by another superuser. The general rule being that only the role who did install the template would be allowed to install an extension with it, so that you can't have a privilege escalation happening, IIUC. As a superuser, use SET ROLE first. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Status of FDW pushdowns
Merlin Moncure mmonc...@gmail.com writes: By 'insignificant' you mean 'necessary to do any non-trivial real work'. Personally, I'd prefer it if FDW was extended to allow arbitrary parameterized queries like every other database connectivity API ever made ever. But in lieu of that, I'll take as much push down power as possible :-D. That sounds more like FOREIGN VIEW and FOREIGN FUNCTION to me, where you could have the whole control of the local/remote boundaries. I mean that when planning a query using a FOREIGN VIEW it would probably make sense to consider it as a CTE as far as the optimizer is concerned. About FOREIGN FUNCTION, that would allow to inject arbitrary parameters anywhere in the remote query when doing SQL functions. We have a very nice version of FOREIGN FUNCTION already, that's plproxy. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] new unicode table border styles for psql
Pavel Stehule pavel.steh...@gmail.com writes: there is other issue - simply parser will be really user unfriendly, and user friendly parser will not by simply :( If simple things are hard to implement, get yourself better tools. Each time we get on the topic of improving scripting abilities for our interactive tool, it's always the same problem: having to invent a scripting language with a whole parser is just too much work. Maybe it's time we step back a little and consider real scripting solutions to embed into psql, and pgbench too: http://ecls.sourceforge.net/ LGPL Common Lisp http://www.gnu.org/software/guile/ LGPL Scheme, Javascript, Emacs Lisp http://www.lua.org/ MIT Lua Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] new unicode table border styles for psql
Andrew Dunstan and...@dunslane.net writes: Even if it is it's totally off topic. Please don't hijack email threads. Well, when I read that parsing a user setup is too complex, for me that calls for a solution that offers more power to the user without us having to write specialized code each time. I'm sorry, but I don't understand how off-topic or hijack applies here. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] new unicode table border styles for psql
Andrew Dunstan and...@dunslane.net writes: I'm sorry, but I don't understand how off-topic or hijack applies here. And I just realize there's another way to read what Pavel said, which is that *user scripts* parsing the output of psql might become harder to write as soon as they don't control the default border style in use. Well in that case, yes I'm vastly off-topic. I was answering to how to parse the user setting itself, so writing C code inside the psql source tree itself, and how to expose a fine grained solution to that problem without having to write a whole new configuration parser. It just seems to me to be a very big stretch to go from the topic of psql border styles to the topic of psql scripting support. Your use case would surely be using a sledgehammer to crack a nut. By all means argue for better scripting support in psql, but I would suggest your argument would be better if the use case were something more important and central to psql's purpose. I think I just understood something entirely different that what you were talking about. -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Completing PL support for Event Triggers
Peter Eisentraut pete...@gmx.net writes: I have committed the PL/Tcl part. I'll work on the PL/Perl part next. Thanks! I believe we're still waiting on something from you for PL/Python. Yes I still need to figure that one out. -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extension Templates S03E11
Hi, Jeff Davis pg...@j-davis.com writes: In the CF app, this is marked Ready for Committer. That's a bit vague here, considering Dimitri, you, Peter, and Alvaro are all committers. Who is this patch waiting on? Is the discussion concluding, or does it need another round of review? Thanks for the confusion I guess, but I'm no committer here ;-) This patch has received extensive review in July and I think it now properly implements the design proposed by Tom and Heikki in 9.3/CF4. As the path didn't make it already, yes it needs another (final) round of review. The main difficulty in reviewing is understanding the design and the relation in between our current model of extensions and what this patch offers. You might find the discussions we had with Markus Wanner quite useful in this light. The current situation is that I believe the patch to implement the same “template” model as the on-disk extensions, down to dependency tracking. IIRC I left only one differing behavior, which is that you're not allowed to DROP an Extension Template when it's needed for a dump and restore cycle, where you could be doing that at the file system level of course (and pg_restore on a new system would depend on other files). Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Easily reading debug_print_plan
Craig Ringer cr...@2ndquadrant.com writes: That's what I'm currently doing, I just wanted something that makes it quicker and easier. Jumping around the tree is good, but easy collapse/expand would be much better. As I'm using psql under an Emacs M-x shell buffer, I wanted to experiment with your problem here, so that fellow PostgreSQL hackers using Emacs too can benefit already. I'm sure most already have some setup for that, but maybe not. Using the external package hide-region as documented at the following place solved it: http://www.emacswiki.org/emacs/HideRegion Now I can easily collapse and expand regions directly from within the M-x shell psql buffer, no copy paste, no effort needed. When the point is on an opening block { it will do the right thing, if I want to hide something more detailed I can select a region then hide it. Of course using the default following tool makes things really easier as a single key stroke will default to selecting the right region, and another one will expand the selection to the next logical block. C-M-SPC runs the command mark-sexp Oh, and you can define the region using your mouse too. As often, when searching for text based interactive manipulation tooling, the best I could find is the one I'm already using, Emacs ;-) Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pre-commit triggers
Andrew Dunstan and...@dunslane.net writes: Perhaps my understanding of when XIDs are acquired is insufficient. When exactly is it optional? My understanding of Noah's comment is that we would be exposing what used to be an optimisation only implementation detail to the user, and so we would need to properly document the current situation and would probably be forbidden to change it in the future. Then I guess it's back to the use cases: do we have use cases where it would be interesting for the pre-commit trigger to only get fired when an XID has been consumed? I don't think so, because IIRC CREATE TEMP TABLE will consume an XID even in an otherwise read-only transaction, and maybe the TEMP TABLE writes will not be considered actual writes by the confused user. What about specifying what notion of data modifying transactions you're interested into and providing an SQL callable C function that the trigger user might then use, or even a new WHEN clause? Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Suggestion: Issue warning when calling SET TRANSACTION outside transaction block
Andres Freund and...@2ndquadrant.com writes: On 2013-11-19 13:09:16 -0500, Bruce Momjian wrote: Because as Tom stated, we already do warnings for other useless transaction commands like BEGIN WORK inside a transaction block: Which imo is a bad, bad historical accident. I've repeatedly seen this hide bugs causing corrupted data in the end. +1 -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Review: pre-commit triggers
Andrew Dunstan and...@dunslane.net writes: Given that, do we want to keep the bar on these operating in single user mode? I can easily drop it and just document this way out of difficulty. Currently Event Triggers are disabled in single user mode, in parts because operating them require accessing to a catalog index, which might be corrupted and the reason why you're in single user mode in the first place. So please keep your new event trigger disabled in single user mode. http://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=cd3413ec3683918c9cb9cfb39ae5b2c32f231e8b Regards, and thanks for this new Event Trigger! -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] additional json functionality
David E. Wheeler da...@justatheory.com writes: You know that both types support scalar values right? 'a'::JSON works now, and 'a'::hstore works with the WIP patch. For that reason I would not think that doc or obj would be good choices. I'm wondering about just pushing hstore in core (even if technically still an extension, install it by default, like we do for PLpgSQL), and calling it a day. If you need pre-9.4 JSON-is-text compatibility, use the json datatype, if you want something with general index support, use hstore. For bikeshedding purposes, what about calling it jstore, as in “we actually know how to store your json documents”? Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] additional json functionality
David E. Wheeler da...@justatheory.com writes: On Nov 17, 2013, at 1:51 PM, Dimitri Fontaine dimi...@2ndquadrant.fr wrote: I'm wondering about just pushing hstore in core (even if technically still an extension, install it by default, like we do for PLpgSQL), and calling it a day. It’s syntax is different than JSON, so one would need to convert to and from JSON all the time to parse and serialize. PITA. Oh I misremembered about that, I though it would take JSON as input as-is and could be made to output JSON. And IIRC the community input at pgconf.eu has been to just always output json texts and get rid of the formating GUCs. Now, if it turns out that the new hstore is not dealing with json input and output, we could have json, jstore and hstore. -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] additional json functionality
Andrew Dunstan and...@dunslane.net writes: That would be one of the silliest and most short-sighted decisions we have made in many years, IMNSHO. The demand for strong JSON support is enormous. One of the silliest and most short-sighted decisions we made recently might have been to actually ship that json variant in 9.2, after all. The most popular PostgreSQL commands in 9.4 are going to be: $ sudo apt-get install postgresql-contrib-9.4 # create extension jstore; # alter table foo alter column documents type jstore; # create index on foo using gist(documents); Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Proof of concept: standalone backend with full FE/BE protocol
Andres Freund and...@2ndquadrant.com writes: I think fixing single user mode to work halfway reasonable is enough justification for the feature. Having to deal with that when solving critical issues is just embarassing. +1 But: I very, very much agree with the other concerns around this. This should be a patch to fix single user mode, not one to make postgres into a single process database. It's not, and trying to make it by using single user mode for it will start to hinder development of normal postgres because we suddenly need to be concerned about performance and features in situations where we previously weren't. +1 Maybe what needs to happen to this patch is away to restrict its usage to --single. I'm thinking that postgres --single maybe could be made to fork the server process underneath the psql controler client process transparently. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Add transforms feature
Hi, Peter Eisentraut pete...@gmx.net writes: Rebased patch. No changes except that merge conflicts were resolved, and I had to add some Data::Dumper tweaks to the regression tests so that the results came out in consistent order on different versions of Perl. I just spent some time reading that patch, here's my first round of review: - Documentation style seems to be to be different from the man page or reference docs style that we use elsewhere, and is instead deriving the general case from examples. Reads strange. - The internal datatype argument and return type discussion for function argument looks misplaced, but I don't have a better proposition for that. - The code looks good, I didn't spot any problem when reading it. Given your Jenkins installation I didn't try (yet at least) to use the patch and compile and run it locally. Interesting note might be the addition of two new system caches, one for the PL languages and another one for the transform functions. I agree with the need, just wanted to raise awereness about that in case there's something to be said on that front. - Do we need an ALTER TRANSFORM command? Usually we have at least an Owner for the new objects and a command to change the owner. Then should we be able to change the function(s) used in a transform? - Should transform live in a schema? At first sight, no reason why, but see next point about a use case that we might be able to solve doing that. - SQL Standard has something different named the same thing, targetting client side types apparently. Is there any reason why we would want to stay away from using the same name for something really different in PostgreSQL? On the higher level design, the big question here is about selective behavior. As soon as you CREATE TRANSFORM FOR hstore LANGUAGE plperl then any plperl function will now receive its hstore arguments as a proper perl hash rather than a string. Any pre-existing plperl function with hstore arguments or return type then needs to be upgraded to handle the new types nicely, and some of those might not be under the direct control of the DBA running the CREATE TRANSFORM command, when using some plperl extensions for example. A mechanism allowing for the transform to only be used in some functions but not others might be useful. The simplest such mechanism I can think of is modeled against the PL/Java classpath facility as specified in the SQL standard: you attach a classpath per schema. We could design transforms to only activate in the schema they are created, thus allowing plperl functions to co-exist where some will receive proper hash for hstores and other will continue to get plain text. Should using the schema to that ends be frowned upon, then we need a way to register each plperl function against using or not using the transform facility, defaulting to not using anything. Maybe something like the following: CREATE FUNCTION foo(hash hstore, x ltree) RETURNS hstore LANGUAGE plperl USING TRANSFORM FOR hstore, ltree AS $$ … $$; Worst case, that I really don't think we need, would be addressing that per-argument: CREATE FUNCTION foo (hash hstore WITH TRANSFORM, kv hstore) … I certainly hope we don't need that, and sure can't imagine use cases for that level of complexity at the time of writing this review. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] LISTEN / NOTIFY enhancement request for Postgresql
Bruce Momjian br...@momjian.us writes: • is used to separate names in a path • * is used to match any name in a path • is used to recursively match any destination starting from this name For example using the example above, these subscriptions are possible Subscription Meaning PRICE. Any price for any product on any exchange PRICE.STOCK.Any price for a stock on any exchange PRICE.STOCK.NASDAQ.* Any stock price on NASDAQ PRICE.STOCK.*.IBMAny IBM stock price on any exchange My request is to implement the same or similar feature in Postgresql. This does seem useful and pretty easy to implement. Should we add a TODO? I think we should consider the ltree syntax in that case, as documented in the following link: http://www.postgresql.org/docs/9.3/interactive/ltree.html Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extension Templates S03E11
Stephen Frost sfr...@snowman.net writes: Are there any other changes you have pending for this..? Would be nice to see the latest version which you've tested and which patches cleanly against master... ;) I just rebased now, please see attached. I had to pick new OIDs in some places too, but that's about it. I'll still go ahead and start looking through this, per our discussion. Thanks! -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support templates.v16.patch.gz Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Save Hash Indexes
Hi, Here's an idea: when a user ask for an Hash Index transparently build a BTree index over an hash function instead. Advantages: - it works - it's crash safe - it's (much?) faster than a hash index anyways Drawbacks: - root access concurrency - we need a hash_any function stable against pg_upgrade After talking about it with Heikki, we don't seem to find ways in which the root access concurrency pattern would be visible enough to matter. Also, talking with Peter Geoghegan, it's unclear that there's a use case where a hash index would be faster than a btree index over the hash function. Comments? -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Save Hash Indexes
Tom Lane t...@sss.pgh.pa.us writes: -1. If someone asks for a hash index, they should get a hash index. If you feel the documentation isn't sufficiently clear about the problems involved, we can work on that. Fair enough. Lastly: what real-world problem are we solving by kicking that code to the curb? Well how long did we wait for the fix already? The lack of crash safety and replication ability is not just a drawback, it's about killing the user adoption. Maybe we should implement UNLOGGED indexes and then Hash Indexes would only exist in the UNLOGGED fashion? But well, yeah ok, my idea is a non-starter. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Add min and max execute statement time in pg_stat_statement
Tom Lane t...@sss.pgh.pa.us writes: Hm. It's been a long time since college statistics, but doesn't the entire concept of standard deviation depend on the assumption that the underlying distribution is more-or-less normal (Gaussian)? Is there a I just had a quick chat with a statistician friends of mine on that topic, and it seems that the only way to make sense of an average is if you know already the distribution. In our case, what I keep experiencing with tuning queries is that we have like 99% of them running under acceptable threshold and 1% of them taking more and more time. In a normal (Gaussian) distribution, there would be no query time farther away from the average than any other, so my experience tells me that the query time distribution is anything BUT normal (Gaussian). good reason to suppose that query runtime is Gaussian? (I'd bet not; in particular, multimodal behavior seems very likely due to things like plan changes.) If not, how much does that affect the usefulness of a standard-deviation calculation? I don't know what multi-modal is. What I've been gathering from my quick chat this morning is that either you know how to characterize the distribution and then the min max and average are useful on their own, or you need to keep track of an histogram where all the bins are of the same size to be able to learn what the distribution actually is. We didn't get to the point where I could understand if storing histogram with a constant size on log10 of the data rather than the data itself is going to allow us to properly characterize the distribution. The main question I want to answer here would be the percentiles one, I want to get the query max execution timing for 95% of the executions, then 99%, then 99.9% etc. There's no way to answer that without knowing the distribution shape, so we need enough stats to learn what the distribution shape is (hence, histograms). Of course keeping enough stats seems to always begin with keeping the min, max and average, so we can just begin there. We would just be unable to answer interesting questions with just that. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Location for external scripts for Extensions?
Josh Berkus j...@agliodbs.com writes: pg_partman has several external (python) scripts which help the extension, located in /extras/ in its source. The problem currently is that if you install pg_partman via pgxn or package, you don't get those scripts, because there's no install location for them. See also my proposal to solve that, I'd welcome some design level discussions about it: http://www.postgresql.org/message-id/m28uyzgof3@2ndquadrant.fr Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Cube extension point support // GSoC'13
Tom Lane t...@sss.pgh.pa.us writes: I believe the reason GIST has compress/decompress functions is not for TOAST (they predate that, if memory serves), but to allow the on-disk representation of an index entry to be different from the data type's normal representation in other ways --- think lossy storage in particular. My understanding of the use case for those functions is to do with storing a different data type in the index upper nodes and in the index leafs. It should be possible to do that in a non-lossy way, so that you would implement compress/decompress and not declare the RECHECK bits. Then again I'm talking from 8.3 era memories of when I tried to understand GiST enough to code the prefix extension. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Statistics collection for CLUSTER command
Noah Misch n...@leadboat.com writes: wrapper function. A backend change that would help here is to extend event triggers to cover the CLUSTER command, permitting you to inject monitoring after plain CLUSTER and dispense with the wrapper. I didn't look in any level of details, but it might be as simple as moving the T_ClusterStmt case from standard_ProcessUtility() down to the Event Trigger friendly part known as ProcessUtilitySlow(). Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Auto-tuning work_mem and maintenance_work_mem
MauMau maumau...@gmail.com writes: I understand this problem occurs only when the user configured the application server to use distributed transactions, the application server crashed between prepare and commit/rollback, and the user doesn't recover the application server. So only improper operation produces the problem. The reason why that parameter default has changed from 5 to 0 is that some people would mistakenly use a prepared transaction without a transaction manager. Few only people are actually using a transaction manager that it's better to have them have to set PostgreSQL. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers