RE: subscription/015_stream sometimes breaks

2023-08-22 Thread Zhijie Hou (Fujitsu)
> -Original Message- > From: Zhijie Hou (Fujitsu) > Sent: Wednesday, August 23, 2023 10:27 AM > To: Thomas Munro > Cc: Amit Kapila ; pgsql-hackers > > Subject: RE: subscription/015_stream sometimes breaks > > On Wednesday, August 23, 2023 4:55 AM Thomas Munro > wrote: > > > > On

Re: Synchronizing slots from primary to standby

2023-08-22 Thread shveta malik
On Thu, Aug 17, 2023 at 11:55 AM shveta malik wrote: > > On Thu, Aug 17, 2023 at 11:44 AM Drouvot, Bertrand > wrote: > > > > Hi, > > > > On 8/14/23 11:52 AM, shveta malik wrote: > > > > > > > > We (myself and Ajin) performed the tests to compute the lag in standby > > > slots as compared to

Re: Oversight in reparameterize_path_by_child leading to executor crash

2023-08-22 Thread Richard Guo
On Tue, Aug 22, 2023 at 10:39 PM Tom Lane wrote: > Richard Guo writes: > > I'm wondering if we can instead adjust the 'inner_req_outer' in > > create_nestloop_path before we perform the check to work around this > > issue for the back branches, something like > > + /* > > +* Adjust the

Re: persist logical slots to disk during shutdown checkpoint

2023-08-22 Thread vignesh C
On Tue, 22 Aug 2023 at 15:42, Amit Kapila wrote: > > On Tue, Aug 22, 2023 at 2:56 PM Ashutosh Bapat > wrote: > > > > On Tue, Aug 22, 2023 at 9:48 AM Amit Kapila wrote: > > > > > > > > Another idea is to record the confirm_flush_lsn at the time of > > > > persisting the slot. We can use it in

RE: persist logical slots to disk during shutdown checkpoint

2023-08-22 Thread Hayato Kuroda (Fujitsu)
Dear hackers, Thanks for forking the thread! I think we would choose another design, but I wanted to post the updated version once with the current approach. All comments came from the parent thread [1]. > 1. GENERAL -- git apply > > The patch fails to apply cleanly. There are whitespace

Re: Direct I/O

2023-08-22 Thread Thomas Munro
On Wed, Aug 23, 2023 at 12:15 AM Peter Eisentraut wrote: > I suggest to also rename the hook functions (check and assign), like in > the attached patch. Mainly because utils/guc_hooks.h says to order the > functions by GUC variable name, which was already wrong under the old > name, but it would

Re: Extending SMgrRelation lifetimes

2023-08-22 Thread Thomas Munro
On Fri, Aug 18, 2023 at 2:30 AM Robert Haas wrote: > I think this direction makes a lot of sense. The lack of a defined > lifetime for SMgrRelation objects makes correct programming difficult, > makes efficient programming difficult, and doesn't really have any > advantages. Thanks for looking!

Re: Doc limitation update proposal: include out-of-line OID usage per TOAST-ed columns

