Triage on old commitfest entries

Started by Tom Laneover 4 years ago20 messages
#1Tom Lane
tgl@sss.pgh.pa.us

As I threatened in another thread, I've looked through all of the
oldest commitfest entries to see which ones should maybe be tossed,
on the grounds that they're unlikely to ever get committed so we
should stop pushing them forward to the next CF.

An important note to make here is that we don't have any explicit
mechanism for saying "sorry, this patch is perhaps useful but it
seems that nobody is going to take an interest in it". Closing
such a patch as "rejected" seems harsh, but R-W-F isn't very
appropriate either if the patch never got any real review.
Perhaps we should create a new closure state?

I looked at entries that are at least 10 CFs old, as indicated by
the handy sort field. That's a pretty small population: 16 items
out of the 317 listed in the 2021-09 CF. A quick look in recent
CFs shows that it's very rare that we commit entries older than
10 CFs.

Here's what I found, along with some commentary about each one.

Patch Age in CFs

Protect syscache from bloating with negative cache entries 23
Last substantive discussion 2021-01, currently passing cfbot

It's well known that I've never liked this patch, so I can't
claim to be unbiased. But what I see here is a lot of focus
on specific test scenarios with little concern for the
possibility that other scenarios will be made worse.
I think we need some new ideas to make progress.
Proposed action: RWF

Transactions involving multiple postgres foreign servers 18
Last substantive discussion 2021-07, currently failing cfbot

This has been worked on fairly recently, but frankly I'm
dubious that we want to integrate a 2PC XM into Postgres.
Proposed action: Reject

schema variables, LET command 18
Last substantive discussion 2021-09, currently passing cfbot

Seems to be actively worked on, but is it ever going to get
committed?

Remove self join on a unique column 16
Last substantive discussion 2021-07, currently passing cfbot

I'm not exactly sold that this has a good planning-cost-to-
usefulness ratio.
Proposed action: RWF

Index Skip Scan 16
Last substantive discussion 2021-05, currently passing cfbot

Seems possibly useful, but we're not making progress.

standby recovery fails when re-replaying due to missing directory which was removed in previous replay 13
Last substantive discussion 2021-09, currently passing cfbot

This is a bug fix, so we shouldn't drop it.

Remove page-read callback from XLogReaderState 12
Last substantive discussion 2021-04, currently failing cfbot

Not sure what to think about this one, but given that it
was pushed and later reverted, I'm suspicious of it.

Incremental Materialized View Maintenance 12
Last substantive discussion 2021-09, currently passing cfbot

Seems to be actively worked on.

pg_upgrade fails with non-standard ACL 12
Last substantive discussion 2021-03, currently passing cfbot

This is a bug fix, so we shouldn't drop it.

Fix up partitionwise join on how equi-join conditions between the partition keys are identified 11
Last substantive discussion 2021-07, currently passing cfbot

This is another one where I feel we need new ideas to make
progress.
Proposed action: RWF

A hook for path-removal decision on add_path 11
Last substantive discussion 2021-03, currently passing cfbot

I don't think this is a great idea: a hook there will be
costly, and it's very unclear how multiple extensions could
interact correctly.
Proposed action: Reject

Implement INSERT SET syntax 11
Last substantive discussion 2020-03, currently passing cfbot

This one is clearly stalled. I don't think it's necessarily
a bad idea, but we seem not to be very interested.
Proposed action: Reject for lack of interest

SQL:2011 application time 11
Last substantive discussion 2021-10, currently failing cfbot

Actively worked on, and it's a big feature so long gestation
isn't surprising.

WITH SYSTEM VERSIONING Temporal Tables 11
Last substantive discussion 2021-09, currently failing cfbot

Actively worked on, and it's a big feature so long gestation
isn't surprising.

psql - add SHOW_ALL_RESULTS option 11
Last substantive discussion 2021-09, currently passing cfbot

This got committed and reverted once already. I have to be
suspicious of whether this is a good design.

Split StdRdOptions into HeapOptions and ToastOptions 10
Last substantive discussion 2021-06, currently failing cfbot

I think the author has despaired of anyone else taking an
interest here. Unless somebody intends to take an interest,
we should put this one out of its misery.
Proposed action: Reject for lack of interest

Thoughts?

regards, tom lane

