Re: [HACKERS] deparsing utility commands

2015-08-20 Thread Alvaro Herrera
Shulgin, Oleksandr wrote: > A particularly nasty one is: > > ERROR: index "cwi_replaced_pkey" does not exist > > The test statement that's causing it is this one: > > ALTER TABLE cwi_test DROP CONSTRAINT cwi_uniq_idx, > ADD CONSTRAINT cwi_replaced_pkey PRIMARY KEY > USING INDEX cwi_uniq2_idx;

Re: [HACKERS] deparsing utility commands

2015-08-20 Thread Alvaro Herrera
Shulgin, Oleksandr wrote: > Another quirk of ALTER TABLE is that due to multi-pass processing in > ATRewriteCatalogs, the same command might be collected a number of times. > For example, in src/test/regress/sql/inherit.sql: > > alter table a alter column aa type integer using bit_length(aa); >

Re: [HACKERS] deparsing utility commands

2015-08-20 Thread Shulgin, Oleksandr
On Thu, Aug 20, 2015 at 4:28 PM, Shulgin, Oleksandr < oleksandr.shul...@zalando.de> wrote: > Which gets deparsed as: > > ALTER TABLE cwi_test DROP CONSTRAINT cwi_uniq_idx, > ADD CONSTRAINT cwi_replaced_pkey PRIMARY KEY > USING INDEX cwi_replaced_pkey; > > The problem is that during constraint tra

Re: [HACKERS] deparsing utility commands

2015-08-06 Thread Alvaro Herrera
Jim Nasby wrote: > FWIW, my interest in this is largely due to the problems I've had with this > in the variant type. In particular, using the same resolution rules for > functions and operators. So I'm wondering if there's a bigger issue here. I'd be glad to review your variant stuff, but I doub

Re: [HACKERS] deparsing utility commands

2015-08-06 Thread Jim Nasby
On 8/5/15 9:55 PM, Alvaro Herrera wrote: Jim Nasby wrote: On 7/31/15 8:45 AM, Shulgin, Oleksandr wrote: STATEMENT: create view v1 as select * from t1 ; ERROR: operator does not exist: pg_catalog.oid = pg_catalog.oid at character 52 HINT: No operator matches the given name and argument type

Re: [HACKERS] deparsing utility commands

2015-08-05 Thread Alvaro Herrera
Jim Nasby wrote: > On 7/31/15 8:45 AM, Shulgin, Oleksandr wrote: > >STATEMENT: create view v1 as select * from t1 ; > >ERROR: operator does not exist: pg_catalog.oid = pg_catalog.oid at > >character 52 > >HINT: No operator matches the given name and argument type(s). You > >might need to add ex

Re: [HACKERS] deparsing utility commands

2015-08-05 Thread Jim Nasby
On 7/31/15 8:45 AM, Shulgin, Oleksandr wrote: While running deparsecheck suite I'm getting a number of oddly looking errors: WARNING: state: 42883 errm: operator does not exist: pg_catalog.oid = pg_catalog.oid This is caused by deparsing create view, e.g.: STATEMENT: create view v1 as select

Re: [HACKERS] deparsing utility commands

2015-07-31 Thread Shulgin, Oleksandr
On Wed, Jul 29, 2015 at 12:44 PM, Shulgin, Oleksandr < oleksandr.shul...@zalando.de> wrote: > On Tue, May 12, 2015 at 12:25 AM, Alvaro Herrera > wrote: > >> David Steele wrote: >> >> > I have reviewed and tested this patch and everything looks good to me. >> > It also looks like you added better

Re: [HACKERS] deparsing utility commands

2015-07-29 Thread Shulgin, Oleksandr
On Tue, May 12, 2015 at 12:25 AM, Alvaro Herrera wrote: > David Steele wrote: > > > I have reviewed and tested this patch and everything looks good to me. > > It also looks like you added better coverage for schema DDL, which is a > > welcome addition. > > Thanks -- I have pushed this now. > Hi,

Re: [HACKERS] deparsing utility commands

2015-05-11 Thread Alvaro Herrera
David Steele wrote: > I have reviewed and tested this patch and everything looks good to me. > It also looks like you added better coverage for schema DDL, which is a > welcome addition. Thanks -- I have pushed this now. My hope is to get this test module extended quite a bit, not only to cover

