Quantcast

Last gasp

classic Classic list List threaded Threaded
242 messages Options
1234 ... 13
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Last gasp

Simon Riggs
These patches aren't marked with a committer

FK arrays
ECPG fetch
foreign stats
command triggers
check function
parallel pg_dump

Does that mean myself or others should be claiming them for commit/reject?

--
 Simon Riggs                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Last gasp

Robert Haas
On Thu, Apr 5, 2012 at 1:28 PM, Simon Riggs <[hidden email]> wrote:

> These patches aren't marked with a committer
>
> FK arrays
> ECPG fetch
> foreign stats
> command triggers
> check function
> parallel pg_dump
>
> Does that mean myself or others should be claiming them for commit/reject?

I've been right in the middle of the command triggers stuff, so I
suppose I would pick that up for commit if it were ready, but there
hasn't been a new version this week unless I've missed it, and even if
the new version arrived right this minute, I don't think I or anyone
else can do a good job committing a patch of that size in the time
remaining.  So I think it's time to formally put that one out of its
misery.

I think the ECPG fetch patch is about ready to go.  Normally Michael
Meskes handles all ECPG patches, but I'm not sure what his schedule is
like.  I'm not sure what the politics are of someone else touching
that code.

Heikki recently produced a revision of the check function patch, but
I'm not sure whether he's planning to commit that or whether it's a
demonstration of a broader rework he wants Pavel (or someone else) to
do.  At any rate, I think it's up to him whether or not to commit
that.

I think Andrew is working on parallel pg_dump, so I suspect the rest
of us should stay out of the way.  I've also looked at it extensively
and will work on it if Andrew isn't, but I don't think it's ready to
commit.  A few preliminary pieces could go in, perhaps.

I am not sure what the state of the foreign stats patch is, or FK arrays.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Last gasp

Heikki Linnakangas-3
On 05.04.2012 21:00, Robert Haas wrote:
> Heikki recently produced a revision of the check function patch, but
> I'm not sure whether he's planning to commit that or whether it's a
> demonstration of a broader rework he wants Pavel (or someone else) to
> do.  At any rate, I think it's up to him whether or not to commit
> that.

Yeah, I guess it's up to me, now that I've touched that code. I don't
feel ready to commit it yet. It needs some more cleanup, which I could
do, but frankly even with the refactoring I did I'm still not totally
happy with the way it works. I feel that there ought to be a less
duplicative approach, but I don't have any more concrete proposals.
Unless someone more motivated picks up the patch now, I think it needs
to be pushed to 9.3.

--
   Heikki Linnakangas
   EnterpriseDB   http://www.enterprisedb.com

--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Last gasp

Simon Riggs
In reply to this post by Robert Haas
On Thu, Apr 5, 2012 at 7:00 PM, Robert Haas <[hidden email]> wrote:

> On Thu, Apr 5, 2012 at 1:28 PM, Simon Riggs <[hidden email]> wrote:
>> These patches aren't marked with a committer
>>
>> FK arrays
>> ECPG fetch
>> foreign stats
>> command triggers
>> check function
>> parallel pg_dump
>>
>> Does that mean myself or others should be claiming them for commit/reject?
>
> I've been right in the middle of the command triggers stuff, so I
> suppose I would pick that up for commit if it were ready, but there
> hasn't been a new version this week unless I've missed it, and even if
> the new version arrived right this minute, I don't think I or anyone
> else can do a good job committing a patch of that size in the time
> remaining.  So I think it's time to formally put that one out of its
> misery.

I think doing so will cause substantial misery for many users. I find
it hard to believe that such a simple concept hasn't managed to
produce some workable subset after months of work.

--
 Simon Riggs                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Last gasp

Tom Lane-2
In reply to this post by Robert Haas
Robert Haas <[hidden email]> writes:
> On Thu, Apr 5, 2012 at 1:28 PM, Simon Riggs <[hidden email]> wrote:
>> These patches aren't marked with a committer

> I think the ECPG fetch patch is about ready to go.  Normally Michael
> Meskes handles all ECPG patches, but I'm not sure what his schedule is
> like.  I'm not sure what the politics are of someone else touching
> that code.

I think we should leave that one for Michael.  Frankly, none of the
rest of us pay enough attention to ecpg to be candidates to take
responsibility for nontrivial patches there.

In fact, for *most* of these patches the fact that they're still here
should give you pause about just committing them.

> I am not sure what the state of the foreign stats patch is, or FK arrays.

I'm taking the foreign stats one; as I mentioned a bit ago, I think we
need to push an FDW ANALYZE hook into 9.2.  I don't like the details of
what's in the current submission but I think it's fixable with not much
effort.

The FK arrays one I'm kind of queasy about.  It's a cool-sounding idea
but I'm not convinced that all the corner-case details have been
adequately thought through, and I'm scared of being unable to fix any
such bugs in later versions because of backwards compatibility worries.
It'd be a lot better to be pushing this in at the start of a devel cycle
than the end.

Most of the rest of this stuff I'm about ready to put off to 9.3.
The risk/reward ratio for committing it on the last day of the last
9.2 fest doesn't look good.

                        regards, tom lane

--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Last gasp

Robert Haas
In reply to this post by Simon Riggs
On Thu, Apr 5, 2012 at 2:17 PM, Simon Riggs <[hidden email]> wrote:

> On Thu, Apr 5, 2012 at 7:00 PM, Robert Haas <[hidden email]> wrote:
>> On Thu, Apr 5, 2012 at 1:28 PM, Simon Riggs <[hidden email]> wrote:
>>> These patches aren't marked with a committer
>>>
>>> FK arrays
>>> ECPG fetch
>>> foreign stats
>>> command triggers
>>> check function
>>> parallel pg_dump
>>>
>>> Does that mean myself or others should be claiming them for commit/reject?
>>
>> I've been right in the middle of the command triggers stuff, so I
>> suppose I would pick that up for commit if it were ready, but there
>> hasn't been a new version this week unless I've missed it, and even if
>> the new version arrived right this minute, I don't think I or anyone
>> else can do a good job committing a patch of that size in the time
>> remaining.  So I think it's time to formally put that one out of its
>> misery.
>
> I think doing so will cause substantial misery for many users. I find
> it hard to believe that such a simple concept hasn't managed to
> produce some workable subset after months of work.

I am not interested in relitigating on this thread what has already
been extensively discussed nearby.  Dimitri and I agreed on numerous
changes to try to make the behavior sane, and those changes were
suggested and agreed to for good reason.  We didn't agree on every
point, of course, but we did agree on most of it, and there is no
patch that implements what was agreed.  Even if there were, there is
not time to review and commit a heavily revised version of a >1000
line patch before tomorrow, and any suggestion to the contrary is just
plain wrong.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Last gasp

Pavel Stehule
In reply to this post by Heikki Linnakangas-3
2012/4/5 Heikki Linnakangas <[hidden email]>:

> On 05.04.2012 21:00, Robert Haas wrote:
>>
>> Heikki recently produced a revision of the check function patch, but
>> I'm not sure whether he's planning to commit that or whether it's a
>> demonstration of a broader rework he wants Pavel (or someone else) to
>> do.  At any rate, I think it's up to him whether or not to commit
>> that.
>
>
> Yeah, I guess it's up to me, now that I've touched that code. I don't feel
> ready to commit it yet. It needs some more cleanup, which I could do, but
> frankly even with the refactoring I did I'm still not totally happy with the
> way it works. I feel that there ought to be a less duplicative approach, but
> I don't have any more concrete proposals. Unless someone more motivated
> picks up the patch now, I think it needs to be pushed to 9.3.

I played with more general plpgpsm walker, but I am sure, so it is
wrong way - request for checking and dumping, and releasing plans are
too different. So there are not too much possibilities :(

The main issue is in design of exec nodes. I have no idea how to move
checking to there without increasing complexity in pl_exec.c. Some are
relative simply, but "case", "if", "block" are not.

Other idea is moving plpgsql_check_function to contrib.

Regards

Pavel

>
> --
>  Heikki Linnakangas
>  EnterpriseDB   http://www.enterprisedb.com

t

--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Last gasp

Robert Haas
In reply to this post by Tom Lane-2
On Thu, Apr 5, 2012 at 2:23 PM, Tom Lane <[hidden email]> wrote:
> In fact, for *most* of these patches the fact that they're still here
> should give you pause about just committing them.

Um, yeah.

>> I am not sure what the state of the foreign stats patch is, or FK arrays.
>
> I'm taking the foreign stats one; as I mentioned a bit ago, I think we
> need to push an FDW ANALYZE hook into 9.2.  I don't like the details of
> what's in the current submission but I think it's fixable with not much
> effort.

Cool.

> The FK arrays one I'm kind of queasy about.  It's a cool-sounding idea
> but I'm not convinced that all the corner-case details have been
> adequately thought through, and I'm scared of being unable to fix any
> such bugs in later versions because of backwards compatibility worries.
> It'd be a lot better to be pushing this in at the start of a devel cycle
> than the end.

I've been feeling that that patch has been suffering from a lack of
reviewer attention, which is a real shame, because I think the
functionality is indeed really cool.  But I haven't looked at it
enough to know what kind of shape it's in.

> Most of the rest of this stuff I'm about ready to put off to 9.3.
> The risk/reward ratio for committing it on the last day of the last
> 9.2 fest doesn't look good.

+1.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Last gasp

Alvaro Herrera-7
In reply to this post by Simon Riggs

Excerpts from Simon Riggs's message of jue abr 05 14:28:54 -0300 2012:

> These patches aren't marked with a committer
>
> FK arrays
> ECPG fetch
> foreign stats
> command triggers
> check function
> parallel pg_dump
>
> Does that mean myself or others should be claiming them for commit/reject?

The FK locking patch isn't on this list; however, I'm hijacking this
thread to say that some benchmarking runs we tried weren't all that
great, showing 9% performance degradation on stock pgbench -- i.e.  a
large hit that will harm everybody even if they are not using FKs at
all.  I'm thus setting the patch returned with feedback, which is sure
to make several hackers happy and tons of users unhappy.

--
Álvaro Herrera <[hidden email]>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Last gasp

Simon Riggs
In reply to this post by Tom Lane-2
On Thu, Apr 5, 2012 at 7:23 PM, Tom Lane <[hidden email]> wrote:

> The FK arrays one I'm kind of queasy about.  It's a cool-sounding idea
> but I'm not convinced that all the corner-case details have been
> adequately thought through, and I'm scared of being unable to fix any
> such bugs in later versions because of backwards compatibility worries.
> It'd be a lot better to be pushing this in at the start of a devel cycle
> than the end.

OK, that's clear. I would have taken it, but not now.

--
 Simon Riggs                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Last gasp

Robert Haas
In reply to this post by Alvaro Herrera-7
On Thu, Apr 5, 2012 at 2:37 PM, Alvaro Herrera
<[hidden email]> wrote:
> The FK locking patch isn't on this list; however, I'm hijacking this
> thread to say that some benchmarking runs we tried weren't all that
> great, showing 9% performance degradation on stock pgbench -- i.e.  a
> large hit that will harm everybody even if they are not using FKs at
> all.  I'm thus setting the patch returned with feedback, which is sure
> to make several hackers happy and tons of users unhappy.

Ouch!  That's a real bummer.  It makes me glad that you tested it, but
I can't say I'm happy about the outcome.  Did you get in any insight
into where the regression is coming from?

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Last gasp

Simon Riggs
In reply to this post by Alvaro Herrera-7
On Thu, Apr 5, 2012 at 7:37 PM, Alvaro Herrera
<[hidden email]> wrote:

> The FK locking patch isn't on this list;

We'd agreed you were the assigned committer, hence not on list.

> however, I'm hijacking this
> thread to say that some benchmarking runs we tried weren't all that
> great, showing 9% performance degradation on stock pgbench -- i.e.  a
> large hit that will harm everybody even if they are not using FKs at
> all.  I'm thus setting the patch returned with feedback, which is sure
> to make several hackers happy and tons of users unhappy.

I've done what I can to alter that, but I think its the right decision
at this point. I would say its been the largest and most subtle patch
submitted, so please don't be down by that.

--
 Simon Riggs                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Last gasp

Hannu Krosing-3
In reply to this post by Robert Haas
On Thu, 2012-04-05 at 14:27 -0400, Robert Haas wrote:

> On Thu, Apr 5, 2012 at 2:17 PM, Simon Riggs <[hidden email]> wrote:
> > On Thu, Apr 5, 2012 at 7:00 PM, Robert Haas <[hidden email]> wrote:
> >> On Thu, Apr 5, 2012 at 1:28 PM, Simon Riggs <[hidden email]> wrote:
> >>> These patches aren't marked with a committer
> >>>
> >>> FK arrays
> >>> ECPG fetch
> >>> foreign stats
> >>> command triggers
> >>> check function
> >>> parallel pg_dump
> >>>
> >>> Does that mean myself or others should be claiming them for commit/reject?
> >>
> >> I've been right in the middle of the command triggers stuff, so I
> >> suppose I would pick that up for commit if it were ready, but there
> >> hasn't been a new version this week unless I've missed it, and even if
> >> the new version arrived right this minute, I don't think I or anyone
> >> else can do a good job committing a patch of that size in the time
> >> remaining.  So I think it's time to formally put that one out of its
> >> misery.
> >
> > I think doing so will cause substantial misery for many users. I find
> > it hard to believe that such a simple concept hasn't managed to
> > produce some workable subset after months of work.
>
> I am not interested in relitigating on this thread what has already
> been extensively discussed nearby.  Dimitri and I agreed on numerous
> changes to try to make the behavior sane,

To me it looked like the scope of the patch started to suddenly expand
exponentially a few days ago from a simple COMMAND TRIGGERS, which would
have finally enabled trigger-based or "logical" replication systems to
do full replication to something recursive which would attempt to cover
all weird combinations of commands triggering other commands for which
there is no real use-case in view, except a suggestion "don't do it" :)

The latest patch (v18) seemed quite ok for its original intended
purpose.

>  and those changes were
> suggested and agreed to for good reason.  We didn't agree on every
> point, of course, but we did agree on most of it, and there is no
> patch that implements what was agreed.  Even if there were, there is
> not time to review and commit a heavily revised version of a >1000
> line patch before tomorrow, and any suggestion to the contrary is just
> plain wrong.
>
> --
> Robert Haas
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>



--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Last gasp

Michael Meskes-3
In reply to this post by Tom Lane-2
On Thu, Apr 05, 2012 at 02:23:03PM -0400, Tom Lane wrote:
> > I think the ECPG fetch patch is about ready to go.  Normally Michael
> > Meskes handles all ECPG patches, but I'm not sure what his schedule is
> > like.  I'm not sure what the politics are of someone else touching
> > that code.
>
> I think we should leave that one for Michael.  Frankly, none of the
> rest of us pay enough attention to ecpg to be candidates to take
> responsibility for nontrivial patches there.

I will take care of this over the next couple days. Is the patch that Zoltan
send last Friday the latest version?

Michael
--
Michael Meskes
Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org)
Michael at BorussiaFan dot De, Meskes at (Debian|Postgresql) dot Org
Jabber: michael.meskes at googlemail dot com
VfL Borussia! Força Barça! Go SF 49ers! Use Debian GNU/Linux, PostgreSQL

--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Last gasp

Hannu Krosing-3
In reply to this post by Hannu Krosing-3
On Thu, 2012-04-05 at 20:46 +0200, Hannu Krosing wrote:
> On Thu, 2012-04-05 at 14:27 -0400, Robert Haas wrote:
> > On Thu, Apr 5, 2012 at 2:17 PM, Simon Riggs <[hidden email]> wrote:

...

> > > I think doing so will cause substantial misery for many users. I find
> > > it hard to believe that such a simple concept hasn't managed to
> > > produce some workable subset after months of work.
> >
> > I am not interested in relitigating on this thread what has already
> > been extensively discussed nearby.  Dimitri and I agreed on numerous
> > changes to try to make the behavior sane,
>
> To me it looked like the scope of the patch started to suddenly expand
> exponentially a few days ago from a simple COMMAND TRIGGERS, which would
> have finally enabled trigger-based or "logical" replication systems to
> do full replication to something recursive which would attempt to cover
> all weird combinations of commands triggering other commands for which
> there is no real use-case in view, except a suggestion "don't do it" :)
>
> The latest patch (v18) seemed quite ok for its original intended
> purpose.

Sorry, i hit "send!" too early.

Would it be possible to put some "command trigger hooks" in a few
strategic places so that some trigger-like functionality could be loaded
at run time, mainly with a view of writing DDL replication
'non-triggers' , mostly based on current v18 code, but of course without
all the nice CREATE TRIGGER syntax ?

perhaps created with a pg_create_command_trigger(...)

that is something in the line of how Full Text Indexing was done for a
long time.

> >  and those changes were
> > suggested and agreed to for good reason.  We didn't agree on every
> > point, of course, but we did agree on most of it, and there is no
> > patch that implements what was agreed.  Even if there were, there is
> > not time to review and commit a heavily revised version of a >1000
> > line patch before tomorrow, and any suggestion to the contrary is just
> > plain wrong.
> >
> > --
> > Robert Haas
> > EnterpriseDB: http://www.enterprisedb.com
> > The Enterprise PostgreSQL Company
> >
>
>
>



--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Last gasp

Andrew Dunstan
In reply to this post by Robert Haas


