Robert Haas writes:
> I guess I don't particularly like either of these changes. The first
Fair enough.
> one is mostly harmless, but I don't really see why it's any better,
> and it does have the downside of traversing the string twice (once for
> strlen and a second time in str_toupper) inste
On Mon, Aug 27, 2012 at 7:52 AM, Dimitri Fontaine
wrote:
> I've been reviewing your changes and here's a very small patch with some
> details I would have spelled out differently. See what you think, I
> mostly needed to edit some code to get back in shape :)
I guess I don't particularly like eit
Hi,
I'm back to PostgreSQL development concerns after some distraction here.
First, thanks for pushing the patch to commit!
I've been reviewing your changes and here's a very small patch with some
details I would have spelled out differently. See what you think, I
mostly needed to edit some code
On Fri, Jul 20, 2012 at 3:51 PM, Robert Haas wrote:
> On Fri, Jul 20, 2012 at 2:12 PM, Tom Lane wrote:
>> The Windows buildfarm members don't seem too happy with the latest
>> patch.
>
> I'm looking at this now, but am so far mystified. Something's
> obviously broken as regards how the trigger f
On Fri, Jul 20, 2012 at 1:22 PM, Thom Brown wrote:
> I've just run my own set of tests against these changes, which tests
> every supported DDL command (with the exception of "CREATE USER
> MAPPING", "ALTER USER MAPPING", "DROP USER MAPPING", "CREATE LANGUAGE"
> and "DROP LANGUAGE"), and many vari
On Fri, Jul 20, 2012 at 2:12 PM, Tom Lane wrote:
> The Windows buildfarm members don't seem too happy with the latest
> patch.
I'm looking at this now, but am so far mystified. Something's
obviously broken as regards how the trigger flags get set up, but if
that were broken in a trivial way it w
The Windows buildfarm members don't seem too happy with the latest
patch.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 20 July 2012 16:50, Robert Haas wrote:
> On Wed, Jul 18, 2012 at 10:22 AM, Robert Haas wrote:
>> The next step here is obviously to complete the work necessary to
>> actually be able to fire these triggers, as opposed to just saying
>> that we fire them. I'll write up my thoughts on that topi
On Wed, Jul 18, 2012 at 10:22 AM, Robert Haas wrote:
> The next step here is obviously to complete the work necessary to
> actually be able to fire these triggers, as opposed to just saying
> that we fire them. I'll write up my thoughts on that topic in a
> separate email. I don't think there's
On Thu, Jul 12, 2012 at 8:50 AM, Dimitri Fontaine
wrote:
> [ new patch ]
Well, I think it's about time to start getting some of this code into
our tree. However, I'm still not confident that the code that
actually runs the triggers is entirely solid, so I decided to rip that
stuff out and commit
On Tue, Jul 10, 2012 at 10:38 AM, Dimitri Fontaine
wrote:
>> Mind you, if I ran the world, this would probably be broken up
>> differently: I'd have ddl_command_start covering all the
>> CREATE/ALTER/DROP commands and nothing else; and separate firing
>> points for anything else I wanted to suppor
Thom Brown writes:
> I also attach various typo/grammar fixes.
In fact Robert's cleanup of the docs make that patch of yours not apply
anymore, and I think a part of it is maybe already fixed. Do you have
time to look at this with the new v1.8 patch that you will receive in a
minute, or with the
On Fri, Jul 6, 2012 at 4:00 PM, Dimitri Fontaine wrote:
> Robert Haas writes:
>> And here is another incremental patch, this one doing some more
>> cleanup. Some of this is cosmetic, but it also:
>
> Thanks, applied in my github repository!
Thanks.
>> I have a large remaining maintainability c
On Fri, Jul 6, 2012 at 3:29 PM, Dimitri Fontaine wrote:
>> Here is an incremental documentation patch which I hope you will like.
>
> Definitely, it's better this way. I'm not thrilled with separating it
> into its own top level chapter, but I can see how it makes sense indeed.
Oh, really? I tho
Robert Haas writes:
> And here is another incremental patch, this one doing some more
> cleanup. Some of this is cosmetic, but it also:
Thanks, applied in my github repository!
> I'm feeling pretty good about this at this point, although I think
> there is still some more work to do before we c
On Fri, Jul 6, 2012 at 12:00 PM, Robert Haas wrote:
> On Fri, Jul 6, 2012 at 7:21 AM, Dimitri Fontaine
> wrote:
>> Robert Haas writes:
>>> Attached is a incremental patch with a bunch of minor cleanups,
>>> including reverts of a few spurious white space changes. Could you
>>> merge this into
Robert Haas writes:
> Here is an incremental documentation patch which I hope you will like.
Definitely, it's better this way. I'm not thrilled with separating it
into its own top level chapter, but I can see how it makes sense indeed.
This part is strange though:
+ A trigger definition can
On Thu, Jul 5, 2012 at 5:31 PM, Dimitri Fontaine wrote:
> [ new patch ]
Attached is a incremental patch with a bunch of minor cleanups,
including reverts of a few spurious white space changes. Could you
merge this into your version?
I have some concerns about pg_dump:
1. Can we spell out EvtTr
On Wed, Jul 4, 2012 at 1:56 PM, Dimitri Fontaine wrote:
> Dimitri Fontaine writes:
>> Robert Haas writes:
>>> No, I'm not asking you to add any more columns right now (in fact,
>>> please do not). But the type of the existing column should change to
>>> text[].
>>
>> Ok, done in the attached. W
On Tue, Jul 3, 2012 at 1:31 PM, Dimitri Fontaine wrote:
> Robert Haas writes:
>> On Tue, Jul 3, 2012 at 12:18 PM, Tom Lane wrote:
>>> Robert Haas writes:
Yeah, I'm of two minds on that. I thought that it made sense to use
integer identifiers internally for speed, but now I'm worried
Robert Haas writes:
> On Tue, Jul 3, 2012 at 12:18 PM, Tom Lane wrote:
>> Robert Haas writes:
>>> Yeah, I'm of two minds on that. I thought that it made sense to use
>>> integer identifiers internally for speed, but now I'm worried that the
>>> effort to translate back and forth between strings
On Tue, Jul 3, 2012 at 12:18 PM, Tom Lane wrote:
> Robert Haas writes:
>> Yeah, I'm of two minds on that. I thought that it made sense to use
>> integer identifiers internally for speed, but now I'm worried that the
>> effort to translate back and forth between strings and integers is
>> going t
Robert Haas writes:
> Yeah, I'm of two minds on that. I thought that it made sense to use
> integer identifiers internally for speed, but now I'm worried that the
> effort to translate back and forth between strings and integers is
> going to end up being more than any speed we might save.
We do
On Tue, Jul 3, 2012 at 11:52 AM, Dimitri Fontaine
wrote:
> Robert Haas writes:
>> in it. That's more or less what Dimitri already has in his latest
>> patch, except that after looking it over I'm inclined to think that
>> we'd be better off storing the keys as text and translating to
>> internal
Robert Haas writes:
> in it. That's more or less what Dimitri already has in his latest
> patch, except that after looking it over I'm inclined to think that
> we'd be better off storing the keys as text and translating to
> internal ID numbers when we read and cache the table, rather than
> stor
On Mon, Jul 2, 2012 at 11:25 PM, Tom Lane wrote:
> Robert Haas writes:
>> On Mon, Jul 2, 2012 at 6:59 PM, Tom Lane wrote:
>>> Um, doesn't that require nonrectangular arrays?
>
>> Doh. You're right: I keep forgetting that arrays have to be rectangular.
>
>> Any suggestions on a sensible way to r
Robert Haas writes:
>> Given what I foresee, simply having another columns in there named
>> evtstags with the exact same content as evttags would be the simplest
>> and most natural implementation, really.
>
> That seems a lot less general for no particular gain.
The gain is code, docs and usage
Robert Haas writes:
> On Mon, Jul 2, 2012 at 6:59 PM, Tom Lane wrote:
>> Um, doesn't that require nonrectangular arrays?
> Doh. You're right: I keep forgetting that arrays have to be rectangular.
> Any suggestions on a sensible way to represent this?
Are there likely to be enough entries that
On Mon, Jul 2, 2012 at 6:59 PM, Tom Lane wrote:
> Robert Haas writes:
>> So let's try to hammer something out now. The obvious thing that
>> occurs to me is to have a column in the catalog that is a 2-D array of
>> text, with the first element of each array being something like "tag"
>> or "subt
Dimitri Fontaine writes:
> I don't foresee more generic needs here, unless you can convince me that
> we need both a. a full stack of arbitrarily nested commands and b. a way
> to match and target any level of that stack.
Um ... isn't the burden of proof the other way around here? That is,
what'
Robert Haas writes:
> So let's try to hammer something out now. The obvious thing that
> occurs to me is to have a column in the catalog that is a 2-D array of
> text, with the first element of each array being something like "tag"
> or "subtag" (i.e. event_trigger_variable) and the remaining arr
On Mon, Jul 2, 2012 at 4:53 PM, Dimitri Fontaine wrote:
> Robert Haas writes:
>> So let's try to hammer something out now. The obvious thing that
>> occurs to me is to have a column in the catalog that is a 2-D array of
>> text, with the first element of each array being something like "tag"
>>
Robert Haas writes:
> I'd really like to not have to change the catalog again in every
> patch, because if we do that then we are just saying we're going to
> rewrite this patch completely every time we want to add a new feature,
> which kind of defeats the purpose IMHO.
Fair enough.
> So let's
On Mon, Jul 2, 2012 at 4:10 PM, Dimitri Fontaine wrote:
>> 2. On a related point, do we anticipate that we might eventually want
>> to allow filtering by more than one event_trigger_variable in the same
>> trigger? That is, might we want to do something like this:
>>
>> CREATE EVENT TRIGGER somet
Robert Haas writes:
> CREATE EVENT TRIGGER name ON event_name WHEN event_trigger_variable IN
> (trigger_command_list) EXECUTE PROCEDURE func_name ()
[...]
> 1. Do we imagine a situation where a given event_name would allow more
> than one choice of event_trigger_variable? If so, then
The only us
On Mon, Jul 2, 2012 at 10:11 AM, Dimitri Fontaine
wrote:
> [ new patch ]
I would really like to start committing parts of this, but there are
still a lot of unfinished loose ends in this code. The one that is
most immediately bothering me is related to the syntax you've
implemented, which is:
C
On Mon, Jul 2, 2012 at 1:56 PM, Robert Haas wrote:
> On Fri, Jun 29, 2012 at 5:38 PM, Dimitri Fontaine
> wrote:
>> Will do a whole warning check pass later. Can you give me your local
>> Makefile trick to turn them into hard errors again please? :)
>
> echo COPT=-Werror > src/Makefile.custom
>
>
On Fri, Jun 29, 2012 at 5:38 PM, Dimitri Fontaine
wrote:
> Will do a whole warning check pass later. Can you give me your local
> Makefile trick to turn them into hard errors again please? :)
echo COPT=-Werror > src/Makefile.custom
Your latest patch contains a warning about using a variable
unin
On 2 July 2012 15:11, Dimitri Fontaine wrote:
> Dimitri Fontaine writes:
>> https://github.com/dimitri/postgres/compare/f99e8d93b7...8da156dc70
>
> The revised incremental diff is here:
>
> https://github.com/dimitri/postgres/compare/f99e8d93b7...74bbeda8
>
> And a new revision of the patch i
Robert Haas writes:
> ../../../src/include/catalog/pg_event_trigger.h:34: error: expected
> specifier-qualifier-list before ‘int2’
>
> This needs to be changed to int16 as a result of commit
> b8b2e3b2deeaab19715af063fc009b7c230b2336.
Done as part of the previous work.
> alter.c:73: warning: imp
Hi,
Here's an answer to your review (thanks!), with no patch attached yet
even if I've been cleanup up most of what you reported. Incremental diff
is available for browsing here:
https://github.com/dimitri/postgres/compare/f99e8d93b7...8da156dc70
Robert Haas writes:
> Some of the pg_dump hunk
On Thu, Jun 28, 2012 at 9:46 AM, Robert Haas wrote:
> [ review ]
Also:
../../../src/include/catalog/pg_event_trigger.h:34: error: expected
specifier-qualifier-list before ‘int2’
This needs to be changed to int16 as a result of commit
b8b2e3b2deeaab19715af063fc009b7c230b2336.
alter.c:73: warnin
On Sun, Jun 24, 2012 at 5:46 PM, Dimitri Fontaine
wrote:
> Here's an early revision 2 of the patch, not yet ready for commit, so
> including the PL stuff still. What's missing is mainly a cache reference
> leak to fix at DROP EVENT TRIGGER, but I failed to spot where it comes
> from.
>
> As I fixe
Robert Haas writes:
>> It's not before/after anymore, but rather addon/replace if you will. I
>> kept the INSTEAD OF keyword for the replace semantics, that you've been
>> asking me to keep IIRC, with security policy plugins as a use case.
>>
>> Now we can of course keep those semantics and embed
On Wed, Jun 20, 2012 at 4:36 PM, Dimitri Fontaine
wrote:
> Robert Haas writes:
>> 1. I still think we ought to get rid of the notion of BEFORE or AFTER
>> (i.e. pg_event_trigger.evttype) and just make that detail part of the
>> event name (e.g. pg_event_trigger.evtevent). Many easily forseeable
Robert Haas writes:
>> Cant you just put it alone in a test group in the parallel_schedule? Several
>> other tests do that?
>
> Yeah, that seems like it should work. If not, I'd say the tests
> themselves are broken.
I completely missed that we could do that. I don't feel bright. Of
course it ju
Hi,
Robert Haas writes:
> 1. I still think we ought to get rid of the notion of BEFORE or AFTER
> (i.e. pg_event_trigger.evttype) and just make that detail part of the
> event name (e.g. pg_event_trigger.evtevent). Many easily forseeable
> event types will be more like "during" rather than "befo
On Wed, Jun 20, 2012 at 3:39 PM, Andres Freund wrote:
> On Wednesday, June 20, 2012 09:33:26 PM Dimitri Fontaine wrote:
>> > Hmm, I don't like the idea of a test that only runs in serial mode.
>> > Maybe we can find some way to make it work in parallel mode as well.
>>
>> I don't see how we can ma
On Wednesday, June 20, 2012 09:33:26 PM Dimitri Fontaine wrote:
> > Hmm, I don't like the idea of a test that only runs in serial mode.
> > Maybe we can find some way to make it work in parallel mode as well.
>
> I don't see how we can manage to do that, as adding an event trigger to
> some comman
Alvaro Herrera writes:
> Did you try REASSIGN OWNED and DROP OWNED with a role that has defined
> some event triggers?
Not yet. Will add to regression tests, good catch.
> Hmm, I don't like the idea of a test that only runs in serial mode.
> Maybe we can find some way to make it work in parallel
On Fri, Jun 15, 2012 at 4:27 PM, Dimitri Fontaine
wrote:
> Allow me to open the new season of the DML trigger series, named
> pg_event_trigger. This first episode is all about setting up the drama,
> so that next ones make perfect sense.
Comments:
1. I still think we ought to get rid of the noti
Excerpts from Dimitri Fontaine's message of vie jun 15 16:27:50 -0400 2012:
> The attached patch contains all the infrastructure for event triggers
> and also a first implementation of them for the event "command_start",
> implemented in a single place in utility.c.
>
> The infrastructure is abo
On 15 June 2012 21:27, Dimitri Fontaine wrote:
> The goal for this first patch is to avoid semantics issues so that we
> can get something technically clean in, and have more time to talk
> semantics next times. The main discussion to avoid is deciding if we
> want to fire event triggers for CREA
53 matches
Mail list logo