#2Erik Rijkers
er@xs4all.nl
In reply to: Tom Lane (#1)
Re: Triage on old commitfest entries - JSON_PATH

Op 03-10-2021 om 21:14 schreef Tom Lane:

As I threatened in another thread, I've looked through all of the
oldest commitfest entries to see which ones should maybe be tossed,
on the grounds that they're unlikely to ever get committed so we
should stop pushing them forward to the next CF.

An important note to make here is that we don't have any explicit
mechanism for saying "sorry, this patch is perhaps useful but it
seems that nobody is going to take an interest in it". Closing
such a patch as "rejected" seems harsh, but R-W-F isn't very
appropriate either if the patch never got any real review.
Perhaps we should create a new closure state?

I looked at entries that are at least 10 CFs old, as indicated by
the handy sort field. That's a pretty small population: 16 items
out of the 317 listed in the 2021-09 CF. A quick look in recent
CFs shows that it's very rare that we commit entries older than
10 CFs.

Here's what I found, along with some commentary about each one.

Patch Age in CFs

May I add one more?

SQL/JSON: JSON_TABLE started 2018 (the commitfest page shows only 4 as
'Age in CFs' but that obviously can't be right)

Although I like the patch & new functionality and Andrew Dunstan has
worked to keep it up-to-date, there seems to be very little further
discussion. I makes me a little worried that the time I put in will end
up sunk in a dead project.

Erik Rijkers

Show quoted text

Protect syscache from bloating with negative cache entries 23
Last substantive discussion 2021-01, currently passing cfbot

It's well known that I've never liked this patch, so I can't
claim to be unbiased. But what I see here is a lot of focus
on specific test scenarios with little concern for the
possibility that other scenarios will be made worse.
I think we need some new ideas to make progress.
Proposed action: RWF

Transactions involving multiple postgres foreign servers 18
Last substantive discussion 2021-07, currently failing cfbot

This has been worked on fairly recently, but frankly I'm
dubious that we want to integrate a 2PC XM into Postgres.
Proposed action: Reject

schema variables, LET command 18
Last substantive discussion 2021-09, currently passing cfbot

Seems to be actively worked on, but is it ever going to get
committed?

Remove self join on a unique column 16
Last substantive discussion 2021-07, currently passing cfbot

I'm not exactly sold that this has a good planning-cost-to-
usefulness ratio.
Proposed action: RWF

Index Skip Scan 16
Last substantive discussion 2021-05, currently passing cfbot

Seems possibly useful, but we're not making progress.

standby recovery fails when re-replaying due to missing directory which was removed in previous replay 13
Last substantive discussion 2021-09, currently passing cfbot

This is a bug fix, so we shouldn't drop it.

Remove page-read callback from XLogReaderState 12
Last substantive discussion 2021-04, currently failing cfbot

Not sure what to think about this one, but given that it
was pushed and later reverted, I'm suspicious of it.

Incremental Materialized View Maintenance 12
Last substantive discussion 2021-09, currently passing cfbot

Seems to be actively worked on.

pg_upgrade fails with non-standard ACL 12
Last substantive discussion 2021-03, currently passing cfbot

This is a bug fix, so we shouldn't drop it.

Fix up partitionwise join on how equi-join conditions between the partition keys are identified 11
Last substantive discussion 2021-07, currently passing cfbot

This is another one where I feel we need new ideas to make
progress.
Proposed action: RWF

A hook for path-removal decision on add_path 11
Last substantive discussion 2021-03, currently passing cfbot

I don't think this is a great idea: a hook there will be
costly, and it's very unclear how multiple extensions could
interact correctly.
Proposed action: Reject

Implement INSERT SET syntax 11
Last substantive discussion 2020-03, currently passing cfbot

This one is clearly stalled. I don't think it's necessarily
a bad idea, but we seem not to be very interested.
Proposed action: Reject for lack of interest

SQL:2011 application time 11
Last substantive discussion 2021-10, currently failing cfbot

Actively worked on, and it's a big feature so long gestation
isn't surprising.

WITH SYSTEM VERSIONING Temporal Tables 11
Last substantive discussion 2021-09, currently failing cfbot

Actively worked on, and it's a big feature so long gestation
isn't surprising.

psql - add SHOW_ALL_RESULTS option 11
Last substantive discussion 2021-09, currently passing cfbot

This got committed and reverted once already. I have to be
suspicious of whether this is a good design.

Split StdRdOptions into HeapOptions and ToastOptions 10
Last substantive discussion 2021-06, currently failing cfbot

I think the author has despaired of anyone else taking an
interest here. Unless somebody intends to take an interest,
we should put this one out of its misery.
Proposed action: Reject for lack of interest

Thoughts?

regards, tom lane

#3Tom Lane
tgl@sss.pgh.pa.us
In reply to: Erik Rijkers (#2)
Re: Triage on old commitfest entries - JSON_PATH

Erik Rijkers <er@xs4all.nl> writes:

Op 03-10-2021 om 21:14 schreef Tom Lane:

I looked at entries that are at least 10 CFs old, as indicated by
the handy sort field. That's a pretty small population: 16 items
out of the 317 listed in the 2021-09 CF. A quick look in recent
CFs shows that it's very rare that we commit entries older than
10 CFs.

May I add one more?

SQL/JSON: JSON_TABLE started 2018 (the commitfest page shows only 4 as
'Age in CFs' but that obviously can't be right)

Hm. It's being actively worked on, so I wouldn't have proposed
killing it even if its age had been shown correctly. Unless you
think it has no hope of ever reaching committability?

regards, tom lane

In reply to: Tom Lane (#1)
Re: Triage on old commitfest entries

On Sun, Oct 3, 2021 at 12:15 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:

An important note to make here is that we don't have any explicit
mechanism for saying "sorry, this patch is perhaps useful but it
seems that nobody is going to take an interest in it". Closing
such a patch as "rejected" seems harsh, but R-W-F isn't very
appropriate either if the patch never got any real review.
Perhaps we should create a new closure state?

We don't reject patches, except in very rare cases where the whole
concept is wildly unreasonable, or when the patch author decides to
mark their own patch rejected. In other words, we only reject patches
where the formal status of being rejected hardly matters at all. I
have to wonder what the point of the status of "rejected" really is.
Ambiguity about what the best way forward is seems to be the thing
that kills patches -- it is seldom mistakes or design problems. They
can usually be corrected easily. Sometimes the ambiguity is very
broad, other times it's just one aspect of the design (e.g., the
planner aspects).

I'd rather go in the opposite direction here: merge "Rejected" and
"Returned with Feedback" into a single "Patch Returned" category
(without adding a third category). The odds of a CF entry that gets
marked R-W-F eventually being committed is, in general, totally
unclear, or seems to be. I myself have zero faith that that status
alone predicts anything, good or bad. I think that under-specifying
why a patch has been returned like this would actually be *more*
informative. Less experienced contributors wouldn't have to waste
their time looking for some signal, when in fact there is little more
than noise.

Index Skip Scan 16
Last substantive discussion 2021-05, currently passing cfbot

Seems possibly useful, but we're not making progress.

This feature is definitely useful. My pet theory is that it hasn't
made more progress because it requires expertise in two fairly
distinct areas of the system. There is a lot of B-Tree stuff here,
which is clearly my thing. But I know that I personally am much less
likely to work on a patch that requires significant changes to the
planner. Maybe this is a coordination problem.

--
Peter Geoghegan

#5Tom Lane
tgl@sss.pgh.pa.us
In reply to: Peter Geoghegan (#4)
Re: Triage on old commitfest entries

Peter Geoghegan <pg@bowt.ie> writes:

On Sun, Oct 3, 2021 at 12:15 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:

Perhaps we should create a new closure state?

I'd rather go in the opposite direction here: merge "Rejected" and
"Returned with Feedback" into a single "Patch Returned" category
(without adding a third category).

Hm, perhaps. You're right that the classification might be slippery.
I do feel it's useful to distinguish "this is a bad idea overall,
we don't want to see follow-on patches" from "this needs work, please
send a follow-on patch when you've done the work". But maybe more
thought could get an idea out of the first category and into the
second.

Index Skip Scan 16
Last substantive discussion 2021-05, currently passing cfbot

Seems possibly useful, but we're not making progress.

This feature is definitely useful. My pet theory is that it hasn't
made more progress because it requires expertise in two fairly
distinct areas of the system. There is a lot of B-Tree stuff here,
which is clearly my thing. But I know that I personally am much less
likely to work on a patch that requires significant changes to the
planner. Maybe this is a coordination problem.

Fair. My concern here is mostly that we not just keep kicking the
can down the road. If we see that a patch has been hanging around
this long without reaching commit, we should either kill it or
form a specific plan for how to advance it.

regards, tom lane

In reply to: Tom Lane (#5)
Re: Triage on old commitfest entries

On Sun, Oct 3, 2021 at 1:30 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:

Hm, perhaps. You're right that the classification might be slippery.
I do feel it's useful to distinguish "this is a bad idea overall,
we don't want to see follow-on patches" from "this needs work, please
send a follow-on patch when you've done the work". But maybe more
thought could get an idea out of the first category and into the
second.

I agree in principle, but experience suggests that there is
approximately zero practical difference.

My whole approach is to filter aggressively. I can only speak for
myself, but I have to imagine that this is what most committers do, in
one way or another. I am focussed on what I can understand with a high
degree of confidence, that seems likely to be relatively beneficial to
users -- nothing more. So patch authors that receive no feedback from
me ought to assume that that means absolutely nothing, even in areas
where my input might be expected. I'm not saying that I *never*
mentally write-off patches without saying anything, but it's rare, and
when it happens it tends to be in the least interesting, most obvious
cases -- cases where speaking up is clearly unnecessary. I would hate
to think that less experienced patch authors are taking radio silence
as a meaningful signal, whether it's from me or from somebody else --
because it's really not like that at all.

My argument boils down to this: I think that less experienced
contributors are better served by a system that plainly admits this
uncertainty. At the same time I think that old patches need to get
bumped for the good of all patch authors collectively. We have a hard
time bumping patches today because we seem to feel the need to justify
it, based on facts about the patch. The reality has always been that
Postgres patches are rejected by default, not accepted by default. We
should be clear about this.

Fair. My concern here is mostly that we not just keep kicking the
can down the road. If we see that a patch has been hanging around
this long without reaching commit, we should either kill it or
form a specific plan for how to advance it.

Also fair.

The pandemic has made the kind of coordination I refer to harder in
practice. It's the kind of thing that face to face communication
really helps with.

--
Peter Geoghegan

#7Pavel Stehule
pavel.stehule@gmail.com
In reply to: Tom Lane (#1)
Re: Triage on old commitfest entries

Hi

schema variables, LET command 18

Last substantive discussion 2021-09, currently passing cfbot

Seems to be actively worked on, but is it ever going to get
committed?

This patch was originally very dirty with a strange design - something
between command and query. But on second hand, these issues are real and
there was a lot of work to have good performance for CALL statements and
still CALL statements is limited to using just simple expressions.

In January of this year I completely rewrote this feature (significant
part). So the implementation is very new, and I hope it can be better
included in Postgres concepts.

This feature is interesting mainly for RLS - it allows secure space in
memory, and it is available from all environments in Postgres. Second usage
can be emulation of package variables. Current emulations are very slow or
require extensions. The schema variables (session variables) can be used
badly or well. I migrated one Oracle's application, where it was an hell,
but when you do migration, then is not too much possibility for complex
redesign. I hope so this feature can be nice for users who need to write
SQL scripts, because it reduce an necessary work for pushing values to
server side. It can be used for parametrisation of "DO" blocks.

The current patch is trimmed to implementation not transactional variables,
what I think should be default behaviour (like any other databases do it).
This limit is just for reducing of necessity work with maintaining of this
patch. I have prepared patch with support transactional behaviour too (that
can have nice uses cases too). But is hard to maintain this part of patch
to be applicable every week, so I postponed this part of patch.

Regards

Pavel

#8Pavel Stehule
pavel.stehule@gmail.com
In reply to: Tom Lane (#3)
Re: Triage on old commitfest entries - JSON_PATH

ne 3. 10. 2021 v 22:16 odesílatel Tom Lane <tgl@sss.pgh.pa.us> napsal:

Erik Rijkers <er@xs4all.nl> writes:

Op 03-10-2021 om 21:14 schreef Tom Lane:

I looked at entries that are at least 10 CFs old, as indicated by
the handy sort field. That's a pretty small population: 16 items
out of the 317 listed in the 2021-09 CF. A quick look in recent
CFs shows that it's very rare that we commit entries older than
10 CFs.

May I add one more?

SQL/JSON: JSON_TABLE started 2018 (the commitfest page shows only 4 as
'Age in CFs' but that obviously can't be right)

Hm. It's being actively worked on, so I wouldn't have proposed
killing it even if its age had been shown correctly. Unless you
think it has no hope of ever reaching committability?

This is a pretty important feature and a nice patch.

Unfortunately, it is a pretty complex patch - JSON_TABLE is a really
complex function, and this patch does complete implementation. I checked
this patch more times, and I think it is good. There is only one problem -
the size (there are not any problems in code, or in behaviour) . In MySQL
or MariaDB, there is a much more simple implementation, that covers maybe
10% of standard. But it is available, and people can use it. Isn't it
possible to reduce this patch to some basic functionality, and commit it
quickly, and later commit step by step all parts.

Regards

Pavel

Show quoted text

regards, tom lane

#9Fabien COELHO
coelho@cri.ensmp.fr
In reply to: Tom Lane (#1)
Re: Triage on old commitfest entries

Hello Tom,

As I threatened in another thread, I've looked through all of the
oldest commitfest entries to see which ones should maybe be tossed,
on the grounds that they're unlikely to ever get committed so we
should stop pushing them forward to the next CF.

psql - add SHOW_ALL_RESULTS option 11
Last substantive discussion 2021-09, currently passing cfbot

This got committed and reverted once already. I have to be
suspicious of whether this is a good design.

Thoughts?

ISTM that the main problem with this patch is that it touches a barely
tested piece of software, aka "psql":-( The second problem is that the
initial code is fragile because it handles different modes with pretty
intricate code.

So, on the first commit it broke a few untested things, among the many
untested things.

This resulted in more tests being added (sql, tap) so that the relevant
features are covered, so my point of view is that this patch is currently
a net improvement both from an engineering perspective and for features
and enabling other features. Also, there is some interest to get it in.

So I do not think that it deserves to be dropped.

--
Fabien.

#10Jaime Casanova
jcasanov@systemguards.com.ec
In reply to: Tom Lane (#1)
Re: Triage on old commitfest entries

On Sun, Oct 03, 2021 at 03:14:58PM -0400, Tom Lane wrote:
[...]

Here's what I found, along with some commentary about each one.

Patch Age in CFs

Protect syscache from bloating with negative cache entries 23
Last substantive discussion 2021-01, currently passing cfbot

It's well known that I've never liked this patch, so I can't
claim to be unbiased. But what I see here is a lot of focus
on specific test scenarios with little concern for the
possibility that other scenarios will be made worse.
I think we need some new ideas to make progress.
Proposed action: RWF

if we RwF this patch we should add the thread to the TODO entry
it refers to

Transactions involving multiple postgres foreign servers 18
Last substantive discussion 2021-07, currently failing cfbot

This has been worked on fairly recently, but frankly I'm
dubious that we want to integrate a 2PC XM into Postgres.
Proposed action: Reject

Masahiko has marked the patch as RwF already

schema variables, LET command 18
Last substantive discussion 2021-09, currently passing cfbot

Seems to be actively worked on, but is it ever going to get
committed?

I had already moved this to Next CF when I read this, but I found this
sounds useful

Remove self join on a unique column 16
Last substantive discussion 2021-07, currently passing cfbot

I'm not exactly sold that this has a good planning-cost-to-
usefulness ratio.
Proposed action: RWF

It seems there is no proof that this will increase performance in the
thread.
David you're reviewer on this patch, what your opinion on this is?

Index Skip Scan 16
Last substantive discussion 2021-05, currently passing cfbot

Seems possibly useful, but we're not making progress.

Peter G mentioned this would be useful. What we need to advance this
one?

standby recovery fails when re-replaying due to missing directory which was removed in previous replay 13
Last substantive discussion 2021-09, currently passing cfbot

This is a bug fix, so we shouldn't drop it.

Moved to Next CF

Remove page-read callback from XLogReaderState 12
Last substantive discussion 2021-04, currently failing cfbot

Not sure what to think about this one, but given that it
was pushed and later reverted, I'm suspicious of it.

I guess those are enough for a decision: marked as RwF
If this is useful a new patch would be sent.

Incremental Materialized View Maintenance 12
Last substantive discussion 2021-09, currently passing cfbot

Seems to be actively worked on.

Moved to Next CF

pg_upgrade fails with non-standard ACL 12
Last substantive discussion 2021-03, currently passing cfbot

This is a bug fix, so we shouldn't drop it.

Moved to Next CF

Fix up partitionwise join on how equi-join conditions between the partition keys are identified 11
Last substantive discussion 2021-07, currently passing cfbot

This is another one where I feel we need new ideas to make
progress.
Proposed action: RWF

It seems there has been no activity since last version of the patch so I
don't think RwF is correct. What do we need to advance on this one?

A hook for path-removal decision on add_path 11
Last substantive discussion 2021-03, currently passing cfbot

I don't think this is a great idea: a hook there will be
costly, and it's very unclear how multiple extensions could
interact correctly.
Proposed action: Reject

Any other comments on this one?

Implement INSERT SET syntax 11
Last substantive discussion 2020-03, currently passing cfbot

This one is clearly stalled. I don't think it's necessarily
a bad idea, but we seem not to be very interested.
Proposed action: Reject for lack of interest

Again, no activity after last patch.

SQL:2011 application time 11
Last substantive discussion 2021-10, currently failing cfbot

Actively worked on, and it's a big feature so long gestation
isn't surprising.

Moved to Next CF

WITH SYSTEM VERSIONING Temporal Tables 11
Last substantive discussion 2021-09, currently failing cfbot

Actively worked on, and it's a big feature so long gestation
isn't surprising.

Moved to Next CF

psql - add SHOW_ALL_RESULTS option 11
Last substantive discussion 2021-09, currently passing cfbot

This got committed and reverted once already. I have to be
suspicious of whether this is a good design.

No activity after last patch

Split StdRdOptions into HeapOptions and ToastOptions 10
Last substantive discussion 2021-06, currently failing cfbot

I think the author has despaired of anyone else taking an
interest here. Unless somebody intends to take an interest,
we should put this one out of its misery.
Proposed action: Reject for lack of interest

The author of the patch claimed that a rebased version should happen at
mid-august but it hasn't happened. RwF seems reasonable to me, done
that.

--
Jaime Casanova
Director de Servicios Profesionales
SystemGuards - Consultores de PostgreSQL

#11Andrew Dunstan
andrew@dunslane.net
In reply to: Erik Rijkers (#2)
Re: Triage on old commitfest entries - JSON_PATH

On 10/3/21 3:56 PM, Erik Rijkers wrote:

Op 03-10-2021 om 21:14 schreef Tom Lane:

As I threatened in another thread, I've looked through all of the
oldest commitfest entries to see which ones should maybe be tossed,
on the grounds that they're unlikely to ever get committed so we
should stop pushing them forward to the next CF.

An important note to make here is that we don't have any explicit
mechanism for saying "sorry, this patch is perhaps useful but it
seems that nobody is going to take an interest in it".  Closing
such a patch as "rejected" seems harsh, but R-W-F isn't very
appropriate either if the patch never got any real review.
Perhaps we should create a new closure state?

I looked at entries that are at least 10 CFs old, as indicated by
the handy sort field.  That's a pretty small population: 16 items
out of the 317 listed in the 2021-09 CF.  A quick look in recent
CFs shows that it's very rare that we commit entries older than
10 CFs.

Here's what I found, along with some commentary about each one.

Patch        Age in CFs

May I add one more?

SQL/JSON: JSON_TABLE started 2018 (the commitfest page shows only 4 as
'Age in CFs' but that obviously can't be right)

Although I like the patch & new functionality and Andrew Dunstan has
worked to keep it up-to-date, there seems to be very little further
discussion. I makes me a little worried that the time I put in will
end up sunk in a dead project.

I'm working on the first piece of it, i.e. the SQL/JSON functions.

cheers

andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

#12Jesper Pedersen
jpederse@redhat.com
In reply to: Peter Geoghegan (#4)
Re: Triage on old commitfest entries

On 10/3/21 16:18, Peter Geoghegan wrote:

Index Skip Scan 16
Last substantive discussion 2021-05, currently passing cfbot

Seems possibly useful, but we're not making progress.

This feature is definitely useful. My pet theory is that it hasn't
made more progress because it requires expertise in two fairly
distinct areas of the system. There is a lot of B-Tree stuff here,
which is clearly my thing. But I know that I personally am much less
likely to work on a patch that requires significant changes to the
planner. Maybe this is a coordination problem.

I still believe that this is an important user-visible improvement.

However, there has been conflicting feedback on the necessary planner
changes leading to doing double work in order to figure the best way
forward.

Dmitry and Andy are doing a good job on keeping the patches current, but
maybe there needs to be a firm decision from a committer on what the
planner changes should look like before these patches can move forward.

So, is RfC the best state for that ?

Best regards,

 Jesper

#13Jesper Pedersen
jesper.pedersen@redhat.com
In reply to: Peter Geoghegan (#4)
Re: Triage on old commitfest entries

Hi,

On 10/3/21 16:18, Peter Geoghegan wrote:

Index Skip Scan 16
Last substantive discussion 2021-05, currently passing cfbot

Seems possibly useful, but we're not making progress.

This feature is definitely useful. My pet theory is that it hasn't
made more progress because it requires expertise in two fairly
distinct areas of the system. There is a lot of B-Tree stuff here,
which is clearly my thing. But I know that I personally am much less
likely to work on a patch that requires significant changes to the
planner. Maybe this is a coordination problem.

I still believe that this is an important user-visible improvement.

However, there has been conflicting feedback on the necessary planner
changes leading to doing double work in order to figure the best way
forward.

Dmitry and Andy are doing a good job on keeping the patches current, but
maybe there needs to be a firm decision from a committer on what the
planner changes should look like before these patches can move forward.

So, is RfC the best state for that ?

Best regards,

 Jesper

#14Erik Rijkers
er@xs4all.nl
In reply to: Andrew Dunstan (#11)
Re: Triage on old commitfest entries - JSON_PATH

Op 04-10-2021 om 14:19 schreef Andrew Dunstan:

On 10/3/21 3:56 PM, Erik Rijkers wrote:

Op 03-10-2021 om 21:14 schreef Tom Lane:

As I threatened in another thread, I've looked through all of the
oldest commitfest entries to see which ones should maybe be tossed,

Patch        Age in CFs

May I add one more?

SQL/JSON: JSON_TABLE started 2018 (the commitfest page shows only 4 as
'Age in CFs' but that obviously can't be right)

Although I like the patch & new functionality and Andrew Dunstan has
worked to keep it up-to-date, there seems to be very little further
discussion. I makes me a little worried that the time I put in will
end up sunk in a dead project.

I'm working on the first piece of it, i.e. the SQL/JSON functions.

Thank you. I am glad to hear that.

Show quoted text

cheers

andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

#15Stephen Frost
sfrost@snowman.net
In reply to: Peter Geoghegan (#6)
Re: Triage on old commitfest entries

Greetings,

* Peter Geoghegan (pg@bowt.ie) wrote:

On Sun, Oct 3, 2021 at 1:30 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:

Fair. My concern here is mostly that we not just keep kicking the
can down the road. If we see that a patch has been hanging around
this long without reaching commit, we should either kill it or
form a specific plan for how to advance it.

Also fair.

The pandemic has made the kind of coordination I refer to harder in
practice. It's the kind of thing that face to face communication
really helps with.

Entirely agree with this. Index skip scan is actually *ridiculously*
useful in terms of an improvement, and we need to get the right people
together to work on it and get it implemented. I'd love to see this
done for v15, in particular. Who do we need to coordinate getting
together to make it happen? I doubt that I'm alone in wanting to make
this happen and I'd be pretty surprised if we weren't able to bring the
right folks together this fall to make it a reality.

Thanks,

Stephen

#16Tom Lane
tgl@sss.pgh.pa.us
In reply to: Stephen Frost (#15)
Re: Triage on old commitfest entries

Stephen Frost <sfrost@snowman.net> writes:

Entirely agree with this. Index skip scan is actually *ridiculously*
useful in terms of an improvement, and we need to get the right people
together to work on it and get it implemented. I'd love to see this
done for v15, in particular. Who do we need to coordinate getting
together to make it happen?

It sounds like Peter is willing to take point on the executor end
of things (b-tree in particular). If he can explain what a reasonable
cost model would look like, I'm willing to see about making that happen
in the planner.

regards, tom lane

In reply to: Tom Lane (#16)
Re: Triage on old commitfest entries

On Mon, Oct 4, 2021 at 7:45 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:

It sounds like Peter is willing to take point on the executor end
of things (b-tree in particular). If he can explain what a reasonable
cost model would look like, I'm willing to see about making that happen
in the planner.

I would be happy to work with you on this. It's clearly an important project.

Having you involved with the core planner aspects (as well as general
design questions) significantly derisks everything. That's *very*
valuable to me.

--
Peter Geoghegan

#18Vik Fearing
vik@postgresfriends.org
In reply to: Stephen Frost (#15)
Re: Triage on old commitfest entries

On 10/5/21 4:29 AM, Stephen Frost wrote:

Greetings,

* Peter Geoghegan (pg@bowt.ie) wrote:

On Sun, Oct 3, 2021 at 1:30 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:

Fair. My concern here is mostly that we not just keep kicking the
can down the road. If we see that a patch has been hanging around
this long without reaching commit, we should either kill it or
form a specific plan for how to advance it.

Also fair.

The pandemic has made the kind of coordination I refer to harder in
practice. It's the kind of thing that face to face communication
really helps with.

Entirely agree with this. Index skip scan is actually *ridiculously*
useful in terms of an improvement, and we need to get the right people
together to work on it and get it implemented. I'd love to see this
done for v15, in particular. Who do we need to coordinate getting
together to make it happen? I doubt that I'm alone in wanting to make
this happen and I'd be pretty surprised if we weren't able to bring the
right folks together this fall to make it a reality.

I don't have the skills to work on either side of this, but I would like
to voice my support in favor of having this feature and I would be happy
to help test it on a user level (as opposed to reviewing code).
--
Vik Fearing

#19Jaime Casanova
jcasanov@systemguards.com.ec
In reply to: Jaime Casanova (#10)
Re: Triage on old commitfest entries

On Mon, Oct 04, 2021 at 02:12:49AM -0500, Jaime Casanova wrote:

On Sun, Oct 03, 2021 at 03:14:58PM -0400, Tom Lane wrote:
[...]

Here's what I found, along with some commentary about each one.

Patch Age in CFs

Protect syscache from bloating with negative cache entries 23
Last substantive discussion 2021-01, currently passing cfbot

It's well known that I've never liked this patch, so I can't
claim to be unbiased. But what I see here is a lot of focus
on specific test scenarios with little concern for the
possibility that other scenarios will be made worse.
I think we need some new ideas to make progress.
Proposed action: RWF

if we RwF this patch we should add the thread to the TODO entry
it refers to

done this way

Remove self join on a unique column 16
Last substantive discussion 2021-07, currently passing cfbot

I'm not exactly sold that this has a good planning-cost-to-
usefulness ratio.
Proposed action: RWF

It seems there is no proof that this will increase performance in the
thread.
David you're reviewer on this patch, what your opinion on this is?

The last action here was a rebased patch.
So, I will try to follow on this one and will make some performance an
functional tests.
Based on that, I will move this to the next CF and put myself as
reviewer.
But of course, I will be happy if some committer/more experienced dev
could look at the design/planner bits.

Index Skip Scan 16
Last substantive discussion 2021-05, currently passing cfbot

Seems possibly useful, but we're not making progress.

Peter G mentioned this would be useful. What we need to advance this
one?

Moved to next CF based on several comments

Fix up partitionwise join on how equi-join conditions between the partition keys are identified 11
Last substantive discussion 2021-07, currently passing cfbot

This is another one where I feel we need new ideas to make
progress.
Proposed action: RWF

It seems there has been no activity since last version of the patch so I
don't think RwF is correct. What do we need to advance on this one?

Ok. You're a reviewer in that patch and know the problems that we
mere mortals are not able to understand.

So will do as you suggest, and then will write to Richard to send the
new version he was talking about in a new entry in the CF

A hook for path-removal decision on add_path 11
Last substantive discussion 2021-03, currently passing cfbot

I don't think this is a great idea: a hook there will be
costly, and it's very unclear how multiple extensions could
interact correctly.
Proposed action: Reject

Any other comments on this one?

Will do as you suggest

Implement INSERT SET syntax 11
Last substantive discussion 2020-03, currently passing cfbot

This one is clearly stalled. I don't think it's necessarily
a bad idea, but we seem not to be very interested.
Proposed action: Reject for lack of interest

Again, no activity after last patch.

I'm not a fan of not SQL Standard syntax but seems there were some
interest on this.
And will follow this one as reviewer.

psql - add SHOW_ALL_RESULTS option 11
Last substantive discussion 2021-09, currently passing cfbot

This got committed and reverted once already. I have to be
suspicious of whether this is a good design.

No activity after last patch

Moved to next CF

--
Jaime Casanova
Director de Servicios Profesionales
SystemGuards - Consultores de PostgreSQL

#20Robert Haas
robertmhaas@gmail.com
In reply to: Jaime Casanova (#19)
Re: Triage on old commitfest entries

On Tue, Oct 5, 2021 at 12:56 PM Jaime Casanova
<jcasanov@systemguards.com.ec> wrote:

psql - add SHOW_ALL_RESULTS option 11
Last substantive discussion 2021-09, currently passing cfbot

This got committed and reverted once already. I have to be
suspicious of whether this is a good design.

No activity after last patch

Moved to next CF

This seems like the kind of thing we should not do. Patches without
activity need to be aggressively booted out of the system. Otherwise
they just generate a lot of noise that makes it harder to identify
patches that should be reviewed and perhaps committed.

--
Robert Haas
EDB: http://www.enterprisedb.com