Re: [HACKERS] deparsing utility commands

2015-05-11 Thread David Steele
On 5/8/15 3:29 PM, Alvaro Herrera wrote: >> Here is a complete version. Barring serious problems, I intend to >> commit this on Monday. I have reviewed and tested this patch and everything looks good to me. It also looks like you added better coverage for schema DDL, which is a welcome addition.

Re: [HACKERS] deparsing utility commands

2015-05-08 Thread Alvaro Herrera
Alvaro Herrera wrote: > Alvaro Herrera wrote: > > Here's a cleaned up version of this patch; I threw together a very quick > > test module, and updated a conflicting OID. As far as I can tell, I'm > > only missing the documentation updates before this is push-able. > > Here is a complete version.

Re: [HACKERS] deparsing utility commands

2015-05-08 Thread Fabrízio de Royes Mello
On Fri, May 8, 2015 at 3:14 PM, Alvaro Herrera wrote: > > Alvaro Herrera wrote: > > Here's a cleaned up version of this patch; I threw together a very quick > > test module, and updated a conflicting OID. As far as I can tell, I'm > > only missing the documentation updates before this is push-abl

Re: [HACKERS] deparsing utility commands

2015-05-08 Thread Alvaro Herrera
Alvaro Herrera wrote: > Here's a cleaned up version of this patch; I threw together a very quick > test module, and updated a conflicting OID. As far as I can tell, I'm > only missing the documentation updates before this is push-able. Here is a complete version. Barring serious problems, I inte

Re: [HACKERS] deparsing utility commands

2015-05-04 Thread Alvaro Herrera
Here's a cleaned up version of this patch; I threw together a very quick test module, and updated a conflicting OID. As far as I can tell, I'm only missing the documentation updates before this is push-able. One change to note is that the AlterTable support used to ignore commands that didn't mat

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 the

Re: [HACKERS] deparsing utility commands

2015-04-18 Thread Amit Kapila
On Thu, Apr 9, 2015 at 9:44 PM, Alvaro Herrera wrote: > > Alvaro Herrera wrote: > > Executive summary: > > > > There is now a CommandDeparse_hook; > > deparse_utility_command is provided as an extension, intended for 9.6; > > rest of patch would be pushed to 9.5. > > Actually here's another approa

Re: [HACKERS] deparsing utility commands

2015-04-14 Thread David Steele
On 4/9/15 12:14 PM, Alvaro Herrera wrote: > Alvaro Herrera wrote: >> Executive summary: >> >> There is now a CommandDeparse_hook; >> deparse_utility_command is provided as an extension, intended for 9.6; >> rest of patch would be pushed to 9.5. > > Actually here's another approach I like better: u

Re: [HACKERS] deparsing utility commands

2015-04-08 Thread David Steele
On 4/7/15 4:32 PM, Alvaro Herrera wrote: > Executive summary: > > There is now a CommandDeparse_hook; > deparse_utility_command is provided as an extension, intended for 9.6; > rest of patch would be pushed to 9.5. > > > Long version: > > I've made command deparsing hookable. Attached there ar

Re: [HACKERS] deparsing utility commands

2015-04-07 Thread Alvaro Herrera
Executive summary: There is now a CommandDeparse_hook; deparse_utility_command is provided as an extension, intended for 9.6; rest of patch would be pushed to 9.5. Long version: I've made command deparsing hookable. Attached there are three patches: the first patch contains changes to core tha

Re: [HACKERS] deparsing utility commands

2015-04-02 Thread Andres Freund
On 2015-04-02 12:05:13 -0400, Robert Haas wrote: > On Wed, Mar 25, 2015 at 3:21 PM, Ryan Pedela wrote: > > On Wed, Mar 25, 2015 at 11:59 AM, Alvaro Herrera > > wrote: > >> * Should we prohibit DDL from within event triggers? > > > > Please don't prohibit DDL unless there is a really, really good

Re: [HACKERS] deparsing utility commands

2015-04-02 Thread Robert Haas
On Wed, Mar 25, 2015 at 3:21 PM, Ryan Pedela wrote: > On Wed, Mar 25, 2015 at 11:59 AM, Alvaro Herrera > wrote: >> * Should we prohibit DDL from within event triggers? > > Please don't prohibit DDL unless there is a really, really good reason to do > so. I have several use cases in mind for event

