Обсуждение: remaining open items
We've got a few open items remaining at https://wiki.postgresql.org/wiki/PostgreSQL_9.5_Open_Items - we should try to get rid of them. Of the 8 items there, 3 are documentation issues. It looks to me like one of those is for Stephen to deal with, one for Andres, and one for Alvaro. Is there any reason we can't get those taken care of and move on? On the remaining five issues: - Refactoring speculative insertion with unique indexes a little. Andres seems to think this shouldn't be an open issue, while Peter thinks maybe it should be, or at least that's my imperfect executive summary. Heikki isn't sure he agrees with all of the changes. I'm not very clear on whether this is a new feature, a bug fix, beautification, or something else. What would happen if we didn't do anything at all? - Oversize item computation needs more testing (c.f. ereport(ERROR) calls in brin_getinsertbuffer). This is pretty vague, and there's no thread linked. If there's a stability issue here, presumably it falls to Alvaro to fix it. But I don't know who added this item or what really needs to be done. - DDL deparsing testing module should have detected that transforms were not supported, but it failed to notice that. There's no thread linked to this one either, but the description of the issue seems clear enough. Alvaro, any chance that you can, as the comment says, whack it until it does? - Foreign join pushdown vs EvalPlanQual. Attempting to use the new foreign join pushdown support can crash the server if the query has locking clauses (or possibly if it's an UPDATE localtab FROM remotetab type of query, though I don't have a test case for that). There's been a lot of discussion about how to fix this, but unless Tom gets involved, I don't see this getting fixed in time for 9.5, because I don't know exactly how to fix it and I don't think anyone else does either. I have some ideas and I think those ideas are slowly getting better, but the reality is that if I'd realized this issue existed, I would not have committed the join pushdown support to 9.5. I didn't, and that was a screw-up on my part. I don't know what to recommend here: the choices seem to be (a) rip out all the foreign join pushdown support from 9.5, or (b) leave it in and accept that trying to use it may fail in some cases. Since postgres_fdw doesn't have any support for this anyway, it's not actively hurting anyone. But it's still bad that it's broken like this. - Strange behavior on track_commit_timestamp. As I've said on the thread, I think that the idea that the redo routines care about the value of the GUC at the time redo is performed (rather than at the time redo is generated), is broken. Fujii's latest post provides some examples of how this creates weird results. I really think this should be changed. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On 2015-10-13 21:57:15 -0400, Robert Haas wrote: > - Refactoring speculative insertion with unique indexes a little. > Andres seems to think this shouldn't be an open issue, while Peter > thinks maybe it should be, or at least that's my imperfect executive > summary. Heikki isn't sure he agrees with all of the changes. I'm > not very clear on whether this is a new feature, a bug fix, > beautification, or something else. > What would happen if we didn't do anything at all? Nothing, really. It's essentially some code beautification. A worthwhile goal, but certainly not a release blocker. Greetings, Andres Freund
Robert Haas wrote: > - Oversize item computation needs more testing (c.f. ereport(ERROR) > calls in brin_getinsertbuffer). This is pretty vague, and there's no > thread linked. If there's a stability issue here, presumably it falls > to Alvaro to fix it. But I don't know who added this item or what > really needs to be done. I added it, sorry it's vague. It means that I should test with values of increasing size and see if all the errors are correctly covered, i.e. we don't get a PANIC because of a failure in PageAddItem. > - DDL deparsing testing module should have detected that transforms > were not supported, but it failed to notice that. There's no thread > linked to this one either, but the description of the issue seems > clear enough. Alvaro, any chance that you can, as the comment says, > whack it until it does? I've been looking at this on and off, without anything useful to share yet. One approach that was suggested (which I don't like much, but I admit is a possible approach) is that we just need to remain vigilant that all features that add DDL properly test the event trigger side of it, just as we remain vigilant about pg_dump support without having any explicit test that it works. > - Strange behavior on track_commit_timestamp. As I've said on the > thread, I think that the idea that the redo routines care about the > value of the GUC at the time redo is performed (rather than at the > time redo is generated), is broken. Fujii's latest post provides some > examples of how this creates weird results. I really think this > should be changed. We have discussed this; Petr is going to post a patch shortly. The other item on me is the documentation patch by Emre Hasegeli for usage of the inclusion opclass framework in BRIN. I think it needs some slight revision by some native English speaker and I'm not completely in love with the proposed third column in the table it adds, but otherwise is factually correct as far as I can tell. -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Wed, Oct 14, 2015 at 3:14 AM, Andres Freund <andres@anarazel.de> wrote: >> What would happen if we didn't do anything at all? > > Nothing, really. It's essentially some code beautification. A worthwhile > goal, but certainly not a release blocker. While I agree with this assessment, I think that there is value in doing it before release, to ease keeping the branches in sync. That seems like the better time to backpatch to 9.5. That was the thinking behind putting it on the open items list. -- Peter Geoghegan
* Robert Haas (robertmhaas@gmail.com) wrote: > We've got a few open items remaining at > https://wiki.postgresql.org/wiki/PostgreSQL_9.5_Open_Items - we should > try to get rid of them. Of the 8 items there, 3 are documentation > issues. It looks to me like one of those is for Stephen to deal with, > one for Andres, and one for Alvaro. Is there any reason we can't get > those taken care of and move on? I'll get the documentation issue dealt with. I've got a bunch of pending documentation improvements that need to be done and hope to post them very shortly for comment. Thanks! Stephen
On Thu, Oct 15, 2015 at 1:48 PM, Peter Geoghegan <pg@heroku.com> wrote: > On Wed, Oct 14, 2015 at 3:14 AM, Andres Freund <andres@anarazel.de> wrote: >>> What would happen if we didn't do anything at all? >> >> Nothing, really. It's essentially some code beautification. A worthwhile >> goal, but certainly not a release blocker. > > While I agree with this assessment, I think that there is value in > doing it before release, to ease keeping the branches in sync. That > seems like the better time to backpatch to 9.5. That was the thinking > behind putting it on the open items list. That makes sense, but I think it's time to remove anything that doesn't smell like an actual release blocker. I'll go do that. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Thu, Oct 15, 2015 at 12:22 PM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote: > Robert Haas wrote: >> - Oversize item computation needs more testing (c.f. ereport(ERROR) >> calls in brin_getinsertbuffer). This is pretty vague, and there's no >> thread linked. If there's a stability issue here, presumably it falls >> to Alvaro to fix it. But I don't know who added this item or what >> really needs to be done. > > I added it, sorry it's vague. It means that I should test with > values of increasing size and see if all the errors are correctly > covered, i.e. we don't get a PANIC because of a failure in PageAddItem. So, are you going to do that? >> - DDL deparsing testing module should have detected that transforms >> were not supported, but it failed to notice that. There's no thread >> linked to this one either, but the description of the issue seems >> clear enough. Alvaro, any chance that you can, as the comment says, >> whack it until it does? > > I've been looking at this on and off, without anything useful to share > yet. One approach that was suggested (which I don't like much, but I > admit is a possible approach) is that we just need to remain vigilant > that all features that add DDL properly test the event trigger side of > it, just as we remain vigilant about pg_dump support without having any > explicit test that it works. I really, really hope that's not where we end up. Just shoot me now. >> - Strange behavior on track_commit_timestamp. As I've said on the >> thread, I think that the idea that the redo routines care about the >> value of the GUC at the time redo is performed (rather than at the >> time redo is generated), is broken. Fujii's latest post provides some >> examples of how this creates weird results. I really think this >> should be changed. > > We have discussed this; Petr is going to post a patch shortly. Cool. > The other item on me is the documentation patch by Emre Hasegeli for > usage of the inclusion opclass framework in BRIN. I think it needs some > slight revision by some native English speaker and I'm not completely in > love with the proposed third column in the table it adds, but otherwise > is factually correct as far as I can tell. I'm not clear whether you are asking for help with this, or ...? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Robert Haas wrote: > On Thu, Oct 15, 2015 at 12:22 PM, Alvaro Herrera > <alvherre@2ndquadrant.com> wrote: > > Robert Haas wrote: > >> - Oversize item computation needs more testing (c.f. ereport(ERROR) > >> calls in brin_getinsertbuffer). This is pretty vague, and there's no > >> thread linked. If there's a stability issue here, presumably it falls > >> to Alvaro to fix it. But I don't know who added this item or what > >> really needs to be done. > > > > I added it, sorry it's vague. It means that I should test with > > values of increasing size and see if all the errors are correctly > > covered, i.e. we don't get a PANIC because of a failure in PageAddItem. > > So, are you going to do that? Yes. > > I've been looking at this on and off, without anything useful to share > > yet. One approach that was suggested (which I don't like much, but I > > admit is a possible approach) is that we just need to remain vigilant > > that all features that add DDL properly test the event trigger side of > > it, just as we remain vigilant about pg_dump support without having any > > explicit test that it works. > > I really, really hope that's not where we end up. Just shoot me now. Me too. I'm going to do something about this, though I can't promise a deadline. > > The other item on me is the documentation patch by Emre Hasegeli for > > usage of the inclusion opclass framework in BRIN. I think it needs some > > slight revision by some native English speaker and I'm not completely in > > love with the proposed third column in the table it adds, but otherwise > > is factually correct as far as I can tell. > > I'm not clear whether you are asking for help with this, or ...? I enlisted the help of Ian Barwick for this one. -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 16 October 2015 at 20:17, Robert Haas <robertmhaas@gmail.com> wrote:
--
>> - DDL deparsing testing module should have detected that transforms
>> were not supported, but it failed to notice that. There's no thread
>> linked to this one either, but the description of the issue seems
>> clear enough. Alvaro, any chance that you can, as the comment says,
>> whack it until it does?
>
> I've been looking at this on and off, without anything useful to share
> yet. One approach that was suggested (which I don't like much, but I
> admit is a possible approach) is that we just need to remain vigilant
> that all features that add DDL properly test the event trigger side of
> it, just as we remain vigilant about pg_dump support without having any
> explicit test that it works.
I really, really hope that's not where we end up. Just shoot me now.
I share your pain, but the latter appears to be the only actionable proposal at present.
A test suite was originally written, but not committed.
Why do we need to fix DDL when pg_dump remains annoying in the same way?
Why do we need to fix it now? Surely this will break things in later releases, but not in 9.5, since we aren't ever going to add DDL to 9.5 again.
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Simon Riggs <simon@2ndQuadrant.com> writes:
> On 16 October 2015 at 20:17, Robert Haas <robertmhaas@gmail.com> wrote:
>>>> - DDL deparsing testing module should have detected that transforms
>>>> were not supported, but it failed to notice that. There's no thread
>>>> linked to this one either, but the description of the issue seems
>>>> clear enough. Alvaro, any chance that you can, as the comment says,
>>>> whack it until it does?
>>> I've been looking at this on and off, without anything useful to share
>>> yet. One approach that was suggested (which I don't like much, but I
>>> admit is a possible approach) is that we just need to remain vigilant
>>> that all features that add DDL properly test the event trigger side of
>>> it, just as we remain vigilant about pg_dump support without having any
>>> explicit test that it works.
>> I really, really hope that's not where we end up. Just shoot me now.
> I share your pain, but the latter appears to be the only actionable
> proposal at present.
I had the idea that the test suite would consist of "run the standard
regression tests with a DDL deparsing hook installed, and see if it fails
anywhere". This would not prove that the deparsing logic is right,
certainly, but it would at least catch errors of omission for any DDL
tested in the regression tests, which we could hope is pretty much
everything.
> Why do we need to fix DDL when pg_dump remains annoying in the same way?
It is not true that we have no test coverage for pg_dump: the pg_upgrade
test exercises pg_dump too. The difficulty with pg_dump is that this
approach only tests dumping of objects that are left behind at the end of
the regression tests, and we get too many submissions from neatniks who
think that test cases ought to delete all the objects they create. But
that is just a matter of needing to formulate test cases with this in
mind.
> Why do we need to fix it now? Surely this will break things in later
> releases, but not in 9.5, since we aren't ever going to add DDL to 9.5
> again.
This is a fair point. We still need a test methodology here, but it's
not clear why it needs to be regarded as a 9.5 blocker.
regards, tom lane
On 17/10/15 04:31, Alvaro Herrera wrote: > Robert Haas wrote: >>> The other item on me is the documentation patch by Emre Hasegeli for >>> usage of the inclusion opclass framework in BRIN. I think it needs some >>> slight revision by some native English speaker and I'm not completely in >>> love with the proposed third column in the table it adds, but otherwise >>> is factually correct as far as I can tell. >> >> I'm not clear whether you are asking for help with this, or ...? > > I enlisted the help of Ian Barwick for this one. Revised version of Emre's patch attached, apologies for the delay. Regards Ian Barwick -- Ian Barwick http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
Вложения
On 14 October 2015 at 09:57, Robert Haas <robertmhaas@gmail.com> wrote: > We've got a few open items remaining at > https://wiki.postgresql.org/wiki/PostgreSQL_9.5_Open_Items - we should > try to get rid of them. I'd like to add, rather than remove, one. A bug in replication origins support. It comes with a simple fix. See patch: http://www.postgresql.org/message-id/CAMsr+YG3cWUXCSyXn1PXJy+ZX8i6o71sGuteGDUZq+SBu4ERRw@mail.gmail.com and thread start: http://www.postgresql.org/message-id/CAMsr+YFhBJLp=qfSz3-J+0P1zLkE8zNXM2otycn20QRMx380gw@mail.gmail.com -- Craig Ringer http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
Craig Ringer wrote: > On 14 October 2015 at 09:57, Robert Haas <robertmhaas@gmail.com> wrote: > > We've got a few open items remaining at > > https://wiki.postgresql.org/wiki/PostgreSQL_9.5_Open_Items - we should > > try to get rid of them. > > I'd like to add, rather than remove, one. A bug in replication origins > support. It comes with a simple fix. Please edit the wiki to get it listed. -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Ian Barwick wrote: > On 17/10/15 04:31, Alvaro Herrera wrote: > > Robert Haas wrote: > >>> The other item on me is the documentation patch by Emre Hasegeli for > >>> usage of the inclusion opclass framework in BRIN. I think it needs some > >>> slight revision by some native English speaker and I'm not completely in > >>> love with the proposed third column in the table it adds, but otherwise > >>> is factually correct as far as I can tell. > >> > >> I'm not clear whether you are asking for help with this, or ...? > > > > I enlisted the help of Ian Barwick for this one. > > Revised version of Emre's patch attached, apologies for the delay. Great, thanks, pushed. I only changed this: > ! strategies are embedded in the support procedures's source code. > --- 535,541 ---- > ! strategies are embedded in the support procedure's source code. to procedures' which is the right possessive form, I think (I want "procedures" to remain in the plural form). -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services