Обсуждение: Triage on old commitfest entries

Поиск
Список
Период
Сортировка

Triage on old commitfest entries

От
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

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



Re: Triage on old commitfest entries - JSON_PATH

От
Erik Rijkers
Дата:
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

> 
> 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
> 
> 



Re: Triage on old commitfest entries - JSON_PATH

От
Tom Lane
Дата:
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



Re: Triage on old commitfest entries

От
Peter Geoghegan
Дата:
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



Re: Triage on old commitfest entries

От
Tom Lane
Дата:
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



Re: Triage on old commitfest entries

От
Peter Geoghegan
Дата:
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



Re: Triage on old commitfest entries

От
Pavel Stehule
Дата:
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

Re: Triage on old commitfest entries - JSON_PATH

От
Pavel Stehule
Дата:


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




                        regards, tom lane


Re: Triage on old commitfest entries

От
Fabien COELHO
Дата:
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.



Re: Triage on old commitfest entries

От
Jaime Casanova
Дата:
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



Re: Triage on old commitfest entries - JSON_PATH

От
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,
>> 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




Re: Triage on old commitfest entries

От
Jesper Pedersen
Дата:
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





Re: Triage on old commitfest entries

От
Jesper Pedersen
Дата:
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





Re: Triage on old commitfest entries - JSON_PATH

От
Erik Rijkers
Дата:
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.



> cheers
> 
> 
> andrew
> 
> 
> --
> Andrew Dunstan
> EDB: https://www.enterprisedb.com
> 



Re: Triage on old commitfest entries

От
Stephen Frost
Дата:
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

Вложения

Re: Triage on old commitfest entries

От
Tom Lane
Дата:
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



Re: Triage on old commitfest entries

От
Peter Geoghegan
Дата:
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



Re: Triage on old commitfest entries

От
Vik Fearing
Дата:
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



Re: Triage on old commitfest entries

От
Jaime Casanova
Дата:
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



Re: Triage on old commitfest entries

От
Robert Haas
Дата:
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