Re: [HACKERS] deparsing utility commands

2015-03-25 Thread Alvaro Herrera
Alvaro Herrera wrote: > Here's an updated version of this series. I just pushed patches 0001 and 0002, with very small tweaks; those had already been reviewed and it didn't seem like there was much controversy. To test the posted series it's probably easiest to git checkout b3196e65f5bfc9

Re: [HACKERS] deparsing utility commands

2015-03-25 Thread Ryan Pedela
On Wed, Mar 25, 2015 at 11:59 AM, Alvaro Herrera wrote: > > > * Should we prohibit DDL from within event triggers? > Please don't prohibit DDL unless there is a really, really good reason to do so. I have several use cases in mind for event triggers, but they are only useful if I can perform DDL.

Re: [HACKERS] deparsing utility commands

2015-03-18 Thread Stephen Frost
* Alvaro Herrera (alvhe...@2ndquadrant.com) wrote: > One thing that Stephen commented on was the ALTER TABLE preparatory > patch. He said why not return ObjectAddress from the subcommand > routines instead of just Oid/attnum? The reason is that to interpret > the address correctly you will still

Re: [HACKERS] deparsing utility commands

2015-03-05 Thread Stephen Frost
Alvaro, * Alvaro Herrera (alvhe...@2ndquadrant.com) wrote: > Thanks for the review. I have pushed these patches, squashed in one > commit, with the exception of the one that changed ALTER TABLE. You had > enough comments about that one that I decided to put it aside for now, > and get the other

Re: [HACKERS] deparsing utility commands