On 04/05/2012 02:00 PM, Robert Haas wrote:
> I think Andrew is working on parallel pg_dump, so I suspect the rest
> of us should stay out of the way. I've also looked at it extensively
> and will work on it if Andrew isn't, but I don't think it's ready to
> commit. A few preliminary pieces could go in, perhaps.


I am working on it when I get time, but I am slammed. It would probably
take me a couple of full days to do a thorough code review, and finding
that is hard.

cheers

andrew

--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Last gasp

Robert Haas
In reply to this post by Hannu Krosing-3
On Thu, Apr 5, 2012 at 2:46 PM, Hannu Krosing <[hidden email]> wrote:
> To me it looked like the scope of the patch started to suddenly expand
> exponentially a few days ago from a simple COMMAND TRIGGERS, which would
> have finally enabled trigger-based or "logical" replication systems to
> do full replication to something recursive which would attempt to cover
> all weird combinations of commands triggering other commands for which
> there is no real use-case in view, except a suggestion "don't do it" :)
>
> The latest patch (v18) seemed quite ok for its original intended
> purpose.

OK, so here we go, rehashing the discussion we already had on thread A
on thread B.  The particular issue you are mentioning there was not
the reason that patch isn't going to end up in 9.2.  If the only thing
the patch had needed was a little renaming and syntax cleanup, I would
have done it myself (or Dimitri would have) and I would have committed
it.  That is not the problem, or at least it's not the only problem.
There are at least two other major issues:

- The patch as posted fires triggers at unpredictable times depending
on which command you're executing.  Some things that are really
sub-commands fire triggers anyway as if they were toplevel commands;
others don't; whether or not it happens in a particular case is
determined by implementation details rather than by any consistent
principle of operation.  In the cases where triggers do fire, they
don't always fire at the same place in the execution sequence.

- The patch isn't safe if the triggers try to execute DDL on the
object being modified.  It's not exactly clear what misbehavior will
result in every case, but it is clear that that it hasn't really been
thought about.

Now, if anyone who was actually following the conversation thought
these things were not problems, they could have written back to the
relevant thread and said, hey, I don't mind if the trigger firing
behavior changes every time someone does any refactoring of our
badly-written DDL code and if the server blows up in random ways when
someone does something unexpected in the trigger that's OK with me
too.  Maybe not surprisingly, no one said that.  Two people wrote into
that thread after my latest round of reviewing and both of them
disagreed with only minor points of my review, and we had a technical
discussion about those issues.  But showing up after the fact and
acting as if there were no serious issues found during that review is
either disingenuous or a sign that you didn't really read the thread.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Last gasp

Robert Haas
In reply to this post by Hannu Krosing-3
On Thu, Apr 5, 2012 at 2:58 PM, Hannu Krosing <[hidden email]> wrote:

> On Thu, 2012-04-05 at 20:46 +0200, Hannu Krosing wrote:
>> On Thu, 2012-04-05 at 14:27 -0400, Robert Haas wrote:
>> > On Thu, Apr 5, 2012 at 2:17 PM, Simon Riggs <[hidden email]> wrote:
>
> ...
>> > > I think doing so will cause substantial misery for many users. I find
>> > > it hard to believe that such a simple concept hasn't managed to
>> > > produce some workable subset after months of work.
>> >
>> > I am not interested in relitigating on this thread what has already
>> > been extensively discussed nearby.  Dimitri and I agreed on numerous
>> > changes to try to make the behavior sane,
>>
>> To me it looked like the scope of the patch started to suddenly expand
>> exponentially a few days ago from a simple COMMAND TRIGGERS, which would
>> have finally enabled trigger-based or "logical" replication systems to
>> do full replication to something recursive which would attempt to cover
>> all weird combinations of commands triggering other commands for which
>> there is no real use-case in view, except a suggestion "don't do it" :)
>>
>> The latest patch (v18) seemed quite ok for its original intended
>> purpose.
>
> Sorry, i hit "send!" too early.
>
> Would it be possible to put some "command trigger hooks" in a few
> strategic places so that some trigger-like functionality could be loaded
> at run time, mainly with a view of writing DDL replication
> 'non-triggers' , mostly based on current v18 code, but of course without
> all the nice CREATE TRIGGER syntax ?

I certainly think that would be a possible way forward, but I don't
think we should try to engineer that in the next 24 hours.  Had the
original goals of the patch been somewhat more modest, I think we
could have gotten it into 9.2, but there's no time to rethink the
scope of the patch now.  With all respect for Dimitri and his *very*
hard work on this subject, submitting a brand new major feature to the
last CommitFest is not really a great way to get it committed,
especially given that we didn't have consensus on the design before he
started coding.  There is every reason to think that we can get this
feature into 9.3 with some more work, but it's not ready yet, and
wishing won't make it so.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Last gasp

Alvaro Herrera-7
In reply to this post by Robert Haas

Excerpts from Robert Haas's message of jue abr 05 15:40:17 -0300 2012:

>
> On Thu, Apr 5, 2012 at 2:37 PM, Alvaro Herrera
> <[hidden email]> wrote:
> > The FK locking patch isn't on this list; however, I'm hijacking this
> > thread to say that some benchmarking runs we tried weren't all that
> > great, showing 9% performance degradation on stock pgbench -- i.e.  a
> > large hit that will harm everybody even if they are not using FKs at
> > all.  I'm thus setting the patch returned with feedback, which is sure
> > to make several hackers happy and tons of users unhappy.
>
> Ouch!  That's a real bummer.  It makes me glad that you tested it, but
> I can't say I'm happy about the outcome.  Did you get in any insight
> into where the regression is coming from?

Not really -- after reaching that conclusion I dropped immediate work on
the patch to do other stuff (like checking whether there's any other
patch I can help with in commitfest).  I will resume work later, for a
(hopefully early) 9.3 submission.

--
Álvaro Herrera <[hidden email]>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Last gasp

Hannu Krosing-3
In reply to this post by Robert Haas
On Thu, 2012-04-05 at 15:02 -0400, Robert Haas wrote:

> On Thu, Apr 5, 2012 at 2:46 PM, Hannu Krosing <[hidden email]> wrote:
> > To me it looked like the scope of the patch started to suddenly expand
> > exponentially a few days ago from a simple COMMAND TRIGGERS, which would
> > have finally enabled trigger-based or "logical" replication systems to
> > do full replication to something recursive which would attempt to cover
> > all weird combinations of commands triggering other commands for which
> > there is no real use-case in view, except a suggestion "don't do it" :)
> >
> > The latest patch (v18) seemed quite ok for its original intended
> > purpose.
>
> OK, so here we go, rehashing the discussion we already had on thread A
> on thread B.  The particular issue you are mentioning there was not
> the reason that patch isn't going to end up in 9.2.  If the only thing
> the patch had needed was a little renaming and syntax cleanup, I would
> have done it myself (or Dimitri would have) and I would have committed
> it.  That is not the problem, or at least it's not the only problem.
> There are at least two other major issues:
>
> - The patch as posted fires triggers at unpredictable times depending
> on which command you're executing.  Some things that are really
> sub-commands fire triggers anyway as if they were toplevel commands;
> others don't; whether or not it happens in a particular case is
> determined by implementation details rather than by any consistent
> principle of operation.  In the cases where triggers do fire, they
> don't always fire at the same place in the execution sequence.

For me it would be enough to know if the trigger is fired by top-level
command or not.

In worst case I could probably detect it myself, just give me something
to hang the detection code to .

> - The patch isn't safe if the triggers try to execute DDL on the
> object being modified.  It's not exactly clear what misbehavior will
> result in every case, but it is clear that that it hasn't really been
> thought about.

It never seemed important for me, as the only thing I was ever expecting
to do in a command trigger was inserting rows in one unrelated table.

To me DDL-triggered-by-DDL seemed like a very bad idea anyway.

But as you seemed to be envisioning some use-cases for that I did not
object to you working it out.

> Now, if anyone who was actually following the conversation thought
> these things were not problems, they could have written back to the
> relevant thread and said, hey, I don't mind if the trigger firing
> behavior changes every time someone does any refactoring of our
> badly-written DDL code and if the server blows up in random ways when
> someone does something unexpected in the trigger that's OK with me
> too.  

I don't mind if the trigger firing behavior changes every time someone
does any refactoring of our badly-written DDL code

Here :)

> Maybe not surprisingly, no one said that.  Two people wrote into
> that thread after my latest round of reviewing and both of them
> disagreed with only minor points of my review, and we had a technical
> discussion about those issues.  But showing up after the fact and
> acting as if there were no serious issues found during that review is
> either disingenuous or a sign that you didn't really read the thread.

As there are other ways to blow up the backend, i did not object to you
either working out a better solution or leaving it as it is.

I am speaking up now as this is the first time I am told I have to wait
another year for this feature to arrive.


> --
> Robert Haas
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>



--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
1234 ... 13
Loading...