Re: CREATE SUBSCRIPTION -- add missing tab-completes
On Fri, 7 Apr 2023 at 01:28, Amit Kapila wrote: > > On Wed, Apr 5, 2023 at 5:58 AM Peter Smith wrote: > > > PSA patches to add those tab completions. > > LGTM, so pushed. I moved this to the next CF but actually I just noticed the thread starts with the original patch being pushed. Maybe we should just close the CF entry? Is this further change relevant? -- Gregory Stark As Commitfest Manager
Re: Use fadvise in wal replay
On Thu, 19 Jan 2023 at 18:19, Andres Freund wrote: > > On 2023-01-19 22:19:10 +0100, Tomas Vondra wrote: > > > So I'm a bit unsure about this patch. I doesn't seem like it can perform > > better than read-ahead (although perhaps it does, on a different storage > > system). > > I really don't see the point of the patch as-is. ... > I don't disagree fundamentally. But that doesn't make this patch a useful > starting point. It sounds like this patch has gotten off on the wrong foot and is not worth moving forward to the next commitfest. Hopefully a starting over from a different approach might target i/o that is more amenable to fadvise. I'll mark it RwF. -- Gregory Stark As Commitfest Manager
Re: Commitfest 2023-03 starting tomorrow!
On Sat, 8 Apr 2023 at 11:37, Greg Stark wrote: > > c) Pick out the Bug Fixes, cleanup patches, and other non-feature > patches that might be open issues for v16. So on further examination it seems there are multiple category of patches that are worth holding onto rather than shifting to the next release: * Bug Fixes * Documentation patches * Build system patches, especially meson-related patches * Testing patches, especially if they're testing new features * Patches that are altering new features that were committed in this release Some of these, especially the last category, are challenging for me to determine. If I move forward a patch of yours that you think makes sense to treat as an open issue that should be resolved in this release then feel free to say. -- Gregory Stark As Commitfest Manager
Re: Remove dead macro exec_subplan_get_plan
On Fri, 16 Sept 2022 at 03:33, Zhang Mingli wrote: > > On Sep 16, 2022, 14:47 +0800, Richard Guo , wrote: > > How about add it to the CF to not lose track of it? > > Will add it, thanks~ I guess not losing track of it is only helpful if we do eventually commit it. Otherwise we would rather lose track of it :) I think the conclusion here was that the actual list is still used and cleaning up unused macros isn't worth the hassle unless it's part of some larger patch? I mean, it doesn't seem like a lot of hassle but nobody seems to have been interested in pursuing it since 2022 so I guess it's not going to happen. I don't want to keep moving patches forward release to release that nobody's interested in committing. So I'm going to mark this one rejected for now. We can always update that if it comes up again. -- Gregory Stark As Commitfest Manager
Re: logical decoding and replication of sequences, take 2
Fwiw the cfbot seems to have some failing tests with this patch: [19:05:11.398] # Failed test 'initial test data replicated' [19:05:11.398] # at t/030_sequences.pl line 75. [19:05:11.398] # got: '1|0|f' [19:05:11.398] # expected: '132|0|t' [19:05:11.398] [19:05:11.398] # Failed test 'advance sequence in rolled-back transaction' [19:05:11.398] # at t/030_sequences.pl line 98. [19:05:11.398] # got: '1|0|f' [19:05:11.398] # expected: '231|0|t' [19:05:11.398] [19:05:11.398] # Failed test 'create sequence, advance it in rolled-back transaction, but commit the create' [19:05:11.398] # at t/030_sequences.pl line 152. [19:05:11.398] # got: '1|0|f' [19:05:11.398] # expected: '132|0|t' [19:05:11.398] [19:05:11.398] # Failed test 'advance the new sequence in a transaction and roll it back' [19:05:11.398] # at t/030_sequences.pl line 175. [19:05:11.398] # got: '1|0|f' [19:05:11.398] # expected: '231|0|t' [19:05:11.398] [19:05:11.398] # Failed test 'advance sequence in a subtransaction' [19:05:11.398] # at t/030_sequences.pl line 198. [19:05:11.398] # got: '1|0|f' [19:05:11.398] # expected: '330|0|t' [19:05:11.398] # Looks like you failed 5 tests of 6. -- Gregory Stark As Commitfest Manager
Re: Improving btree performance through specializing by key shape, take 2
Hm. The cfbot has a fairly trivial issue with this with a unused variable: 18:36:17.405] In file included from ../../src/include/access/nbtree.h:1184, [18:36:17.405] from verify_nbtree.c:27: [18:36:17.405] verify_nbtree.c: In function ‘palloc_btree_page’: [18:36:17.405] ../../src/include/access/nbtree_spec.h:51:23: error: unused variable ‘__nbts_ctx’ [-Werror=unused-variable] [18:36:17.405] 51 | #define NBTS_CTX_NAME __nbts_ctx [18:36:17.405] | ^~ [18:36:17.405] ../../src/include/access/nbtree_spec.h:54:43: note: in expansion of macro ‘NBTS_CTX_NAME’ [18:36:17.405] 54 | #define NBTS_MAKE_CTX(rel) const NBTS_CTX NBTS_CTX_NAME = _nbt_spec_context(rel) [18:36:17.405] | ^ [18:36:17.405] ../../src/include/access/nbtree_spec.h:264:28: note: in expansion of macro ‘NBTS_MAKE_CTX’ [18:36:17.405] 264 | #define nbts_prep_ctx(rel) NBTS_MAKE_CTX(rel) [18:36:17.405] | ^ [18:36:17.405] verify_nbtree.c:2974:2: note: in expansion of macro ‘nbts_prep_ctx’ [18:36:17.405] 2974 | nbts_prep_ctx(NULL); [18:36:17.405] | ^
Re: Commitfest 2023-03 starting tomorrow!
Only a few more days remain before feature freeze. We've now crossed over 100 patches committed according to the CF app: Status summary: March 15March 22March 28April 4 Needs review: 152 128 116 96 Waiting on Author: 42 36 30 21 Ready for Committer: 39 32 32 35 Committed: 61 82 94 101 Moved to next CF: 4 15 17 28 Withdrawn: 17 16 18 20 Rejected: 0 5 6 8 Returned with Feedback: 4 5 6 10 Total: 319. Perhaps more importantly we've crossed *under* 100 patches waiting for review. However I tried to do a pass of the Needs Review patches and found that a lot of them were really waiting for comment and seemed to be useful features or bug fixes that already had a significant amount of work put into them and seemed likely to get committed if there was time available to work on them. There seems to be a bit of a mix of either a) patches that just never got any feedback -- in some cases presumably because the patch required special competency in a niche area or b) patches that had active discussion and patches being updated until discussion died out. Presumably because the author either got busy elsewhere or perhaps the discussion seemed unproductive and exhausted them. What I didn't see, that I expected to see, was patches that were just uninteresting to anyone other than the author but that people were just too polite to reject. So I think these patches are actual useful patches that we would want to have but are likely, modulo some bug fixes, to get moved along to the next CF again without any progress this CF: * Remove dead macro exec_subplan_get_plan * pg_rewind WAL deletion pitfall * Avoid hiding shared filesets in pg_ls_tmpdir (pg_ls_* functions for showing metadata ...) * Fix bogus error emitted by pg_recvlogical when interrupted * Lockless queue of waiters based on atomic operations for LWLock * Add sortsupport for range types and btree_gist * Enable jitlink as an alternative jit linker of legacy Rtdyld and add riscv jitting support * basebackup: support zstd long distance matching * Function to log backtrace of postgres processes * More scalable multixacts buffers and locking * COPY FROM enable FORCE_NULL/FORCE_NOT_NULL on all columns * Add semi-join pushdown to postgres_fdw * Skip replicating the tables specified in except table option * Post-special Page Storage TDE support * Direct I/O (developer-only feature) * Improve doc for autovacuum on partitioned tables * An attempt to avoid locally-committed-but-not-replicated-to-standby-transactions in synchronous replication * Check lateral references within PHVs for memoize cache keys * monitoring usage count distribution * New [relation] options engine * nbtree performance improvements through specialization on key shape * Fix assertion failure in SnapBuildInitialSnapshot() * Speed up releasing of locks * Improve pg_bsd_indent's handling of multiline initialization expressions * Refactoring postgres_fdw/connection.c * Add pg_stat_session * archive modules loose ends * Fix dsa_free() to re-bin segment * Reduce timing overhead of EXPLAIN ANALYZE using rdtsc * clean up permission checks after 599b33b94 * Fix the description of GUC "max_locks_per_transaction" and "max_pred_locks_per_transaction" in guc_table.c * some namespace.c refactoring * Add function to_oct * Switching XLog source from archive to streaming when primary available * BRIN - SK_SEARCHARRAY and scan key preprocessing * Reuse Workers and Replication Slots during Logical Replication
Re: [PATCH] Reuse Workers and Replication Slots during Logical Replication
On Sun, 26 Feb 2023 at 19:11, Melanie Plageman wrote: > > This is cool! Thanks for working on this. > I had a chance to review your patchset and I had some thoughts and > questions. It looks like this patch got a pretty solid review from Melanie Plageman in February just before the CF started. It was never set to Waiting on Author but I think that may be the right state for it. -- Gregory Stark As Commitfest Manager
Re: Patch proposal: New hooks in the connection path
This looks like it was a good discussion -- last summer. But it doesn't seem to be a patch under active development now. It sounds like there were some design constraints that still need some new ideas to solve and a new patch will be needed to address them. Should this be marked Returned With Feedback? -- Gregory Stark As Commitfest Manager
Re: Split index and table statistics into different types of stats
On Thu, 16 Mar 2023 at 05:25, Drouvot, Bertrand wrote: > > My plan was to get [1] done before resuming working on the "Split index and > table statistics into different types of stats" one. > [1]: https://commitfest.postgresql.org/42/4106/ Generate pg_stat_get_xact*() functions with Macros ([1]) was committed March 27. There's only a few days left in this CF. Would you like to leave this here? Should it be marked Needs Review or Ready for Commit? Or should we move it to the next CF now? -- Gregory Stark As Commitfest Manager
Re: WIP: Aggregation push-down - take2
It looks like in November 2022 Tomas Vondra said: > I did a quick initial review of the v20 patch series. > I plan to do a more thorough review over the next couple days, if time permits. > In general I think the patch is in pretty good shape. Following which Antonin Houska updated the patch responding to his review comments. Since then this patch has demonstrated the unfortunate "please rebase thx" followed by the author rebasing and getting no feedback until "please rebase again thx"... So while the patch doesn't currently apply it seems like it really should be either Needs Review or Ready for Commit. That said, I suspect this patch has missed the boat for this CF. Hopefully it will get more attention next release. I'll move it to the next CF but set it to Needs Review even though it needs a rebase. -- Gregory Stark As Commitfest Manager
Re: [PATCH] Introduce array_shuffle() and array_sample()
Given that there's been no updates since September 22 I'm going to make this patch Returned with Feedback. The patch can be resurrected when there's more work done -- don't forget to move it to the new CF when you do that. -- Gregory Stark As Commitfest Manager
Re: pg_stats and range statistics
On Fri, 24 Mar 2023 at 14:48, Egor Rogov wrote: > > Done. > There is one thing I'm not sure what to do about. This check: > > if (typentry->typtype != TYPTYPE_RANGE) > ereport(ERROR, > (errcode(ERRCODE_DATATYPE_MISMATCH), >errmsg("expected array of ranges"))); > > doesn't work, because the range_get_typcache() call errors out first > ("type %u is not a range type"). The message doesn't look friendly > enough for user-faced SQL function. Should we duplicate > range_get_typcache's logic and replace the error message? > Okay. I've corrected the examples a bit. It sounds like you've addressed Tomas's feedback and still have one open question. Fwiw I rebased it, it seemed to merge fine automatically. I've updated the CF entry to Needs Review. But at this late date it may have to wait until the next release. -- Gregory Stark As Commitfest Manager From 87424880f1a970448979681684e6916f33567eeb Mon Sep 17 00:00:00 2001 From: Greg Stark Date: Mon, 3 Apr 2023 17:04:11 -0400 Subject: [PATCH] pg_stats and range statistics --- doc/src/sgml/func.sgml| 36 ++ doc/src/sgml/system-views.sgml| 40 +++ src/backend/catalog/system_views.sql | 30 - src/backend/utils/adt/rangetypes_typanalyze.c | 107 ++ src/include/catalog/pg_proc.dat | 10 ++ src/test/regress/expected/rangetypes.out | 22 src/test/regress/expected/rules.out | 34 +- src/test/regress/sql/rangetypes.sql | 7 ++ 8 files changed, 284 insertions(+), 2 deletions(-) diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml index 918a492234..548078a12e 100644 --- a/doc/src/sgml/func.sgml +++ b/doc/src/sgml/func.sgml @@ -19643,6 +19643,24 @@ SELECT NULLIF(value, '(none)') ... + + + + ranges_lower + +ranges_lower ( anyarray ) +anyarray + + +Extracts lower bounds of ranges in the array (NULL if +the range is empty or the lower bound is infinite). + + +lower(ARRAY[numrange(1.1,2.2),numrange(3.3,4.4)]) +{1.1,3.3} + + + @@ -19661,6 +19679,24 @@ SELECT NULLIF(value, '(none)') ... + + + + ranges_upper + +ranges_upper ( anyarray ) +anyarray + + +Extracts upper bounds of ranges (NULL if the +range is empty or the upper bound is infinite). + + +upper(ARRAY[numrange(1.1,2.2),numrange(3.3,4.4)]) +{2.2,4.4} + + + diff --git a/doc/src/sgml/system-views.sgml b/doc/src/sgml/system-views.sgml index bb1a418450..d7760838ae 100644 --- a/doc/src/sgml/system-views.sgml +++ b/doc/src/sgml/system-views.sgml @@ -3784,6 +3784,46 @@ SELECT * FROM pg_locks pl LEFT JOIN pg_prepared_xacts ppx non-null elements. (Null for scalar types.) + + + + range_length_histogram anyarray + + + A histogram of the lengths of non-empty and non-null range values of a + range type column. (Null for non-range types.) + + + + + + range_empty_frac float4 + + + Fraction of column entries whose values are empty ranges. + (Null for non-range types.) + + + + + + range_lower_histogram anyarray + + + A histogram of lower bounds of non-empty and non-null range values. + (Null for non-range types.) + + + + + + range_upper_histogram anyarray + + + A histogram of upper bounds of non-empty and non-null range values. + (Null for non-range types.) + + diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql index 574cbc2e44..3fb7ee448f 100644 --- a/src/backend/catalog/system_views.sql +++ b/src/backend/catalog/system_views.sql @@ -243,7 +243,35 @@ CREATE VIEW pg_stats WITH (security_barrier) AS WHEN stakind3 = 5 THEN stanumbers3 WHEN stakind4 = 5 THEN stanumbers4 WHEN stakind5 = 5 THEN stanumbers5 -END AS elem_count_histogram +END AS elem_count_histogram, +CASE +WHEN stakind1 = 6 THEN stanumbers1[1] +WHEN stakind2 = 6 THEN stanumbers2[1] +WHEN stakind3 = 6 THEN stanumbers3[1] +WHEN stakind4 = 6 THEN stanumbers4[1] +WHEN stakind5 = 6 THEN stanumbers5[1] +END AS range_empty_frac, +CASE +WHEN stakind1 = 6 THEN stavalues1 +WHEN stakind2 = 6 THEN stavalues2 +WHEN stakind3 = 6 THEN stavalues3 +WHEN stakind4 = 6 THEN stavalues4 +WHEN stakind5 = 6 THEN stavalues5 +END AS range_length_histogram, +CASE +
Re: pg_usleep for multisecond delays
On Mon, 13 Mar 2023 at 17:17, Nathan Bossart wrote: > > On Fri, Mar 10, 2023 at 12:28:28PM -0500, Tom Lane wrote: > > A quick grep for pg_usleep with large intervals finds rather more > > than you touched: > > > > [...] > > > > Did you have reasons for excluding the rest of these? > > I'm still looking into each of these, but my main concerns were 1) ensuring > latch support had been set up and 2) figuring out the right interrupt > function to use. Thus far, I don't think latch support is a problem > because InitializeLatchSupport() is called quite early. However, I'm not > as confident with the interrupt functions yet. Some of these multisecond > sleeps are done very early before the signal handlers are set up. Others > are done within the sigsetjmp() block. And at least one is in a code path > that both the startup process and single-user mode touch. Is this still a WIP? Is it targeting this release? There are only a few days left before the feature freeze. I'm guessing it should just move to the next CF for the next release? -- Gregory Stark As Commitfest Manager
Re: Removing unneeded self joins
On Mon, 6 Mar 2023 at 00:30, Michał Kłeczek wrote: > > Hi All, > > I just wanted to ask about the status and plans for this patch. > I can see it being stuck at “Waiting for Author” status in several commit > tests. Sadly it seems to now be badly in need of a rebase. There are large hunks failing in the guts of analyzejoins.c as well as minor failures elsewhere and lots of offsets which need to be reviewed. I think given the lack of activity it's out of time for this release at this point. I'm moving it ahead to the next CF. -- Gregory Stark As Commitfest Manager
Re: psql - factor out echo code
On Mon, 13 Feb 2023 at 05:41, Peter Eisentraut wrote: > > I think this patch requires an up-to-date summary and explanation. The > thread is over a year old and the patch has evolved quite a bit. There > are some test changes that are not explained. Please provide more > detail so that the patch can be considered. Given this feedback I'm going to mark this Returned with Feedback. I think it'll be clearer to start with a new thread explaining the intent of the patch as it is now. -- Gregory Stark As Commitfest Manager
Re: Prefetch the next tuple's memory during seqscans
On Sun, 29 Jan 2023 at 21:24, David Rowley wrote: > > I've moved this patch to the next CF. This patch has a dependency on > what's being proposed in [1]. The referenced patch was committed March 19th but there's been no comment here. Is this patch likely to go ahead this release or should I move it forward again? -- Gregory Stark As Commitfest Manager
Re: [PATCH]Feature improvement for MERGE tab completion
Ah, another thread with a bouncing email address... Please respond to to thread from this point to avoid bounces. -- Gregory Stark As Commitfest Manager
Re: [PATCH]Feature improvement for MERGE tab completion
It looks like this remaining work isn't going to happen this CF and therefore this release. There hasn't been an update since January when Dean Rasheed posted a review. Unless there's any updates soon I'll move this on to the next commitfest or mark it returned with feedback. -- Gregory Stark As Commitfest Manager
Re: CAST(... ON DEFAULT) - WIP build on top of Error-Safe User Functions
On Tue, 3 Jan 2023 at 14:16, Tom Lane wrote: > > Vik Fearing writes: > > > I don't think we should add that syntax until I do get it through the > > committee, just in case they change something. > > Agreed. So this is something we won't be able to put into v16; > it'll have to wait till there's something solid from the committee. I guess I'll mark this Rejected in the CF then. Who knows when the SQL committee will look at this... -- Gregory Stark As Commitfest Manager
Re: Request for comment on setting binary format output per session
FYI I attached this thread to https://commitfest.postgresql.org/42/3777 which I believe is the same issue. I mistakenly had this listed as a CF entry with no discussion for a long time due to that missing link. -- Gregory Stark As Commitfest Manager
Re: POC: Better infrastructure for automated testing of concurrency issues
On Wed, 31 Aug 2022 at 20:52, Alexander Korotkov wrote: > > I'll continue work on this patch. The rebased patch is attached. It > implements stop events as configure option (not runtime GUC option). It looks like this patch isn't going to be ready this commitfest. And it hasn't received much discussion since August 2022. If I'm wrong say something but otherwise I'll mark it Returned With Feedback. It can be resurrected (and moved to the next commitfest) when you're free to work on it again. -- Gregory Stark As Commitfest Manager
Re: Commitfest 2023-03 starting tomorrow!
Status summary: Needs review: 116. Waiting on Author: 30. Ready for Committer: 32. Committed: 94. Moved to next CF: 17. Returned with Feedback: 6. Rejected: 6. Withdrawn: 18. Total: 319. Ok, here are the patches that have been stuck in "Waiting on Author" for a while. I divided them into three groups. * The first group have been stuck for over a week and mostly look like they should be RwF. Some I guess just moved to next CF. But some of them I'm uncertain if I should leave or if they really should be RfC or NR. * The other two groups have had some updates in the last week (actually I used 10 days). But some of them still look like they're pretty much dead for this CF and should either be moved forward or Rwf or Rejected. So here's the triage list. I'm going to send emails and start clearing out the patches pretty much right away. Some of these are pretty clearcut. Nothing in over a week: -- * Better infrastructure for automated testing of concurrency issues - Consensus that this is desirable. But it's not clear what it's actually waiting on Author for. RwF? * Provide the facility to set binary format output for specific OID's per session - I think Dave was looking for feedback and got it from Tom and Peter. I don't actually see a specific patch here but there are two patches linked in the original message. There seems to be enough feedback to proceed but nobody's working on it. RwF? * pg_visibility's pg_check_visible() yields false positive when working in parallel with autovacuum - Bug, but tentatively a false positive... * CAST( ... ON DEFAULT) - it'll have to wait till there's something solid from the committee" -- Rejected? * Fix tab completion MERGE - Partly committed but v9-0002-psql-Add-PartialMatches-macro-for-better-tab-complet.patch remains. There was a review from Dean Rasheed. Move to next CF? * Fix recovery conflict SIGUSR1 handling - This looks like a suite of bug fixes and looks like it should be Needs Review or Ready for Commit * Prefetch the next tuple's memory during seqscans - David Rowley said it was dependeny on "heapgettup() refactoring" which has been refactored. So is it now Needs Review or Ready for Commit? Is it likely to happen this CF? * Pluggable toaster - This seems to have digressed from the original patch. There were patches early on and a lot of feedback. Is the result that the original patches are Rejected or are they still live? * psql - refactor echo code - "I think this patch requires an up-to-date summary and explanation" from Peter. But it seems like Tom was ok with it and just had some additional improvements he wanted that were added. It sounds like this might be "Ready for Commit" if someone familiar with the patch looked at it. * Push aggregation down to base relations and joins - Needs a significant rebase (since March 1). * Remove self join on a unique column - An offer of a Bounty! There was one failing test which was apparently fixed? But it looks like this should be in Needs Review or Ready for Commit. * Split index and table statistics into different types of stats - Was stuck on "Generate pg_stat_get_xact*() functions with Macros" which was committed. So "Ready for Commit" now? * Default to ICU during initdb - Partly committed, 0001 waiting until after CF * suppressing useless wakeups in logical/worker.c - Got feedback March 17. Doesn't look like it's going to be ready this CF. * explain analyze rows=%.0f - Patch updated January, but I think Tom still had some simple if tedious changes he asked for * Fix order of checking ICU options in initdb and create database - Feedback Last November but no further questions or progress * Introduce array_shuffle() and array_sample() functions - Feedback from Tom last September. No further questions or progress Status Updates in last week: * Some revises in adding sorting path - Got feedback Feb 21 and author responded but doesn't look like it's going to be ready this CF * Add TAP tests for psql \g piped into program - Peter Eisentraut asked for a one-line change, otherwise it looks like it's Ready for Commit? * Improvements to Meson docs - Some feedback March 15 but no response. I assume this is still in play Emails in last week: --- * RADIUS tests and improvements - last feedback March 20, last patch March 4. Should probably be moved to the next CF unless there's progress soon. * Direct SSL Connections - (This is mine) Code for SSL is pretty finished. The last patch for ALPN support needs a bit of polish. I'll be doing that momentarily. * Fix alter subscription concurrency errors - "patch as-submitted is pretty uninteresting" and "patch that I don't care much about" ... I guess this is Rejected or Withdrawn * Fix improper qual pushdown after applying outer join identity 3 - Tom Lane's patch. Active discussion as of
Re: Move backup-related code to xlogbackup.c/.h
On Wed, 26 Oct 2022 at 02:08, Bharath Rupireddy wrote: > > I'm attaching the v9 patch set herewith after rebasing. Please review > it further. It looks like neither reviewer has been really convinced this is the direction they want to go and I think that's why the thread has been pretty dead since last October. I think people are pretty hesitant to give bad news but I don't think we're doing you any favours having you rebasing and rebasing and trying to justify specific code changes when it looks like people are skeptical about the basic approach. So I'm going to mark this Rejected for now. Perhaps a fresh approach next release cycle starting with a discussion of the specific goals rather than starting with a patch would be better. -- Gregory Stark As Commitfest Manager
Re: Consider parallel for lateral subqueries with limit
This patch has been "Needs Review" and bouncing from CF to CF. It actually looks like it got quite a bit of design discussion and while James Coleman seems convinced of its safety it doesn't sound like Tom Lane and Robert Haas are yet and it doesn't look like that's going to happen in this CF. I think I'm going to mark this Returned With Feedback on the basis that it did actually get a significant amount of discussion and no further review seems to be forthcoming. Perhaps a more rigorous approach of proving the correctness or perhaps an easier-to-reason-about set of constraints would make it easier to reach a consensus? Once you think the patchset is ready for review again, you (or any interested party) can resurrect the patch entry by visiting https://commitfest.postgresql.org/42/2851/ and changing the status to "Needs Review", and then changing the status again to "Move to next CF". (Don't forget the second step; hopefully we will have streamlined this in the near future!) -- Gregory Stark As Commitfest Manager
Re: [PATCH] Optional OR REPLACE in CREATE OPERATOR statement
On Tue, 5 Jul 2022 at 11:29, Tom Lane wrote: > > No, that's not acceptable. CREATE OR REPLACE should always produce > exactly the same final state of the object, but in this case we cannot > change the underlying function if the operator already exists. It sounds like this patch isn't the direction to go in. I don't know if IF NOT EXISTS is better but that design discussion should probably happen after this commitfest. I'm sorry but I guess I'll mark this patch Rejected. -- Gregory Stark As Commitfest Manager
Re: Raising the SCRAM iteration count
On Tue, 14 Mar 2023 at 14:54, Gregory Stark (as CFM) wrote: > > CFBot is failing with this test failure... I'm not sure if this just > represents a timing dependency or a bad test or what? CFBot is now consistently showing these test failures. I think there might actually be a problem here? > [09:44:49.937] --- stdout --- > [09:44:49.937] # executing test in > /tmp/cirrus-ci-build/build-32/testrun/authentication/001_password > group authentication test 001_password > [09:44:49.937] ok 1 - scram_iterations in server side ROLE > [09:44:49.937] # test failed > [09:44:49.937] --- stderr --- > [09:44:49.937] # Tests were run but no plan was declared and > done_testing() was not seen. > [09:44:49.937] # Looks like your test exited with 2 just after 1. > [09:44:49.937] > [09:44:49.937] (test program exited with status code 2) > > It looks like perhaps a Perl issue? > > # Running: /tmp/cirrus-ci-build/build-32/src/test/regress/pg_regress > --config-auth > /tmp/cirrus-ci-build/build-32/testrun/authentication/001_password/data/t_001_password_primary_data/pgdata > ### Starting node "primary" > # Running: pg_ctl -w -D > /tmp/cirrus-ci-build/build-32/testrun/authentication/001_password/data/t_001_password_primary_data/pgdata > -l > /tmp/cirrus-ci-build/build-32/testrun/authentication/001_password/log/001_password_primary.log > -o --cluster-name=primary start > waiting for server to start done > server started > # Postmaster PID for node "primary" is 66930 > [09:44:07.411](1.875s) ok 1 - scram_iterations in server side ROLE > Can't locate IO/Pty.pm in @INC (you may need to install the IO::Pty > module) (@INC contains: /tmp/cirrus-ci-build/src/test/perl > /tmp/cirrus-ci-build/src/test/authentication /etc/perl > /usr/local/lib/i386-linux-gnu/perl/5.32.1 /usr/local/share/perl/5.32.1 > /usr/lib/i386-linux-gnu/perl5/5.32 /usr/share/perl5 > /usr/lib/i386-linux-gnu/perl/5.32 /usr/share/perl/5.32 > /usr/local/lib/site_perl) at /usr/share/perl5/IPC/Run.pm line 1828. > Unexpected SCALAR(0x5814b508) in harness() parameter 3 at > /tmp/cirrus-ci-build/src/test/perl/PostgreSQL/Test/Cluster.pm line > 2112. > Can't locate IO/Pty.pm in @INC (you may need to install the IO::Pty > module) (@INC contains: /tmp/cirrus-ci-build/src/test/perl > /tmp/cirrus-ci-build/src/test/authentication /etc/perl > /usr/local/lib/i386-linux-gnu/perl/5.32.1 /usr/local/share/perl/5.32.1 > /usr/lib/i386-linux-gnu/perl5/5.32 /usr/share/perl5 > /usr/lib/i386-linux-gnu/perl/5.32 /usr/share/perl/5.32 > /usr/local/lib/site_perl) at /usr/share/perl5/IPC/Run.pm line 1939. > # Postmaster PID for node "primary" is 66930 > ### Stopping node "primary" using mode immediate > # Running: pg_ctl -D > /tmp/cirrus-ci-build/build-32/testrun/authentication/001_password/data/t_001_password_primary_data/pgdata > -m immediate stop > waiting for server to shut down done > server stopped > # No postmaster PID for node "primary" > [09:44:07.521](0.110s) # Tests were run but no plan was declared and > done_testing() was not seen. > [09:44:07.521](0.000s) # Looks like your test exited with 2 just after 1. -- Gregory Stark As Commitfest Manager
Re: [PATCH] Add <> support to sepgsql_restorecon
> Ok, sounds reasonable. Maybe just add a comment to that effect. > This needs regression test support for the feature and some minimal > documentation that shows how to make use of it. Hm. It sounds like this patch is uncontroversial but is missing documentation and tests? Has this been addressed? Do you think you'll get a chance to resolve those issues this month in time for this release? -- Gregory Stark As Commitfest Manager
Re: Inconsistency in reporting checkpointer stats
On Sun, 19 Feb 2023 at 04:58, Nitin Jadhav wrote: > > > This doesn't pass the tests, because the regression tests weren't adjusted: > > https://api.cirrus-ci.com/v1/artifact/task/5937624817336320/testrun/build/testrun/regress/regress/regression.diffs > > Thanks for sharing this. I have fixed this in the patch attached. AFAICS this patch was marked Waiting on Author due to this feedback on Feb 14 but the patch was updated Feb 19 and is still passing today. I've marked it Needs Review. I'm not sure if all of the feedback from Bharath Rupireddy and Kyotaro Horiguchi was addressed. If there's any specific questions remaining you need feedback on it would be good to ask explicitly. Otherwise if you think it's ready you could mark it Ready for Commit. -- Gregory Stark As Commitfest Manager
Re: Optimizing Node Files Support
I think it's clear we aren't going to be taking this patch in its current form. Perhaps there are better ways to do what these files do (I sure think there are!) but I don't think microoptimizing the copying is something people are super excited about. It sounds like rethinking how to make these functions more convenient for programmers to maintain reliably would be more valuable. I guess I'll mark it Returned with Feedback -- if there are significant performance gains to show without making the code harder to maintain and/or a nicer way to structure this code in general then we can revisit this. -- Gregory Stark As Commitfest Manager
Re: [PATCH] Introduce array_shuffle() and array_sample()
On Thu, 29 Sept 2022 at 15:34, Tom Lane wrote: > > Martin Kalcher writes: > > New patch: array_shuffle() and array_sample() use pg_global_prng_state now. > > I took a closer look at the patch today. I find this behavior a bit > surprising: > It looks like this patch received useful feedback and it wouldn't take much to push it over the line. But it's been Waiting On Author since last September. Martin, any chance of getting these last bits of feedback resolved so it can be Ready for Commit? -- Gregory Stark As Commitfest Manager
Re: [PATCH] Add initial xid/mxid/mxoff to initdb
On Fri, 25 Nov 2022 at 07:22, Peter Eisentraut wrote: > > Just for completeness, over in the other thread the feedback was that > this functionality is better put into pg_resetwal. So is that other thread tracked in a different commitfest entry and this one completely redundant? I'll mark it Rejected then? -- Gregory Stark As Commitfest Manager
Re: pg_stats and range statistics
On Sun, 22 Jan 2023 at 18:22, Tomas Vondra wrote: > > I wonder if we have other functions doing something similar, i.e. > accepting a polymorphic type and then imposing additional restrictions > on it. Meh, there's things like array comparison functions that require both arguments to be the same kind of arrays. And array_agg that requires the elements to be the same type as the state array (ie, same type as the first element). Not sure there are any taking just one specific type though. > > Shouldn't this add some sql tests ? > > Yeah, I guess we should have a couple tests calling these functions on > different range arrays. > > This reminds me lower()/upper() have some extra rules about handling > empty ranges / infinite boundaries etc. These functions should behave > consistently (as if we called lower() in a loop) and I'm pretty sure > that's not the current state. Are we still waiting on these two items? Egor, do you think you'll have a chance to work it for this month? -- Gregory Stark As Commitfest Manager
Re: Eliminating SPI from RI triggers - take 2
On Mon, 17 Oct 2022 at 14:59, Robert Haas wrote: > > On Sat, Oct 15, 2022 at 1:47 AM Amit Langote wrote: > > But I think the bigger problem for this patch set is that the > design-level feedback from > https://www.postgresql.org/message-id/CA%2BTgmoaiTNj4DgQy42OT9JmTTP1NWcMV%2Bke0i%3D%2Ba7%3DVgnzqGXw%40mail.gmail.com > hasn't really been addressed, AFAICS. ri_LookupKeyInPkRelPlanIsValid > is still trivial in v7, and that still seems wrong to me. And I still > don't know how we're going to avoid changing the semantics in ways > that are undesirable, or even knowing precisely what we did change. If > we don't have answers to those questions, then I suspect that this > patch set isn't going anywhere. Amit, do you plan to work on this patch for this commitfest (and therefore this release?). And do you think it has a realistic chance of being ready for commit this month? It looks to me like you have some good feedback and can progress and are unlikely to finish this patch for this release. In which case maybe we can move it forward to the next release? -- Gregory Stark As Commitfest Manager
Re: doc: add missing "id" attributes to extension packaging page
On Thu, 26 Jan 2023 at 15:55, Brar Piening wrote: > > On 18.01.2023 at 06:50, Brar Piening wrote: > > > I'll give it a proper look this weekend. > > It turns out I didn't get a round tuit. > > ... and I'm afraid I probably will not be able to work on this until > mid/end February so we'll have to move this to the next commitfest until > somebody wants to take it over and push it through. Looks like a lot of good work was happening on this patch right up until mid-February. Is there a lot of work left? Do you think you'll have a chance to wrap it up this commitfest for this release? -- Gregory Stark As Commitfest Manager
Re: [PATCH] psql: Add tab-complete for optional view parameters
On Sun, 29 Jan 2023 at 05:20, Mikhail Gribkov wrote: > > The problem is obviously in the newly added second line of the following > clause: > COMPLETE_WITH("ALTER COLUMN", "OWNER TO", "RENAME", > "SET SCHEMA", "SET (", "RESET ("); > > "set schema" and "set (" alternatives are competing, while completion of the > common part "set" leads to a string composition which does not have > the check branch (Matches("ALTER", "VIEW", MatchAny, "SET")). > > I think it may worth looking at "alter materialized view" completion tree > and making "alter view" the same way. > > The new status of this patch is: Waiting on Author I think this patch received real feedback and it looks like it's clear where there's more work needed. I'll move this to the next commitfest. If you plan to work on it this month we can always move it back. -- Gregory Stark As Commitfest Manager
Re: Make ON_ERROR_STOP stop on shell script failure
On Mon, 20 Mar 2023 at 14:34, Gregory Stark (as CFM) wrote: > > Pruning bouncing email address -- please respond from this point in > thread, not previous messages. Oh for heaven's sake. Trying again to prune bouncing email address. Please respond from *here* on the thread Sorry -- Gregory Stark As Commitfest Manager
Re: Make ON_ERROR_STOP stop on shell script failure
Pruning bouncing email address -- please respond from this point in thread, not previous messages. -- Gregory Stark As Commitfest Manager
Re: Fix order of checking ICU options in initdb and create database
Is this feedback enough to focus the work on the right things? I feel like Marina Polyakova pointed out some real confusing behaviour and perhaps there's a way to solve them by focusing on one at a time without making large changes in the code. Perhaps an idea would be to have each module provide two functions, one which is called early and signals an error if that module's parameters are provided when it's not compiled in, and a second which verifies that the parameters are consistent at the point in time where that's appropriate. (Not entirely unlike what we do for GUCs, though simpler) If every module did that consistently then it would avoid making the changes "unprincipled" or "spaghetti" though frankly I find words like that not very helpful to someone receiving that feedback. The patch is obviously not ready for commit now but it also seems like the feedback has not been really sufficient for Marina Polyakova to make progress either. -- Gregory Stark As Commitfest Manager
Re: Error "initial slot snapshot too large" in create replication slot
On Wed, 12 Oct 2022 at 01:10, Michael Paquier wrote: > > On Tue, Sep 13, 2022 at 05:31:05PM +0900, Kyotaro Horiguchi wrote: > > And ExportSnapshot repalces oversized subxip with overflowed. > > > > So even when GetSnapshotData() returns a snapshot with oversized > > subxip, it is saved as just "overflowed" on exporting. I don't think > > this is the expected behavior since such (no xip and overflowed) > > snapshot no longer works. > > > > On the other hand, it seems to me that snapbuild doesn't like > > takenDuringRecovery snapshots. > > > > So snapshot needs additional flag signals that xip is oversized and > > all xid are holded there. And also need to let takenDuringRecovery > > suggest subxip is oversized. > > The discussion seems to have stopped here. As this is classified as a > bug fix, I have moved this patch to next CF, waiting on author for the > moment. Kyotoro Horiguchi, any chance you'll be able to work on this for this commitfest? If so shout (or anyone else is planning to push it over the line Andres?) otherwise I'll move it on to the next release. -- Gregory Stark As Commitfest Manager
Re: [PATCH] Fix alter subscription concurrency errors
On Mon, 16 Jan 2023 at 09:47, vignesh C wrote: > > Jeite, please post an updated version with the fixes. As CommitFest > 2023-01 is currently underway, this would be an excellent time to > update the patch. Hm. This patch is still waiting on updates. But it's a bug fix and it would be good to get this in. Is someone else interested in finishing this if Jeite isn't available? -- Gregory Stark As Commitfest Manager
Re: real/float example for testlibpq3
On Mon, 23 Jan 2023 at 11:54, Mark Wong wrote: fficient way to communicate useful information. > > Yeah, I will try to cover all the data types we ship. :) I'll keep at > it and drop the code examples. I assume this isn't going to happen for this commitfest? If you think it is then shout otherwise I'll mark it Returned with Feedback and move it on to the next release. -- Gregory Stark As Commitfest Manager
Re: Allow parallel plan for referential integrity checks?
On Mon, 12 Dec 2022 at 11:37, Frédéric Yhuel wrote: > > I've planned to work on it full time on week 10 (6-10 March), if you > agree to bear with me. The idea would be to bootstrap my brain on it, > and then continue to work on it from time to time. Any updates on this patch? Should we move it to next release at this point? Even if you get time to work on it this commitfest do you think it's likely to be committable in the next few weeks? -- Gregory Stark As Commitfest Manager
Re: explain analyze rows=%.0f
On Wed, 4 Jan 2023 at 10:05, Ibrar Ahmed wrote: > > Thanks, I have modified everything as suggested, except one point > > > Don't use variable format strings. They're hard to read and they > > probably defeat compile-time checks that the arguments match the > > format string. You could use a "*" field width instead, ie ... > > * Another thought is that the non-text formats tend to prize output > > consistency over readability, so maybe we should just always use 2 > > fractional digits there, rather than trying to minimize visible changes. > > In that, we need to adjust a lot of test case outputs. > > * We need some doc adjustments, surely, to explain what the heck this > > means. That sounds like three points :) But they seem like pretty good arguments to me and straightforward if a little tedious to adjust. This patch was marked Returned with Feedback and then later Waiting on Author. And it hasn't had any updates since January. What is the state on this feedback? If it's already done we can set the patch to Ready for Commit and if not do you disagree with the proposed changes? It's actually the kind of code cleanup changes I'm reluctant to bump a patch for. It's not like a committer can't make these kinds of changes when committing. But there are so many patches they're likely to just focus on a different patch when there are adjustments like this pending. -- Gregory Stark As Commitfest Manager
Re: Commitfest 2023-03 starting tomorrow!
So, sorry I've been a bit under the weather but can concentrate on the commitfest now. I tried to recapitulate the history but the activity log only goes back a certain distance on the web. If I can log in to the database I guess I could construct the history from sql queries if it was important. But where we stand now is: Status summary: Needs review: 152. Waiting on Author: 42. Ready for Committer: 39. Committed: 61. Moved to next CF: 4. Withdrawn: 17. Returned with Feedback: 4. Total: 319. Of the Needs Review patches there are 81 that have received no email messages since the CF began. A lot of those have reviews attached but presumably those reviewers are mostly from earlier CFs and may have already contributed all they can. I don't know, should we move some or all of these to the next CF already? I'm reluctant to bounce them en masse because there are definitely some patches that will get reviewed and some that should really be marked "Ready for Committer". These patches that are "Needs Review" and have received no comments at all since before March 1st are these. If your patch is amongst this list I would suggest any of: 1) Move it yourself to the next CF (or withdraw it) 2) Post to the list with any pending questions asking for specific feedback -- it's much more likely to get feedback than just a generic "here's a patch plz review"... 3) Mark it Ready for Committer and possibly post explaining the resolution to any earlier questions to make it easier for a committer to understand the state If there's still no emails on these at some point I suppose it will make sense to move them out of the CF. * ALTER TABLE SET ACCESS METHOD on partitioned tables * New hooks in the connection path * Add log messages when replication slots become active and inactive * Avoid use deprecated Windows Memory API * Remove dead macro exec_subplan_get_plan * Consider parallel for LATERAL subqueries having LIMIT/OFFSET * pg_rewind WAL deletion pitfall * Simplify find_my_exec by using realpath(3) * Move backup-related code to xlogbackup.c/.h * Avoid hiding shared filesets in pg_ls_tmpdir (pg_ls_* functions for showing metadata ...) * Fix bogus error emitted by pg_recvlogical when interrupted * warn if GUC set to an invalid shared library * Check consistency of GUC defaults between .sample.conf and pg_settings.boot_val * Code checks for App Devs, using new options for transaction behavior * Lockless queue of waiters based on atomic operations for LWLock * Fix assertion failure with next_phase_at in snapbuild.c * Add SPLIT PARTITION/MERGE PARTITIONS commands * Add sortsupport for range types and btree_gist * asynchronous execution support for Custom Scan * Periodic burst growth of the checkpoint_req counter on replica. * CREATE INDEX CONCURRENTLY on partitioned table * Fix ParamPathInfo for union-all AppendPath * Add OR REPLACE option for CREATE OPERATOR * ALTER TABLE and CLUSTER fail to use a BulkInsertState for toast tables * Partial aggregates push down * Non-replayable WAL records through overflows and >MaxAllocSize lengths * Enable jitlink as an alternative jit linker of legacy Rtdyld and add riscv jitting support * Test for function error in logrep worker * basebackup: support zstd long distance matching * pgbench - adding pl/pgsql versions of tests * Function to log backtrace of postgres processes * More scalable multixacts buffers and locking * Remove nonmeaningful prefixes in PgStat_* fields * COPY FROM enable FORCE_NULL/FORCE_NOT_NULL on all columns * postgres_fdw: commit remote (sub)transactions in parallel during pre-commit * Add semi-join pushdown to postgres_fdw * Skip replicating the tables specified in except table option * Split index and table statistics into different types of stats * Exclusion constraints on partitioned tables * Post-special Page Storage TDE support * Direct I/O (developer-only feature) * Improve doc for autovacuum on partitioned tables * Patch to implement missing join selectivity estimation for range types * Clarify the behavior of the system when approaching XID wraparound * Set arbitrary GUC options during initdb * An attempt to avoid locally-committed-but-not-replicated-to-standby-transactions in synchronous replication * Check lateral references within PHVs for memoize cache keys * Add n_tup_newpage_upd to pg_stat table views * monitoring usage count distribution * Reduce wakeup on idle for bgwriter & walwriter for >5s * Report the query string that caused a memory error under Valgrind * New [relation] options engine * Data is copied twice when specifying both child and parent table in publication * possibility to take name, signature and oid of currently executed function in GET DIAGNOSTICS statement * Named Operators * nbtree performance improvements through specialization on key shape * Fix assertion failure in SnapBuildInitialSnapshot() * Speed up releasing of locks * Compression dictionaries *
Re: On login trigger: take three
It looks like the patch is failing a test by not dumping login_event_trigger_func? I'm guessing there's a race condition in the test but I don't know. I also don't see the tmp_test_jI6t output file being preserved in the artifacts anywhere :( https://cirrus-ci.com/task/6391161871400960?logs=test_world#L2671 [16:16:48.594] # Looks like you failed 1 test of 10350. # Running: pg_dump --no-sync --file=/tmp/cirrus-ci-build/src/bin/pg_dump/tmp_check/tmp_test_jI6t/only_dump_measurement.sql --table-and-children=dump_test.measurement --lock-wait-timeout=18 postgres [16:16:27.027](0.154s) ok 6765 - only_dump_measurement: pg_dump runs . [16:16:27.035](0.000s) not ok 6870 - only_dump_measurement: should dump CREATE FUNCTION dump_test.login_event_trigger_func [16:16:27.035](0.000s) [16:16:27.035](0.000s) # Failed test 'only_dump_measurement: should dump CREATE FUNCTION dump_test.login_event_trigger_func' # at t/002_pg_dump.pl line 4761. . [16:16:48.594] +++ tap check in src/bin/pg_dump +++ [16:16:48.594] [16:16:05] t/001_basic.pl ok 612 ms ( 0.01 usr 0.00 sys + 0.24 cusr 0.26 csys = 0.51 CPU) [16:16:48.594] [16:16:48.594] # Failed test 'only_dump_measurement: should dump CREATE FUNCTION dump_test.login_event_trigger_func' [16:16:48.594] # at t/002_pg_dump.pl line 4761. [16:16:48.594] # Review only_dump_measurement results in /tmp/cirrus-ci-build/src/bin/pg_dump/tmp_check/tmp_test_jI6t
Re: Raising the SCRAM iteration count
CFBot is failing with this test failure... I'm not sure if this just represents a timing dependency or a bad test or what? [09:44:49.937] --- stdout --- [09:44:49.937] # executing test in /tmp/cirrus-ci-build/build-32/testrun/authentication/001_password group authentication test 001_password [09:44:49.937] ok 1 - scram_iterations in server side ROLE [09:44:49.937] # test failed [09:44:49.937] --- stderr --- [09:44:49.937] # Tests were run but no plan was declared and done_testing() was not seen. [09:44:49.937] # Looks like your test exited with 2 just after 1. [09:44:49.937] [09:44:49.937] (test program exited with status code 2) It looks like perhaps a Perl issue? # Running: /tmp/cirrus-ci-build/build-32/src/test/regress/pg_regress --config-auth /tmp/cirrus-ci-build/build-32/testrun/authentication/001_password/data/t_001_password_primary_data/pgdata ### Starting node "primary" # Running: pg_ctl -w -D /tmp/cirrus-ci-build/build-32/testrun/authentication/001_password/data/t_001_password_primary_data/pgdata -l /tmp/cirrus-ci-build/build-32/testrun/authentication/001_password/log/001_password_primary.log -o --cluster-name=primary start waiting for server to start done server started # Postmaster PID for node "primary" is 66930 [09:44:07.411](1.875s) ok 1 - scram_iterations in server side ROLE Can't locate IO/Pty.pm in @INC (you may need to install the IO::Pty module) (@INC contains: /tmp/cirrus-ci-build/src/test/perl /tmp/cirrus-ci-build/src/test/authentication /etc/perl /usr/local/lib/i386-linux-gnu/perl/5.32.1 /usr/local/share/perl/5.32.1 /usr/lib/i386-linux-gnu/perl5/5.32 /usr/share/perl5 /usr/lib/i386-linux-gnu/perl/5.32 /usr/share/perl/5.32 /usr/local/lib/site_perl) at /usr/share/perl5/IPC/Run.pm line 1828. Unexpected SCALAR(0x5814b508) in harness() parameter 3 at /tmp/cirrus-ci-build/src/test/perl/PostgreSQL/Test/Cluster.pm line 2112. Can't locate IO/Pty.pm in @INC (you may need to install the IO::Pty module) (@INC contains: /tmp/cirrus-ci-build/src/test/perl /tmp/cirrus-ci-build/src/test/authentication /etc/perl /usr/local/lib/i386-linux-gnu/perl/5.32.1 /usr/local/share/perl/5.32.1 /usr/lib/i386-linux-gnu/perl5/5.32 /usr/share/perl5 /usr/lib/i386-linux-gnu/perl/5.32 /usr/share/perl/5.32 /usr/local/lib/site_perl) at /usr/share/perl5/IPC/Run.pm line 1939. # Postmaster PID for node "primary" is 66930 ### Stopping node "primary" using mode immediate # Running: pg_ctl -D /tmp/cirrus-ci-build/build-32/testrun/authentication/001_password/data/t_001_password_primary_data/pgdata -m immediate stop waiting for server to shut down done server stopped # No postmaster PID for node "primary" [09:44:07.521](0.110s) # Tests were run but no plan was declared and done_testing() was not seen. [09:44:07.521](0.000s) # Looks like your test exited with 2 just after 1.
Re: Privileges on PUBLICATION
FYI this looks like it needs a rebase due to a conflict in copy.c and an offset in pgoutput.c. Is there anything specific that still needs review or do you think you've handled all Peter's concerns? In particular, is there "a comprehensive description of what it is trying to do"? :)
Re: Using each rel as both outer and inner for JOIN_ANTI
So what is the status of this patch? It looks like you received some feedback from Emre, Tom, Ronan, and Alvaro but it also looks like you responded to most or all of that. Are you still blocked waiting for feedback? Anything specific you need help with? Or is the patch ready for commit now? In which case it would be good to rebase it since it's currently failing to apply. Well it would be good to rebase regardless but it would be especially important if we want to get it committed :)
Re: pg_stat_statements and "IN" conditions
So I was seeing that this patch needs a rebase according to cfbot. However it looks like the review feedback you're looking for is more of design questions. What jumbling is best to include in the feature set and which is best to add in later patches. It sounds like you've gotten conflicting feedback from initial reviews. It does sound like the patch is pretty mature and you're actively responding to feedback so if you got more authoritative feedback it might even be committable now. It's already been two years of being rolled forward so it would be a shame to keep rolling it forward. Or is there some fatal problem that you're trying to work around and still haven't found the magic combination that convinces any committers this is something we want? In which case perhaps we set this patch returned? I don't get that impression myself though.
Re: [EXTERNAL] Re: Support load balancing in libpq
The pgindent run in b6dfee28f is causing this patch to need a rebase for the cfbot to apply it.
Re: [PATCH] Add `verify-system` sslmode to use system CA pool for server cert
It does look like a rebase for meson.build would be helpful. I'm not marking it waiting on author just for meson.build conflicts but it would be perhaps more likely to be picked up by a committer if it's showing green in cfbot.
Re: [EXTERNAL] Re: Add non-blocking version of PQcancel
It looks like this needs a big rebase in fea-uth.c fe-auth-scram.c and fe-connect.c. Every hunk is failing which perhaps means the code you're patching has been moved or refactored?
Re: On login trigger: take three
It looks like Daniel Gustafsson, Andres, and Tom have all weighed in on this patch with at least a neutral comment (+-0 from Andres :) It looks like the main concern was breaking physical replicas and that there was consensus that as long as single-user mode worked that it was ok? So maybe it's time after 2 1/2 years to get this one committed? -- Gregory Stark As Commitfest Manager
Re: proposal: possibility to read dumped table's name from file
So This patch has been through a lot of commitfests. And it really doesn't seem that hard to resolve -- Pavel has seemingly been willing to go along whichever way the wind has been blowing but honestly it kind of seems like he's just gotten drive-by suggestions and he's put a lot of work into trying to satisfy them. He implemented --include-tables-from-file=... etc. Then he implemented a hand-written parser for a DSL to select objects, then he implemented a bison parser, then he went back to the hand-written parser. Can we get some consensus on whether the DSL looks right and whether the hand-written parser is sensible. And if so then can a committer step up to actual review and commit the patch? The last review said it might need a native English speaker to tweak some wording but otherwise looked good. -- Gregory Stark As Commitfest Manager
Re: Add a hook to allow modification of the ldapbindpasswd
The CFBot says this patch is failing but I find it hard to believe this is related to this patch... 2023-03-05 20:56:58.705 UTC [33902][client backend] [pg_regress/btree_index][18/750:0] STATEMENT: ALTER INDEX btree_part_idx ALTER COLUMN id SET (n_distinct=100); 2023-03-05 20:56:58.709 UTC [33902][client backend] [pg_regress/btree_index][:0] LOG: disconnection: session time: 0:00:02.287 user=postgres database=regression host=[local] 2023-03-05 20:56:58.710 UTC [33889][client backend] [pg_regress/join][:0] LOG: disconnection: session time: 0:00:02.289 user=postgres database=regression host=[local] 2023-03-05 20:56:58.749 UTC [33045][postmaster] LOG: server process (PID 33898) was terminated by signal 6: Abort trap 2023-03-05 20:56:58.749 UTC [33045][postmaster] DETAIL: Failed process was running: SELECT * FROM writetest; 2023-03-05 20:56:58.749 UTC [33045][postmaster] LOG: terminating any other active server processes -- Gregory Stark As Commitfest Manager
Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements
I'm sorry, It seems this is failing again? It's Makefile and meson.build again :( Though there are also patching file contrib/pg_stat_statements/sql/oldextversions.sql can't find file to patch at input line 1833 and |--- a/contrib/pg_stat_statements/sql/pg_stat_statements.sql |+++ b/contrib/pg_stat_statements/sql/pg_stat_statements.sql -- No file to patch. Skipping patch. -- Gregory Stark As Commitfest Manager
Re: Make mesage at end-of-recovery less scary.
It looks like this needs a rebase and at a quick glance it looks like more than a trivial conflict. I'll mark it Waiting on Author. Please update it back when it's rebased -- Gregory Stark As Commitfest Manager
Re: [PATCH] Support % wildcard in extension upgrade filenames
This patch too is conflicting on meson.build. Are these two patches interdependent? This one looks a bit more controversial. I'm not sure if Tom has been convinced and I haven't seen anyone else speak up. Perhaps this needs a bit more discussion of other options to solve this issue. Maybe it can just be solved with multiple one-line scripts that call to a master script? -- Gregory Stark As Commitfest Manager
Re: Ability to reference other extensions by schema in extension scripts
It looks like this patch needs a quick rebase, there's a conflict in the meson.build. I'll leave the state since presumably this would be easy to resolve but it would be more likely to get attention if it's actually building cleanly. http://cfbot.cputube.org/patch_42_4023.log On Tue, 28 Feb 2023 at 18:44, Sandro Santilli wrote: > > The following review has been posted through the commitfest application: > make installcheck-world: tested, failed > Implements feature: tested, passed > Spec compliant: tested, passed > Documentation:tested, passed > > I've applied the patch attached to message > https://www.postgresql.org/message-id/000401d94bc8%2448dff700%24da9fe500%24%40pcorp.us > (md5sum a7d45a32c054919d94cd4a26d7d07c20) onto current tip of the master > branch being 128dd9f9eca0b633b51ffcd5b0f798fbc48ec4c0 > > The review written in > https://www.postgresql.org/message-id/20230228224608.ak7br5shev4wic5a%40c19 > all still applies. > > The `make installcheck-world` test fails for me but the failures seem > unrelated to the patch (many occurrences of "+ERROR: function > pg_input_error_info(unknown, unknown) does not exist" in the regression.diff). > > Documentation exists for the new feature > > The new status of this patch is: Ready for Committer -- Gregory Stark As Commitfest Manager
Re: Commitfest 2023-03 starting tomorrow!
Sorry, I wasn't feeling very well since Friday. I'm still not 100% but I'm going to try to do some triage this afternoon. There are a few patches that need a rebase. And a few patches failing Meson builds or autoconf stages -- I wonder if there's something unrelated broken there? But what I think is really needed is for committers to pick up patches that are ready to commit and grab them. There are currently two patches with macdice marked as committer and one with michael-kun (i.e. you:) But what can we do to get more some patches picked up now instead of at the end of the commitfest? Would it help if I started asking on existing threads if there's a committer willing to take it up? On Wed, 1 Mar 2023 at 20:27, Michael Paquier wrote: > > On Wed, Mar 01, 2023 at 03:47:17PM +0900, Michael Paquier wrote: > > The CF would begin in more or less 5 hours as of the moment of this > > message: > > https://www.timeanddate.com/time/zones/aoe > > Note: I have switched this CF as "In Process" a few hours ago. > -- > Michael -- Gregory Stark As Commitfest Manager
Re: WIP: Aggregation push-down - take2
On Thu, 5 Jan 2023 at 02:59, Antonin Houska wrote: > > vignesh C wrote: > > > The patch does not apply on top of HEAD as in [1], please post a rebased > > patch: And again... Setting this to Waiting on Author for the moment. Do you think this patch is likely to be ready for this release or the next one? Is there specific feedback you're looking for? patching file src/backend/optimizer/util/relnode.c Hunk #1 FAILED at 18. Hunk #2 succeeded at 85 (offset 8 lines). Hunk #3 succeeded at 405 with fuzz 1 (offset 25 lines). Hunk #4 succeeded at 595 (offset 63 lines). Hunk #5 succeeded at 657 (offset 63 lines). Hunk #6 succeeded at 692 (offset 63 lines). Hunk #7 succeeded at 731 (offset 63 lines). Hunk #8 succeeded at 849 (offset 62 lines). Hunk #9 succeeded at 860 (offset 62 lines). Hunk #10 succeeded at 873 (offset 62 lines). Hunk #11 FAILED at 911. Hunk #12 FAILED at 945. Hunk #13 succeeded at 2585 (offset 310 lines). 3 out of 13 hunks FAILED -- saving rejects to file src/backend/optimizer/util/relnode.c.rej patching file src/backend/optimizer/util/tlist.c patching file src/backend/utils/misc/guc_tables.c Hunk #1 succeeded at 946 (offset 1 line). patching file src/backend/utils/misc/postgresql.conf.sample Hunk #1 succeeded at 390 (offset 2 lines). patching file src/include/nodes/pathnodes.h Hunk #1 succeeded at 386 (offset 10 lines). Hunk #2 succeeded at 429 (offset 10 lines). Hunk #3 succeeded at 477 (offset 38 lines). Hunk #4 succeeded at 1084 (offset 37 lines). Hunk #5 succeeded at 3117 (offset 146 lines). patching file src/include/optimizer/clauses.h patching file src/include/optimizer/pathnode.h Hunk #2 FAILED at 311. Hunk #3 FAILED at 344. 2 out of 3 hunks FAILED -- saving rejects to file src/include/optimizer/pathnode.h.rej -- Gregory Stark As Commitfest Manager
Re: Operation log for major operations
On Thu, 19 Jan 2023 at 16:12, Dmitry Koval wrote: > > >The patch does not apply on top of HEAD ... > > Thanks! > Here is a fixed version. Sorry to say, but this needs a rebase again... Setting to Waiting on Author... Are there specific feedback needed to make progress? Once it's rebased if you think it's ready set it to Ready for Committer or if you still need feedback then Needs Review -- but it's usually more helpful to do that with an email expressing what questions you're blocked on. -- Gregory Stark As Commitfest Manager
Re: File descriptors in exec'd subprocesses
On Mon, 20 Feb 2023 at 23:04, Thomas Munro wrote: > > Done like that in this version. This is the version I'm thinking of > committing, unless someone wants to argue for another level. FWIW the cfbot doesn't understand this patch series. I'm not sure why but it's only trying to apply the first (the MacOS one) and it's failing to apply even that. -- Gregory Stark As Commitfest Manager
Re: Logical replication timeout problem
On Wed, 8 Feb 2023 at 15:04, Andres Freund wrote: > > Attached is a current, quite rough, prototype. It addresses some of the points > raised, but far from all. There's also several XXXs/FIXMEs in it. I changed > the file-ending to .txt to avoid hijacking the CF entry. It looks like this patch has received quite a generous helping of feedback from Andres. I'm setting it to Waiting on Author. On the one hand it looks like there's a lot of work to do on this but on the other hand it sounds like this is a live problem in the field so if it can get done in time for release that would be great but if not then feel free to move it to the next commitfest (which means next release). -- Gregory Stark As Commitfest Manager
Re: Infinite Interval
On Sun, 15 Jan 2023 at 11:44, Joseph Koshakow wrote: > > On Sat, Jan 14, 2023 at 4:22 PM Joseph Koshakow wrote: > I've gone ahead and updated the patch to only look at the months field. > I'll submit this email and patch to the Feb commitfest. It looks like this patch needs a (perhaps trivial) rebase. It sounds like all the design questions are resolved so perhaps this can be set to Ready for Committer once it's rebased? -- Gregory Stark As Commitfest Manager
Re: In-placre persistance change of a relation
On Mon, 6 Feb 2023 at 23:48, Kyotaro Horiguchi wrote: > > Thank you for the comment! > > At Fri, 3 Feb 2023 08:42:52 +0100, Heikki Linnakangas wrote > in > > I want to call out this part of this patch: Looks like this patch has received some solid feedback from Heikki and you have a path forward. It's not currently building in the build farm either. I'll set the patch to Waiting on Author for now. -- Gregory Stark As Commitfest Manager
Re: Generate pg_stat_get_xact*() functions with Macros
Looks like you have a path forward on this and it's not ready to commit yet? In which case I'll mark it Waiting on Author? On Fri, 13 Jan 2023 at 13:38, Andres Freund wrote: > > On 2023-01-13 10:36:49 +0100, Drouvot, Bertrand wrote: > > > I'll first look at 1). > > Makes sense. > > > And it looks to me that removing PgStat_BackendFunctionEntry can be done > > independently > > It's imo the function version of 1), just a bit simpler to implement due to > the much simpler reconciliation. It could be done together with it, or > separately. -- Gregory Stark As Commitfest Manager
Re: [EXTERNAL] Re: Add non-blocking version of PQcancel
On Wed, 1 Mar 2023 at 14:48, Jelte Fennema wrote: > > > This looks like it needs a rebase. > > done Great. Please update the CF entry to Needs Review or Ready for Committer as appropriate :) -- Gregory Stark As Commitfest Manager
Re: Flush SLRU counters in checkpointer process
On Thu, 12 Jan 2023 at 03:46, Anthonin Bonnefoy wrote: > > > That would make sense. I've created a new patch with everything moved in > pgstat_report_checkpointer(). > I did split the checkpointer flush in a pgstat_flush_checkpointer() function > as it seemed more readable. Thought? This patch appears to need a rebase. Is there really any feedback needed or is it ready for committer once it's rebased? -- Gregory Stark As Commitfest Manager
Re: [PATCH] Make ON CONFLICT DO NOTHING and ON CONFLICT DO UPDATE consistent
On Thu, 9 Feb 2023 at 05:43, Aleksander Alekseev wrote: > > >> I don't buy your argument about DO UPDATE needing to be brought into > >> line with DO NOTHING. In any case I'm pretty sure that Tom's remarks > >> in 2016 about a behavioral inconsistencies (which you cited) actually > >> called for making DO NOTHING more like DO UPDATE -- not the other way > >> around. > > > > Interesting. Yep, we could use a bit of input from Tom on this one. I realize there are still unanswered conceptual questions about this patch but with two votes against it seems unlikely to make much more progress unless you rethink what you're trying to accomplish and package it in a way that doesn't step on these more controversial issues. I'm going to mark the patch Returned With Feedback. If Tom or someone else disagrees with Peter and Andres or has some new insights about how to make it more palatable then we can always revisit that. > > This of course would break backward compatibility. But we can always > > invent something like: > > > > ``` > > INSERT INTO .. > > ON CONFLICT DO [NOTHING|UPDATE .. ] > > [ALLOWING|FORBIDDING] SELF CONFLICTS; > > ``` > > > > ... if we really want to. Something like that might be what I mean about new insights though I suspect this is overly complex. It looks like having the ON CONFLICT UPDATE happen before the row is already inserted might simplify things conceptually but then it might make the implementation prohibitively complex. -- Gregory Stark As Commitfest Manager
Re: Allow +group in pg_ident.conf
On Wed, 11 Jan 2023 at 04:00, Jelte Fennema wrote: > > I'm working on a new patchset for my commitfest entry. So I'll set it to "Waiting on Author" pending that new patchset... -- Gregory Stark As Commitfest Manager
Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements
On Wed, 1 Mar 2023 at 04:04, Andrei Zubkov wrote: > > Hi! > > I've attached a new version of a patch - rebase to the current master. The CFBot (http://cfbot.cputube.org/) doesn't seem to like this. It looks like all the Meson builds are failing, perhaps there's something particular about the test environment that is either not set up right or is exposing a bug? Please check if this is a real failure or a cfbot failure. -- Gregory Stark As Commitfest Manager