2015-03-03 Thread Alvaro Herrera
Stephen, Thanks for the review. I have pushed these patches, squashed in one commit, with the exception of the one that changed ALTER TABLE. You had enough comments about that one that I decided to put it aside for now, and get the other ones done. I will post a new series shortly. I fixed (mo

Re: [HACKERS] deparsing utility commands

2015-03-02 Thread Andres Freund
Hi, On 2015-02-15 01:48:15 -0300, Alvaro Herrera wrote: > This is a repost of the patch to add CREATE command deparsing support to > event triggers. It now supports not only CREATE but also ALTER and > other commands such as GRANT/REVOKE, COMMENT ON and SECURITY LABEL. > This patch series is brok

Re: [HACKERS] deparsing utility commands

2015-02-27 Thread Stephen Frost
Alvaro, * Alvaro Herrera (alvhe...@2ndquadrant.com) wrote: > Thanks. I have adjusted the code to use ObjectAddress in all functions > called by ProcessUtilitySlow; the patch is a bit tedious but seems > pretty reasonable to me. As I said, there is quite a lot of churn. Thanks for doing this! >

Re: [HACKERS] deparsing utility commands

2015-02-24 Thread Stephen Frost
* Alvaro Herrera (alvhe...@2ndquadrant.com) wrote: > Stephen Frost wrote: > > > Regardless, as I've tried to point out above, the > > changes which I'm actually suggesting for this initial body of work are > > just to avoid the parsetree and go based off of what the catalog has. > > I'm hopeful th

Re: [HACKERS] deparsing utility commands

2015-02-24 Thread Ryan Pedela
On Tue, Feb 24, 2015 at 6:48 AM, Alvaro Herrera wrote: > Stephen Frost wrote: > > I'm thinking something like > SELECT * FROM pg_creation_commands({'pg_class'::regclass, > 'sometable'::pg_class}); > would return a set of commands in the JSON-blob format that creates the > table. The input argum

Re: [HACKERS] deparsing utility commands

2015-02-24 Thread Andres Freund
On 2015-02-24 10:48:38 -0300, Alvaro Herrera wrote: > Stephen Frost wrote: > > > Regardless, as I've tried to point out above, the > > changes which I'm actually suggesting for this initial body of work are > > just to avoid the parsetree and go based off of what the catalog has. > > I'm hopeful t

Re: [HACKERS] deparsing utility commands

2015-02-24 Thread Alvaro Herrera
Stephen Frost wrote: > Regardless, as I've tried to point out above, the > changes which I'm actually suggesting for this initial body of work are > just to avoid the parsetree and go based off of what the catalog has. > I'm hopeful that's a small enough and reasonable enough change to happen > du

Re: [HACKERS] deparsing utility commands

2015-02-24 Thread Stephen Frost
* Andres Freund (and...@2ndquadrant.com) wrote: > Hi, > > On 2015-02-23 19:48:43 -0500, Stephen Frost wrote: > > > Yes, it might be possible to use the same code for a bunch of minor > > > commands, but not for the interesting/complex stuff. > > > > We can clearly rebuild at least CREATE commands

Re: [HACKERS] deparsing utility commands

2015-02-23 Thread Andres Freund
Hi, On 2015-02-23 19:48:43 -0500, Stephen Frost wrote: > > Yes, it might be possible to use the same code for a bunch of minor > > commands, but not for the interesting/complex stuff. > > We can clearly rebuild at least CREATE commands for all objects without > access to the parse tree, obviously

Re: [HACKERS] deparsing utility commands

2015-02-23 Thread Stephen Frost
Andres, * Andres Freund (and...@2ndquadrant.com) wrote: > On 2015-02-21 14:51:32 -0500, Stephen Frost wrote: > > It'd be *really* nice to be able to pass an object identifier to some > > function and get back the CREATE (in particular, though perhaps DROP, or > > whatever) command for it. This ge

Re: [HACKERS] deparsing utility commands

2015-02-22 Thread Andres Freund
On 2015-02-21 17:30:24 +0100, Andres Freund wrote: > > /* > > + * deparse_CreateFunctionStmt > > + * Deparse a CreateFunctionStmt (CREATE FUNCTION) > > + * > > + * Given a function OID and the parsetree that created it, return the JSON > > + * blob representing the creation command. > > +

Re: [HACKERS] deparsing utility commands

2015-02-21 Thread Andres Freund
On 2015-02-21 14:51:32 -0500, Stephen Frost wrote: > It'd be *really* nice to be able to pass an object identifier to some > function and get back the CREATE (in particular, though perhaps DROP, or > whatever) command for it. This gets us *awful* close to that without > actually giving it to us an

Re: [HACKERS] deparsing utility commands

2015-02-21 Thread Stephen Frost
Alvaro, all, * Alvaro Herrera (alvhe...@2ndquadrant.com) wrote: > This is a repost of the patch to add CREATE command deparsing support to > event triggers. It now supports not only CREATE but also ALTER and > other commands such as GRANT/REVOKE, COMMENT ON and SECURITY LABEL. > This patch series

Re: [HACKERS] deparsing utility commands

2015-02-21 Thread Andres Freund
Hi, On 2015-02-15 01:48:15 -0300, Alvaro Herrera wrote: > One line of defense which I just tought about is that instead of > sprinkling EventTriggerStashCommand() calls all over ProcessUtilitySlow, > we should add only one at the bottom. Doesn't sound like a bad idea, but I'm not sure whether it'

Re: [HACKERS] deparsing utility commands

2015-02-21 Thread Andres Freund
On 2015-02-19 14:39:27 -0300, Alvaro Herrera wrote: > diff --git a/src/backend/catalog/objectaddress.c > b/src/backend/catalog/objectaddress.c > index d899dd7..2bbc15d 100644 > --- a/src/backend/catalog/objectaddress.c > +++ b/src/backend/catalog/objectaddress.c > @@ -531,6 +531,12 @@ ObjectTypeMa

Re: [HACKERS] deparsing utility commands

2015-02-21 Thread Andres Freund
On 2015-02-19 17:14:57 -0300, Alvaro Herrera wrote: > The ddl_command_start event occurs just before the execution of > a CREATE, ALTER, DROP, SECURITY LABEL, COMMENT, GRANT or REVOKE > command. No check whether the affected object exists or doesn't > exist is performed befo

Re: [HACKERS] deparsing utility commands

2015-02-19 Thread Alvaro Herrera
Stephen Frost wrote: > > > > Yes, I will push these unless somebody objects soon, as they seem > > > > perfectly reasonable to me. The only troubling thing is that there is > > > > no regression test for this kind of thing in event triggers (i.e. verify > > > > which command tags get support and

Re: [HACKERS] deparsing utility commands

2015-02-19 Thread Alvaro Herrera
Stephen Frost wrote: > * Alvaro Herrera (alvhe...@2ndquadrant.com) wrote: > > > > Now, we probably don't want to hack *all* the utility commands to return > > > > ObjectAddress instead of OID, because it many cases that's just not > > > > going to be convenient (not to speak of the code churn); so

Re: [HACKERS] deparsing utility commands

2015-02-18 Thread Stephen Frost
* Alvaro Herrera (alvhe...@2ndquadrant.com) wrote: > > > Now, we probably don't want to hack *all* the utility commands to return > > > ObjectAddress instead of OID, because it many cases that's just not > > > going to be convenient (not to speak of the code churn); so I think for > > > most objtyp

Re: [HACKERS] deparsing utility commands

2015-02-18 Thread Alvaro Herrera
> > Now, we probably don't want to hack *all* the utility commands to return > > ObjectAddress instead of OID, because it many cases that's just not > > going to be convenient (not to speak of the code churn); so I think for > > most objtypes the ProcessUtilitySlow stanza would look like this: > T

Re: [HACKERS] deparsing utility commands

2015-02-18 Thread Stephen Frost
Alvaro, * Alvaro Herrera (alvhe...@2ndquadrant.com) wrote: > Stephen Frost wrote: > > * Alvaro Herrera (alvhe...@2ndquadrant.com) wrote: > > > Patch 0002 I think is good to go as well, AFAICT (have the various > > > RENAME commands return the OID and attnum of affected objects). > > > > It's not

Re: [HACKERS] deparsing utility commands

2015-02-18 Thread Alvaro Herrera
Stephen, Stephen Frost wrote: > * Alvaro Herrera (alvhe...@2ndquadrant.com) wrote: > > Patch 0002 I think is good to go as well, AFAICT (have the various > > RENAME commands return the OID and attnum of affected objects). > > It's not a huge complaint, but it feels a bit awkward to me that > Ex

Re: [HACKERS] deparsing utility commands

2015-02-18 Thread Stephen Frost
Alvaro, * Alvaro Herrera (alvhe...@2ndquadrant.com) wrote: > Andres Freund wrote: > > On 2015-02-15 01:48:15 -0300, Alvaro Herrera wrote: > > I think you should just go ahead and commit 0001, 0002, 0006. Those have > > previously been discussed and seem like a good idea and uncontroversial > > eve

Re: [HACKERS] deparsing utility commands

2015-02-18 Thread Alvaro Herrera
Andres Freund wrote: > Hi, > > On 2015-02-15 01:48:15 -0300, Alvaro Herrera wrote: > > This is a repost of the patch to add CREATE command deparsing support to > > event triggers. It now supports not only CREATE but also ALTER and > > other commands such as GRANT/REVOKE, COMMENT ON and SECURITY L

Re: [HACKERS] deparsing utility commands

2015-02-18 Thread Stephen Frost
Alvaro, * Alvaro Herrera (alvhe...@2ndquadrant.com) wrote: > Stephen Frost wrote: > > I've started taking a look at this as the pgaudit bits depend on it and > > noticed that the patch set implies there's 42 patches, but there were > > only 37 attached..? > > Ah, I didn't realize when I posted th

Re: [HACKERS] deparsing utility commands

2015-02-18 Thread Alvaro Herrera
Stephen Frost wrote: > I've started taking a look at this as the pgaudit bits depend on it and > noticed that the patch set implies there's 42 patches, but there were > only 37 attached..? Ah, I didn't realize when I posted that the subject was counting those extra patches. They are later patche

Re: [HACKERS] deparsing utility commands

2015-02-18 Thread Stephen Frost
Alvaro, * Alvaro Herrera (alvhe...@2ndquadrant.com) wrote: > This is a repost of the patch to add CREATE command deparsing support to > event triggers. It now supports not only CREATE but also ALTER and > other commands such as GRANT/REVOKE, COMMENT ON and SECURITY LABEL. > This patch series is b

Re: [HACKERS] deparsing utility commands

2015-02-18 Thread Andres Freund
Hi, On 2015-02-15 01:48:15 -0300, Alvaro Herrera wrote: > This is a repost of the patch to add CREATE command deparsing support to > event triggers. It now supports not only CREATE but also ALTER and > other commands such as GRANT/REVOKE, COMMENT ON and SECURITY LABEL. > This patch series is brok