2023-08-22 Thread John Naylor
On Mon, Aug 21, 2023 at 1:33 PM Gurjeet Singh wrote: > > Please see attached the proposed patch, which attempts to make that > language newcomer-friendly. The patch adds one link for TOAST, and > replaces Postgres-specific terms with generic ones. This is off-topic for this thread (which has a

Re: subscription/015_stream sometimes breaks

2023-08-22 Thread vignesh C
On Wed, 23 Aug 2023 at 02:25, Thomas Munro wrote: > > On Wed, Aug 23, 2023 at 8:21 AM Thomas Munro wrote: > > I didn't study it closely but it looks like there might be a second > > deadlock, after the one that is expected by the test? Examples from > > the past couple of weeks: > > I should

RE: [PoC] pg_upgrade: allow to upgrade publisher node

2023-08-22 Thread Hayato Kuroda (Fujitsu)
Dear Peter, Thanks for giving comments! New version can be available in [1]. > 1. +#include "access/xlogdefs.h" #include "catalog/pg_authid_d.h" Was this #include needed here? I noticed you've already included the same in the "pg_upgrade.h". > It was needed because the macro

RE: [PoC] pg_upgrade: allow to upgrade publisher node

2023-08-22 Thread Hayato Kuroda (Fujitsu)
Dear Peter, Thanks for giving comments! PSA the new version. > == 1. GENERAL Please try to run a spell/grammar check on all the text like commit message and docs changes before posting (e.g. cut/paste the rendered text into some tool like MSWord or Grammarly or ChatGPT or whatever tool

RE: [PoC] pg_upgrade: allow to upgrade publisher node

2023-08-22 Thread Hayato Kuroda (Fujitsu)
Dear Amit, Thanks for giving comment. New version will be available in the upcoming post. > + res = executeQueryOrDie(conn, "SELECT slot_name " > + "FROM pg_catalog.pg_replication_slots " > + "WHERE slot_type = 'logical' AND " > + "temporary IS FALSE;"); > + > + if (PQntuples(res)) > +

RE: [PoC] pg_upgrade: allow to upgrade publisher node

2023-08-22 Thread Hayato Kuroda (Fujitsu)
Dear Amit, Thanks for the comment! Next version will be available in upcoming post. > > > + /* logical replication slots can be migrated since PG17. */ > > > + if (GET_MAJOR_VERSION(new_cluster->major_version) <= 1600) > > > + return; > > > > > > IMO the code matches the comment better if you

RE: [PoC] pg_upgrade: allow to upgrade publisher node

2023-08-22 Thread Hayato Kuroda (Fujitsu)
Dear Peter, > Here are some review comments for v23-0001 Thanks for the comment! But I did not update 0001 patch in this thread. It will be managed in the forked one... Best Regards, Hayato Kuroda FUJITSU LIMITED

RE: [PoC] pg_upgrade: allow to upgrade publisher node

2023-08-22 Thread Hayato Kuroda (Fujitsu)
Dear Peter, Thanks for giving comments! New version will be available in the upcoming post. > 1. check_for_lost_slots + /* logical slots can be migrated since PG17. */ + if (GET_MAJOR_VERSION(cluster->major_version) <= 1600) + return; 1a Maybe the comment should start uppercase for consistency

RE: subscription/015_stream sometimes breaks

2023-08-22 Thread Zhijie Hou (Fujitsu)
On Wednesday, August 23, 2023 4:55 AM Thomas Munro wrote: > > On Wed, Aug 23, 2023 at 8:21 AM Thomas Munro > wrote: > > I didn't study it closely but it looks like there might be a second > > deadlock, after the one that is expected by the test? Examples from > > the past couple of weeks: >

Re: Using defines for protocol characters

2023-08-22 Thread Nathan Bossart
On Thu, Aug 17, 2023 at 09:52:03AM -0700, Nathan Bossart wrote: > On Thu, Aug 17, 2023 at 12:22:24PM -0400, Robert Haas wrote: >> I think this is going to be a major improvement in terms of readability! >> >> I'm pretty happy with this version, too. We may be running out of >> things to argue

Re: PG 16 draft release notes ready

2023-08-22 Thread Bruce Momjian
On Tue, Aug 22, 2023 at 01:42:41PM -0700, Jeff Davis wrote: > On Thu, 2023-05-18 at 16:49 -0400, Bruce Momjian wrote: > > I have completed the first draft of the PG 16 release notes.  You can > > see the output here: > > >

Re: Support run-time partition pruning for hash join

2023-08-22 Thread Andy Fan
> > > fwiw, the current master totally ignores the cost reduction for run-time > > partition prune, even for init partition prune. So in some real cases, > > pg chooses a hash join just because the cost of nest loop join is > > highly over estimated. > > This is true about the existing code. It's

pg_upgrade - a function parameter shadows global 'new_cluster'

2023-08-22 Thread Peter Smith
Hi hackers. During a recent review of nearby code I noticed that there was a shadowing of the 'new_cluster' global variable by a function parameter: Here: static void check_for_new_tablespace_dir(ClusterInfo *new_cluster); ~~~ It looks like it has been like this for a couple of years. I guess

Re: Support run-time partition pruning for hash join

2023-08-22 Thread Andy Fan
On Tue, Aug 22, 2023 at 5:43 PM Richard Guo wrote: > > On Mon, Aug 21, 2023 at 8:34 PM Andy Fan wrote: > >> This feature looks good, but is it possible to know if we can prune >> any subnodes before we pay the extra effort (building the Hash >> table, for each row... stuff)? >> > > It might be

Re: initdb caching during tests

2023-08-22 Thread Andres Freund
Hi, On 2023-08-22 23:47:24 +0200, Daniel Gustafsson wrote: > I had a look at this today and have been running a lot of tests with it > without > finding anything that breaks. Thanks! > The duplicated code is unfortunate, but after playing around with some > options I agree that it's likely

False "pg_serial": apparent wraparound” in logs

2023-08-22 Thread Imseih (AWS), Sami
Hi, I Recently encountered a situation on the field in which the message “could not truncate directory "pg_serial": apparent wraparound” was logged even through there was no danger of wraparound. This was on a brand new cluster and only took a few minutes to see the message in the logs. Reading

Re: Make --help output fit within 80 columns per line

2023-08-22 Thread Masahiro Ikeda
Hi, On 2023-08-22 22:57, torikoshia wrote: On 2023-08-21 13:08, Masahiro Ikeda wrote: (2) Is there any reason that only src/bin commands are targeted? I found that we also need to fix vacuumlo with the above test. I think it's better to fix it because it's a contrib module. $ vacuumlo

Re: Adding a LogicalRepWorker type field

2023-08-22 Thread Peter Smith
On Tue, Aug 22, 2023 at 6:24 PM Amit Kapila wrote: > On Mon, Aug 21, 2023 at 3:48 PM Amit Kapila > wrote: > > > > On Mon, Aug 21, 2023 at 5:34 AM Peter Smith > wrote: > > > > > > PSA v9 > > > > > > > LGTM. I'll push this tomorrow unless there are any more comments. > > > > Pushed. > Thanks

Re: Prevent psql \watch from running queries that return no rows

2023-08-22 Thread Daniel Gustafsson
> On 22 Aug 2023, at 23:23, Greg Sabino Mullane wrote: > > Thank you for the feedback, everyone. Attached is version 4 of the patch, > featuring a few tests and minor rewordings. Thanks, the changes seem good from a quick skim. I'll take a better look tomorrow to hopefully close this one. --

Re: initdb caching during tests

2023-08-22 Thread Daniel Gustafsson
> On 5 Aug 2023, at 21:56, Andres Freund wrote: > We have some issues with CI on macos and windows being too expensive (more on > that soon in a separate email), which reminded me of this thread (with > original title: [1]) > > I've attached a somewhat cleaned up version of the patch to cache

Re: Faster "SET search_path"

2023-08-22 Thread Jeff Davis
On Wed, 2023-08-16 at 15:09 -0700, Jeff Davis wrote: > To bring the overhead closer to zero we need to somehow avoid > repeating > so much work in guc.c, though. If we don't go around it, another > approach would be to separate GUC setting into two phases: one that > does the checks, and one that

Re: [17] collation provider "builtin"

2023-08-22 Thread Jeff Davis
On Wed, 2023-06-14 at 15:55 -0700, Jeff Davis wrote: > The locale "C" (and equivalently, "POSIX") is not really a libc > locale; > it's implemented internally with memcmp for collation and > pg_ascii_tolower, etc., for ctype. > > The attached patch implements a new collation provider, "builtin",

Re: BUG #18059: Unexpected error 25001 in stored procedure

2023-08-22 Thread Tom Lane
Robert Haas writes: > On Sat, Aug 19, 2023 at 1:19 PM Tom Lane wrote: >> What I'm inclined to propose, therefore, is that we make revalidation >> be a no-op for every statement type for which transformStmt() reaches >> its default: case. (When it does so, the resulting CMD_UTILITY Query >> will

Re: Prevent psql \watch from running queries that return no rows

2023-08-22 Thread Greg Sabino Mullane
Thank you for the feedback, everyone. Attached is version 4 of the patch, featuring a few tests and minor rewordings. Cheers, Greg psql_watch_exit_on_zero_rows_v4.patch Description: Binary data

Re: subscription/015_stream sometimes breaks

2023-08-22 Thread Thomas Munro
On Wed, Aug 23, 2023 at 8:21 AM Thomas Munro wrote: > I didn't study it closely but it looks like there might be a second > deadlock, after the one that is expected by the test? Examples from > the past couple of weeks: I should add, it's not correlated with the patches that cfbot is testing,

Re: Direct I/O

2023-08-22 Thread Peter Geoghegan
On Wed, Apr 19, 2023 at 10:43 AM Andres Freund wrote: > I don't think the "evict on every buffer access" works quite that way - unless > you have a completely even access pattern, buffer access frequency will > increase usage count much more frequently on some buffers than others. And if > you

Re: PG 16 draft release notes ready

2023-08-22 Thread Jeff Davis
On Thu, 2023-05-18 at 16:49 -0400, Bruce Momjian wrote: > I have completed the first draft of the PG 16 release notes.  You can > see the output here: https://www.postgresql.org/docs/16/release-16.html#RELEASE-16-LOCALIZATION I notice that this item is still listed: * Determine the ICU

subscription/015_stream sometimes breaks

2023-08-22 Thread Thomas Munro
Hi, A couple of times a day, cfbot reports an error like this: https://cirrus-ci.com/task/6424286882168832 I didn't study it closely but it looks like there might be a second deadlock, after the one that is expected by the test? Examples from the past couple of weeks: cfbot=> select

Re: list of acknowledgments for PG16

2023-08-22 Thread Bruce Momjian
On Tue, Aug 22, 2023 at 10:03:29AM -0400, Joe Conway wrote: > On 8/22/23 09:44, Tom Lane wrote: > > Alvaro Herrera writes: > > > On 2023-Aug-22, Peter Eisentraut wrote: > > > > The list of acknowledgments for the PG16 release notes has been > > > > committed. > > > > It should show up here

Re: PG 16 draft release notes ready

2023-08-22 Thread Bruce Momjian
On Tue, Aug 22, 2023 at 10:02:16AM +0300, Pavel Luzanov wrote: > On 22.08.2023 00:58, Bruce Momjian wrote: > > Attached is an applied patch that moves the inherit item into > > incompatibilities. clarifies it, and splits out the ADMIN syntax item. > > > The role's default inheritance behavior can

Re: Convert encrypted SSL test keys to PKCS#8 format

2023-08-22 Thread Jacob Champion
On Tue, Aug 22, 2023 at 1:07 AM Peter Eisentraut wrote: > I have attached two patches, one to update the generation rules, and one > where I have converted the existing test files. (I didn't generate them > from scratch, so for example > src/test/modules/ssl_passphrase_callback/server.crt that

Re: Make all Perl warnings fatal

2023-08-22 Thread Andrew Dunstan
On 2023-08-22 Tu 09:20, Alvaro Herrera wrote: On 2023-Aug-10, Peter Eisentraut wrote: I wanted to figure put if we can catch these more reliably, in the style of -Werror. AFAICT, there is no way to automatically turn all warnings into fatal errors. But there is a way to do it per script, by

Re: pgsql: Allow tailoring of ICU locales with custom rules

2023-08-22 Thread Jeff Davis
On Mon, 2023-08-14 at 10:34 +0200, Peter Eisentraut wrote: > I have investigated this.  My assessment is that how PostgreSQL > interfaces with ICU is correct.  Whether what ICU does is correct > might > be debatable.  I have filed a bug with ICU about this: >

Re: should frontend tools use syncfs() ?

2023-08-22 Thread Nathan Bossart
On Tue, Aug 22, 2023 at 12:53:53PM +0900, Michael Paquier wrote: > On Mon, Aug 21, 2023 at 07:06:32PM -0700, Nathan Bossart wrote: >> This would look something like the attached patch. I think this is nicer. >> With this patch, we don't have to choose between including fd.h or >> redefining the

Re: DecodeInterval fixes

2023-08-22 Thread Jacob Champion
On Mon, Aug 21, 2023 at 10:39 PM Michael Paquier wrote: > 0002 and 0003 make this stuff fail, but isn't there a risk that this > breaks applications that relied on these accidental behaviors? > Assuming that this is OK makes me nervous. I wouldn't argue for backpatching, for sure, but I guess I

Re: Oversight in reparameterize_path_by_child leading to executor crash

2023-08-22 Thread Ashutosh Bapat
Hi Tom, On Tue, Aug 22, 2023 at 2:18 AM Tom Lane wrote: > > (BTW, I did look at Ashutosh's idea of merging the > reparameterize_path_by_child and path_is_reparameterizable_by_child > functions, but I didn't think that would be an improvement, > because we'd have to clutter

Re: Oversight in reparameterize_path_by_child leading to executor crash

2023-08-22 Thread Tom Lane
I wrote: > ... I think the risk/reward ratio for messing with this in the > back branches is unattractive in any case: to fix a corner case that > apparently nobody uses in the field, we risk breaking any number of > mainstream parameterized-path cases. I'm content to commit the v5 patch > (or a

Re: [PATCH] Add function to_oct

2023-08-22 Thread Peter Eisentraut
On 22.08.23 16:26, Nathan Bossart wrote: I don't understand the reason for this handling of negative values. I would expect that, say, to_hex(-1234) would return '-' || to_hex(1234). For this patch set, I was trying to retain the current behavior, which is to return the two's complement

Re: persist logical slots to disk during shutdown checkpoint

2023-08-22 Thread Ashutosh Bapat
On Tue, Aug 22, 2023 at 3:42 PM Amit Kapila wrote: > > > > Once we have last_persisted_confirm_flush_lsn, (1) is just an > > optimization on top of that. With that we take the opportunity to > > persist confirmed_flush_lsn which is much farther than the current > > persisted value and thus

Re: Oversight in reparameterize_path_by_child leading to executor crash

2023-08-22 Thread Tom Lane
Richard Guo writes: > On Tue, Aug 22, 2023 at 4:48 AM Tom Lane wrote: >> So that led me to the attached v5, which seemed committable to me so I >> set about back-patching it ... and it fell over immediately in v15, as >> shown in the attached regression diffs from v15. It looks to me like >> we

Re: Make error messages about WAL segment size more consistent

2023-08-22 Thread Aleksander Alekseev
Hi Peter, > This started out as a small patch to make pg_controldata use the logging > API instead of printf statements, and then it became a larger patch to > adjust error and warning messages about invalid WAL segment sizes > (IsValidWalSegSize()) across the board. Thanks for working on this.

Re: [PATCH] Add function to_oct

2023-08-22 Thread Nathan Bossart
On Tue, Aug 22, 2023 at 04:20:02PM +0200, Peter Eisentraut wrote: > On 20.08.23 17:25, Nathan Bossart wrote: >> > Doing a quick test, shows that this changes the current behaviour, >> > because all inputs are now treated as 64-bit: >> > >> > HEAD: >> > >> > select to_hex((-1234)::int); >> >

Re: list of acknowledgments for PG16

2023-08-22 Thread Peter Eisentraut
On 22.08.23 15:29, Tom Lane wrote: Alvaro Herrera writes: Yeah, I've been proposing this kind of thing for many years; the problem, until not long ago, was that the tooling was unable to process non-Latin1 characters in all the output formats that we use. But tooling has changed and the

Re: [PATCH] Add function to_oct

2023-08-22 Thread Peter Eisentraut
On 20.08.23 17:25, Nathan Bossart wrote: Doing a quick test, shows that this changes the current behaviour, because all inputs are now treated as 64-bit: HEAD: select to_hex((-1234)::int); to_hex -- fb2e With patch: select to_hex((-1234)::int); to_hex

Re: add timing information to pg_upgrade

2023-08-22 Thread Nathan Bossart
On Tue, Aug 22, 2023 at 11:49:33AM +0200, Peter Eisentraut wrote: > Let's change MESSAGE_WIDTH to 62 in v16, and then pursue the larger > restructuring leisurely. Sounds good. I plan to commit this within the next couple of days. -- Nathan Bossart Amazon Web Services: https://aws.amazon.com

Re: list of acknowledgments for PG16

2023-08-22 Thread Joe Conway
On 8/22/23 09:44, Tom Lane wrote: Alvaro Herrera writes: On 2023-Aug-22, Peter Eisentraut wrote: The list of acknowledgments for the PG16 release notes has been committed. It should show up here sometime: .

Re: Make --help output fit within 80 columns per line

2023-08-22 Thread torikoshia
On 2023-08-21 13:08, Masahiro Ikeda wrote: Thanks for your review! (1) Why don't you add test for the purpose? It could be overkill... I though the following function is the best place. diff --git a/src/test/perl/PostgreSQL/Test/Utils.pm b/src/test/perl/PostgreSQL/Test/Utils.pm index

Re: list of acknowledgments for PG16

2023-08-22 Thread Vik Fearing
On 8/22/23 11:33, Peter Eisentraut wrote: The list of acknowledgments for the PG16 release notes has been committed.  It should show up here sometime: .  As usual, please check for problems such as wrong sorting,

Re: list of acknowledgments for PG16

2023-08-22 Thread Tom Lane
Alvaro Herrera writes: > On 2023-Aug-22, Peter Eisentraut wrote: >> The list of acknowledgments for the PG16 release notes has been committed. >> It should show up here sometime: >> . > Hmm, I think these docs would

Make error messages about WAL segment size more consistent

2023-08-22 Thread Peter Eisentraut
This started out as a small patch to make pg_controldata use the logging API instead of printf statements, and then it became a larger patch to adjust error and warning messages about invalid WAL segment sizes (IsValidWalSegSize()) across the board. I went through and made the primary

Re: list of acknowledgments for PG16

2023-08-22 Thread Vik Fearing
On 8/22/23 15:29, Tom Lane wrote: Alvaro Herrera writes: Yeah, I've been proposing this kind of thing for many years; the problem, until not long ago, was that the tooling was unable to process non-Latin1 characters in all the output formats that we use. But tooling has changed and the oldest

Re: list of acknowledgments for PG16

2023-08-22 Thread Tom Lane
Alvaro Herrera writes: > Yeah, I've been proposing this kind of thing for many years; the > problem, until not long ago, was that the tooling was unable to process > non-Latin1 characters in all the output formats that we use. But > tooling has changed and the oldest platforms have disappeared,

Re: Make all Perl warnings fatal

2023-08-22 Thread Alvaro Herrera
On 2023-Aug-10, Peter Eisentraut wrote: > I wanted to figure put if we can catch these more reliably, in the style of > -Werror. AFAICT, there is no way to automatically turn all warnings into > fatal errors. But there is a way to do it per script, by replacing > > use warnings; > > by >

Re: Make all Perl warnings fatal

2023-08-22 Thread Andrew Dunstan
On 2023-08-22 Tu 00:05, Michael Paquier wrote: On Mon, Aug 21, 2023 at 11:51:24AM -0400, Andrew Dunstan wrote: It's not really the same as -Werror, because many warnings can be generated at runtime rather than compile-time. Still, I guess that might not matter too much since apart from plperl

Re: Fix the error message when failing to restore the snapshot

2023-08-22 Thread Amit Kapila
On Tue, Aug 22, 2023 at 6:09 PM Zhijie Hou (Fujitsu) wrote: > > While testing the logical snapshot restore functionality, I noticed the > data size reported in the error message seems not correct. > > I think it's because we used a const value here: > > SnapBuildRestoreContents(int fd, char

Re: list of acknowledgments for PG16

2023-08-22 Thread Alvaro Herrera
On 2023-Aug-22, Vik Fearing wrote: > On 8/22/23 11:33, Peter Eisentraut wrote: > > As usual, please check for problems such as wrong sorting, duplicate > > names in different variants, or names in the wrong order etc.  (Our > > convention is given name followed by surname.) > > Not necessarily

Fix the error message when failing to restore the snapshot

2023-08-22 Thread Zhijie Hou (Fujitsu)
Hi, While testing the logical snapshot restore functionality, I noticed the data size reported in the error message seems not correct. I think it's because we used a const value here: SnapBuildRestoreContents(int fd, char *dest, Size size, const char *path) ... readBytes = read(fd,

Re: Extract numeric filed in JSONB more effectively

2023-08-22 Thread Chapman Flack
On 2023-08-22 01:54, Andy Fan wrote: After we label it, we will get error like this: select (a->'a')::int4 from m; ERROR: cannot display a value of type internal Without looking in depth right now, I would double-check what relabel node is being applied at the result. The idea, of course,

Re: Direct I/O

2023-08-22 Thread Peter Eisentraut
On 01.05.23 04:47, Thomas Munro wrote: On Mon, May 1, 2023 at 12:00 PM Tom Lane wrote: Justin Pryzby writes: On Sun, Apr 30, 2023 at 06:35:30PM +1200, Thomas Munro wrote: What about a warning message about that at startup if it's on? Such a warning wouldn't be particularly likely to be

Re: list of acknowledgments for PG16

2023-08-22 Thread Vik Fearing
On 8/22/23 11:33, Peter Eisentraut wrote: As usual, please check for problems such as wrong sorting, duplicate names in different variants, or names in the wrong order etc.  (Our convention is given name followed by surname.) Not necessarily for this time around, but I would like to see this

Re: ALTER COLUMN ... SET EXPRESSION to alter stored generated column's expression

2023-08-22 Thread Vik Fearing
On 8/2/23 12:35, Amul Sul wrote: Hi, Currently, we have an option to drop the expression of stored generated columns as: ALTER [ COLUMN ] column_name DROP EXPRESSION [ IF EXISTS ] But don't have support to update that expression. The attached patch provides that as: ALTER [ COLUMN ]

Re: list of acknowledgments for PG16

2023-08-22 Thread Alvaro Herrera
On 2023-Aug-22, Peter Eisentraut wrote: > The list of acknowledgments for the PG16 release notes has been committed. > It should show up here sometime: > . > As usual, please check for problems such as wrong sorting,

Re: persist logical slots to disk during shutdown checkpoint

2023-08-22 Thread Amit Kapila
On Tue, Aug 22, 2023 at 2:56 PM Ashutosh Bapat wrote: > > On Tue, Aug 22, 2023 at 9:48 AM Amit Kapila wrote: > > > > > > Another idea is to record the confirm_flush_lsn at the time of > > > persisting the slot. We can use it in two different ways 1. to mark a > > > slot dirty and persist if the

Re: Support run-time partition pruning for hash join

2023-08-22 Thread Richard Guo
On Tue, Aug 22, 2023 at 2:38 PM David Rowley wrote: > With Hash Join, it seems to me that the pruning must take place for > every row that makes it into the hash table. There will be maybe > cases where the unioned set of partitions simply yields every > partition and all the work results in no

Re: add timing information to pg_upgrade

2023-08-22 Thread Peter Eisentraut
On 01.08.23 17:45, Nathan Bossart wrote: On Tue, Aug 01, 2023 at 09:46:02AM +0200, Peter Eisentraut wrote: On 31.07.23 20:37, Nathan Bossart wrote: - prep_status("Checking for incompatible \"aclitem\" data type in user tables"); + prep_status("Checking for \"aclitem\" data type in

Re: Support run-time partition pruning for hash join

2023-08-22 Thread Richard Guo
On Mon, Aug 21, 2023 at 8:34 PM Andy Fan wrote: > This feature looks good, but is it possible to know if we can prune > any subnodes before we pay the extra effort (building the Hash > table, for each row... stuff)? > It might be possible if we take the partition prunning into consideration

list of acknowledgments for PG16

2023-08-22 Thread Peter Eisentraut
The list of acknowledgments for the PG16 release notes has been committed. It should show up here sometime: . As usual, please check for problems such as wrong sorting, duplicate names in different variants, or

Re: persist logical slots to disk during shutdown checkpoint

2023-08-22 Thread Ashutosh Bapat
On Tue, Aug 22, 2023 at 9:48 AM Amit Kapila wrote: > > > > Another idea is to record the confirm_flush_lsn at the time of > > persisting the slot. We can use it in two different ways 1. to mark a > > slot dirty and persist if the last confirm_flush_lsn when slot was > > persisted was too far from

Re: [PoC] pg_upgrade: allow to upgrade publisher node

2023-08-22 Thread Peter Smith
Hi Kuroda-san. I already posted a review for v22-0003 earlier today, but v23-0003 was already posted so those are not yet addressed. Here are a few more review comments I noticed when looking at the latest v23-0003. == src/bin/pg_upgrade/check.c 1. +#include "access/xlogdefs.h" #include

Re: Adding a LogicalRepWorker type field

2023-08-22 Thread Amit Kapila
On Mon, Aug 21, 2023 at 3:48 PM Amit Kapila wrote: > > On Mon, Aug 21, 2023 at 5:34 AM Peter Smith wrote: > > > > PSA v9 > > > > LGTM. I'll push this tomorrow unless there are any more comments. > Pushed. -- With Regards, Amit Kapila.

Convert encrypted SSL test keys to PKCS#8 format

2023-08-22 Thread Peter Eisentraut
This is part of the larger project of allowing all test suites to pass in OpenSSL FIPS mode. We had previously integrated several patches that avoid or isolate use of MD5 in various forms in the tests. Now to another issue. OpenSSL in FIPS mode rejects several encrypted private keys used in

Re: Report planning memory in EXPLAIN ANALYZE

2023-08-22 Thread jian he
On Mon, Aug 14, 2023 at 3:13 PM Ashutosh Bapat wrote: > > On Mon, Aug 14, 2023 at 8:22 AM Andrey Lepikhov > wrote: > > > > Really, the current approach with the final value of consumed memory > > smooths peaks of memory consumption. I recall examples likewise massive > > million-sized arrays or

Re: ALTER COLUMN ... SET EXPRESSION to alter stored generated column's expression

2023-08-22 Thread ajitpostgres awekar
The following review has been posted through the commitfest application: make installcheck-world: tested, passed Implements feature: tested, passed Spec compliant: tested, passed Documentation:tested, passed Hi Amul, Patch changes look fine. Below are some of my

Re: PG 16 draft release notes ready

2023-08-22 Thread Pavel Luzanov
On 22.08.2023 00:58, Bruce Momjian wrote: Attached is an applied patch that moves the inherit item into incompatibilities. clarifies it, and splits out the ADMIN syntax item. > The role's default inheritance behavior can be overridden with the new GRANT ... WITH INHERIT clause. The only

Re: [PoC] pg_upgrade: allow to upgrade publisher node

2023-08-22 Thread Amit Kapila
On Mon, Aug 21, 2023 at 6:32 PM Hayato Kuroda (Fujitsu) wrote: > > > 2. > > + /* > > + * Checking for logical slots must be done before > > + * check_new_cluster_is_empty() because the slot_arr attribute of the > > + * new_cluster will be checked in that function. > > + */ > > + if

Re: Support run-time partition pruning for hash join

2023-08-22 Thread David Rowley
On Tue, 22 Aug 2023 at 00:34, Andy Fan wrote: > > On Mon, Aug 21, 2023 at 11:48 AM Richard Guo wrote: >> 1. All the join partition prunning decisions are made in createplan.c >>where the best path tree has been decided. This is not great. Maybe >>it's better to make it happen when we

Re: [PoC] pg_upgrade: allow to upgrade publisher node

2023-08-22 Thread Peter Smith
Hi Kuroda-san, Here are some review comments for patch v23-0002 == 1. GENERAL Please try to run a spell/grammar check on all the text like commit message and docs changes before posting (e.g. cut/paste the rendered text into some tool like MSWord or Grammarly or ChatGPT or whatever tool you

Re: [PoC] pg_upgrade: allow to upgrade publisher node

2023-08-22 Thread Amit Kapila
On Mon, Aug 21, 2023 at 6:35 PM Hayato Kuroda (Fujitsu) wrote: > > > 9. check_for_logical_replication_slots > > > > + /* logical replication slots can be migrated since PG17. */ > > + if (GET_MAJOR_VERSION(new_cluster->major_version) <= 1600) > > + return; > > > > IMO the code matches the comment