Re: [HACKERS] deparsing utility commands

2015-04-28 Thread Dimitri Fontaine
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

2014-11-20 Thread Dimitri Fontaine
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

2014-11-19 Thread Dimitri Fontaine
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

2014-11-18 Thread Dimitri Fontaine
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

2014-11-07 Thread Dimitri Fontaine
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

2014-10-16 Thread Dimitri Fontaine
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

2014-10-14 Thread Dimitri Fontaine
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

2014-10-03 Thread Dimitri Fontaine
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

2014-10-02 Thread Dimitri Fontaine
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

2014-10-02 Thread Dimitri Fontaine
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

2014-03-10 Thread Dimitri Fontaine
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

2014-03-07 Thread Dimitri Fontaine
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

2014-02-28 Thread Dimitri Fontaine
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

2014-02-28 Thread Dimitri Fontaine
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

2014-02-28 Thread Dimitri Fontaine
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

2014-02-27 Thread Dimitri Fontaine
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

2014-02-26 Thread Dimitri Fontaine
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

2014-02-26 Thread Dimitri Fontaine
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

2014-02-26 Thread Dimitri Fontaine
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

2014-02-26 Thread Dimitri Fontaine
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

2014-02-26 Thread Dimitri Fontaine
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

2014-01-30 Thread Dimitri Fontaine
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

2014-01-27 Thread Dimitri Fontaine
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

2014-01-25 Thread Dimitri Fontaine
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

2014-01-24 Thread Dimitri Fontaine
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

2014-01-14 Thread Dimitri Fontaine
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

2014-01-14 Thread Dimitri Fontaine
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

2014-01-07 Thread Dimitri Fontaine
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

2014-01-02 Thread Dimitri Fontaine
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

2014-01-01 Thread Dimitri Fontaine
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

2013-12-21 Thread Dimitri Fontaine
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)

2013-12-18 Thread Dimitri Fontaine
, 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

2013-12-18 Thread Dimitri Fontaine
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

2013-12-17 Thread Dimitri Fontaine
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

2013-12-17 Thread Dimitri Fontaine
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

2013-12-13 Thread Dimitri Fontaine
 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

2013-12-12 Thread Dimitri Fontaine
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

2013-12-11 Thread Dimitri Fontaine
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

2013-12-11 Thread Dimitri Fontaine
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

2013-12-11 Thread Dimitri Fontaine
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

2013-12-09 Thread Dimitri Fontaine
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

2013-12-06 Thread Dimitri Fontaine
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

2013-12-03 Thread Dimitri Fontaine
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

2013-12-03 Thread Dimitri Fontaine
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

2013-12-03 Thread Dimitri Fontaine
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

2013-12-03 Thread Dimitri Fontaine
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

2013-12-03 Thread Dimitri Fontaine
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

2013-12-02 Thread Dimitri Fontaine
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

2013-12-02 Thread Dimitri Fontaine
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

2013-12-02 Thread Dimitri Fontaine
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

2013-12-02 Thread Dimitri Fontaine
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

2013-12-02 Thread Dimitri Fontaine
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

2013-12-02 Thread Dimitri Fontaine
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

2013-12-02 Thread Dimitri Fontaine
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

2013-12-02 Thread Dimitri Fontaine
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

2013-12-02 Thread Dimitri Fontaine
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

2013-12-02 Thread Dimitri Fontaine
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

2013-12-02 Thread Dimitri Fontaine
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

2013-12-02 Thread Dimitri Fontaine
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

2013-12-02 Thread Dimitri Fontaine
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

2013-12-02 Thread Dimitri Fontaine
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

2013-12-01 Thread Dimitri Fontaine
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

2013-12-01 Thread Dimitri Fontaine
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

2013-11-30 Thread Dimitri Fontaine
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

2013-11-30 Thread Dimitri Fontaine
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

2013-11-30 Thread Dimitri Fontaine
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

2013-11-30 Thread Dimitri Fontaine
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

2013-11-28 Thread Dimitri Fontaine
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

2013-11-27 Thread Dimitri Fontaine
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

2013-11-27 Thread Dimitri Fontaine
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

2013-11-27 Thread Dimitri Fontaine
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

2013-11-27 Thread Dimitri Fontaine
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

2013-11-26 Thread Dimitri Fontaine
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

2013-11-26 Thread Dimitri Fontaine
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

2013-11-26 Thread Dimitri Fontaine
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

2013-11-26 Thread Dimitri Fontaine
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

2013-11-25 Thread Dimitri Fontaine
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

2013-11-25 Thread Dimitri Fontaine
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

2013-11-25 Thread Dimitri Fontaine
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

2013-11-25 Thread Dimitri Fontaine
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

2013-11-24 Thread Dimitri Fontaine
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

2013-11-23 Thread Dimitri Fontaine
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

2013-11-20 Thread Dimitri Fontaine
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

2013-11-19 Thread Dimitri Fontaine
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

2013-11-19 Thread Dimitri Fontaine
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

2013-11-18 Thread Dimitri Fontaine
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

2013-11-17 Thread Dimitri Fontaine
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

2013-11-17 Thread Dimitri Fontaine
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

2013-11-17 Thread Dimitri Fontaine
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

2013-11-15 Thread Dimitri Fontaine
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

2013-11-15 Thread Dimitri Fontaine
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

2013-11-14 Thread Dimitri Fontaine
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

2013-11-12 Thread Dimitri Fontaine
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

2013-11-01 Thread Dimitri Fontaine
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

2013-11-01 Thread Dimitri Fontaine
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

2013-10-22 Thread Dimitri Fontaine
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?

2013-10-22 Thread Dimitri Fontaine
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

2013-10-21 Thread Dimitri Fontaine
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

2013-10-20 Thread Dimitri Fontaine
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

2013-10-13 Thread Dimitri Fontaine
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


  1   2   3   4   5   6   7   8   9   10   >