Thread: Commitfest 2022-03 Patch Triage Part 1b
> 2096: psql - add SHOW_ALL_RESULTS option > ======================================== > Peter posted an updated version of Fabiens patch about a month ago (which at > this point no longer applies) fixing a few issues, but also point at old review > comments still unaddressed. Since this was pushed, but had to be reverted, I > assume there is a desire for the feature but it seems to need more work still. It looks like Peter and Fabien were debating the merits of a libpq change and probably that won't happen this release cycle. Is there a kernel of psql functionality that can be extracted from this without the libpq change in this release cycle or should it wait until we add the functionality to libpq? If it's the latter then perhaps we should move this to 16? > 1651: GROUP BY optimization > =========================== > This is IMO a desired optimization, and after all the heavy lifting by Tomas > the patch seems to be in pretty good shape. This is two patches and it sounds like the first one is ready for committer whereas the second one is less clear. Or is the second one meant to be an alternative for the first one? > > 2377: pg_ls_* functions for showing metadata and recurse (pg_ls_tmpdir to show > shared filesets) > ============================================================================== > The question of what to do with lstat() on Windows is still left unanswered, > but the patchset has been split to up to be able to avoid it. Stephen and Tom, > having done prior reviews do you have any thoughts on this? Is this still blocked on lstat for windows? I couldn't tell, is there consensus on a behaviour for windows even if that just means failing or returning partial results on windows? Other than that it seems like there's a lot of this patch that has positive reviews and is ready for committing. > 2349: global temporary table > ============================ > GTT has been up for discussion numerous times in tbe past, and I can't judge > whether this proposal has a better chance than previous ones. I do note the > patch has a number of crashes reported lately, and no reviews from senior > contributors in a while, making it seem unlikely to be committed in this CF. > Since the patch is very big, can it be teased apart and committed separately > for easier review? I think Andres's review decisively makes it clear this in an uncommittable state. https://www.postgresql.org/message-id/20220225074500.sfizxbmlrj2s6hp5%40alap3.anarazel.de https://www.postgresql.org/message-id/20220227041304.mnimeqkhwktrjyht%40alap3.anarazel.de It's definitely not going to make it this release and will probably need a significant amount of time next release cycle. IMHO dividing it up into smaller features does seem like it would be more effective at getting things committed. Should we mark this returned with feedback or just move it to the next commitfest as waiting on author? > 2433: Erase the distinctClause if the result is unique by definition > ==================================================================== > (parts of) The approach taken in this patch has been objected against in favor > of work that Tom has proposed. Until that work materialize this patch is > blocked, and thus I think we are better of closing it and re-opening it when it > gets unstuck. Unless Tom has plans to hack on this shortly? Ugh. This is a problematic dynamic. Tom has a different idea of what direction to take this but hasn't had a chance to work on it. So what's Andy Fan supposed to do here? He can't read Tom's mind and nobody else can really help him. Ultimately we all have limited time so this is a thing that will happen but is there anything we can do to resolve it in this case? We definitely shouldn't spend lots of time on this patch unless we're going to be ok going ahead without Tom's version of it. Is this something we can do using the Andy's data structure for now and change in the future? It looks like the Skip Scan patch was related to this work in some way? Is it blocked on it? -- greg
Greg Stark <stark@mit.edu> writes:
>> 2433: Erase the distinctClause if the result is unique by definition
>> ====================================================================
>> (parts of) The approach taken in this patch has been objected against in favor
>> of work that Tom has proposed. Until that work materialize this patch is
>> blocked, and thus I think we are better of closing it and re-opening it when it
>> gets unstuck. Unless Tom has plans to hack on this shortly?
> Ugh. This is a problematic dynamic. Tom has a different idea of what
> direction to take this but hasn't had a chance to work on it. So
> what's Andy Fan supposed to do here? He can't read Tom's mind and
> nobody else can really help him. Ultimately we all have limited time
> so this is a thing that will happen but is there anything we can do to
> resolve it in this case?
> We definitely shouldn't spend lots of time on this patch unless we're
> going to be ok going ahead without Tom's version of it. Is this
> something we can do using the Andy's data structure for now and change
> in the future?
> It looks like the Skip Scan patch was related to this work in some
> way? Is it blocked on it?
I did promise some time ago to get involved in the skip scan work.
I've so far failed to make good on that promise, but I will make
it a high priority to look at the area during this CF.
regards, tom lane
Hello Greg, >> Peter posted an updated version of Fabiens patch about a month ago (which at >> this point no longer applies) Attached a v15 which is a rebase, after some minor changes in the source and some new test cases added (good!). >> fixing a few issues, but also point at old review comments still >> unaddressed. ISTM that all comments have been addressed. However, the latest patch raises issues about work around libpq corner case behaviors which are really just that, corner cases. >> Since this was pushed, but had to be reverted, I assume there is a >> desire for the feature but it seems to need more work still. > It looks like Peter and Fabien were debating the merits of a libpq > change and probably that won't happen this release cycle. ISTM these are really very minor issues that could be resolved in this cycle. > Is there a kernel of psql functionality that can be extracted from this > without the libpq change in this release cycle or should it wait until > we add the functionality to libpq? The patch can wait for the issues to be resolved one way or an other before proceeding, *or* it can be applied, maybe with a small tweak, and the libpq issues be fixed separately. For a reminder, there are two actual "issues"features" or "bug" with libpq, which are made visible by the patch, but are pre-existing: (1) under some circumstances a infinite stream of empty results is returned, that has to be filtered out manually. (2) under some special circumstances some error messages may be output twice because of when libpq decides to reset the error buffer. (1) has been there for ever, and (2) is under investigation to possibly improve the situation, so as to remove a hack in the code to avoid it. The alternative which IMO would be ok is to admit that under some very special conditions the same error message may be output twice, and if it is resolved later on then fine. > If it's the latter then perhaps we should move this to 16? I'm not that pessimistic! I may be proven wrong:-) -- Fabien.
Attachment
Just FYI. Better to follow up to the thread for the patch that's already in the CF. Otherwise your patch will missed by someone who looks at the CF entry to see the latest patch.
> Just FYI. Better to follow up to the thread for the patch that's > already in the CF. Otherwise your patch will missed by someone who > looks at the CF entry to see the latest patch. Indeed. Done. -- Fabien.