remaining open items

Started by Robert Haasover 10 years ago14 messageshackers
Jump to latest
#1Robert Haas
robertmhaas@gmail.com

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#2Andres Freund
andres@anarazel.de
In reply to: Robert Haas (#1)
Re: remaining open items

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#3Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Robert Haas (#1)
Re: remaining open items

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

In reply to: Andres Freund (#2)
Re: remaining open items

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#5Stephen Frost
sfrost@snowman.net
In reply to: Robert Haas (#1)
Re: remaining open items

* 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

#6Robert Haas
robertmhaas@gmail.com
In reply to: Peter Geoghegan (#4)
Re: remaining open items

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#7Robert Haas
robertmhaas@gmail.com
In reply to: Alvaro Herrera (#3)
Re: remaining open items

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#8Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Robert Haas (#7)
Re: remaining open items

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#9Simon Riggs
simon@2ndQuadrant.com
In reply to: Robert Haas (#7)
Re: remaining open items

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/
<http://www.2ndquadrant.com/&gt;
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#10Tom Lane
tgl@sss.pgh.pa.us
In reply to: Simon Riggs (#9)
Re: remaining open items

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#11Ian Lawrence Barwick
barwick@gmail.com
In reply to: Alvaro Herrera (#8)
Re: remaining open items

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

Attachments:

0001-Improve-BRIN-documentation-for-Inclusion-opclass-v2.patchtext/x-patch; name=0001-Improve-BRIN-documentation-for-Inclusion-opclass-v2.patchDownload+196-10
#12Craig Ringer
craig@2ndquadrant.com
In reply to: Robert Haas (#1)
Re: remaining open items

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:

/messages/by-id/CAMsr+YG3cWUXCSyXn1PXJy+ZX8i6o71sGuteGDUZq+SBu4ERRw@mail.gmail.com

and thread start:

/messages/by-id/CAMsr+YFhBJLp=qfSz3-J+0P1zLkE8zNXM2otycn20QRMx380gw@mail.gmail.com

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#13Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Craig Ringer (#12)
Re: remaining open items

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#14Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Ian Lawrence Barwick (#11)
Re: remaining open items

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers