Commitfest 2023-11 is almost over

I'm currently spending some time looking for "Needs Review" threads that have some combination of 1) stalled for a long time and 2) lack consensus, with an aim to clearing them out of CF. Since we got a late start, I will begin moving entries over to January on Monday. I thought I remembered

Re: User functions for building SCRAM secrets

On Wed, Mar 22, 2023 at 9:06 PM Jonathan S. Katz wrote: > > Maybe I'm not conveying the problem this is solving -- I'm happy to go > one more round trying to make it clearer -- but if this is not clear, > it'd be good to at develop an alternative approach to this before > withdrawing the patch.

Re: Improving asan/ubsan support

Hello Tristan, 02.12.2023 00:48, Tristan Partin wrote: So it looks like the asan feature detect_stack_use_after_return implemented in gcc allows itself to add some data on stack, that breaks our alignment expectations. With all three such Asserts in md.c removed, `make check-world` passes for

Re: Synchronizing slots from primary to standby

On Fri, Dec 1, 2023 at 9:31 PM Nisha Moond wrote: > > On Fri, Dec 1, 2023 at 5:40 PM Nisha Moond wrote: > > > > Review for v41 patch. > > > > 1. > > == > > src/backend/utils/misc/postgresql.conf.sample > > > > +#enable_syncslot = on # enables slot synchronization on the physical > > standby

Re: Proposal: Filter irrelevant change before reassemble transactions during logical decoding

On Fri, Dec 1, 2023 at 1:55 PM li jie wrote: > > > This is just an immature idea. I haven't started to implement it yet. > > Maybe it was designed this way because there > > are key factors that I didn't consider. So I want to hear everyone's > > opinions, especially the designers of logic

Re: Emitting JSON to file using COPY TO

On 12/1/23 22:00, Davin Shearer wrote: I'm really glad to see this taken up as a possible new feature and will definitely use it if it gets released.  I'm impressed with how clean, understandable, and approachable the postgres codebase is in general and how easy it is to read and understand

Re: Bug in pgbench prepared statements

> The patch you have sent does not apply cleanly on the master branch, > could you rebase please? Attached. PR against master also here , just to make sure it's mergeable . > Wouldn't it > better

Re: Emitting JSON to file using COPY TO

I'm really glad to see this taken up as a possible new feature and will definitely use it if it gets released. I'm impressed with how clean, understandable, and approachable the postgres codebase is in general and how easy it is to read and understand this patch. I reviewed the patch (though I

Re: Refactoring backend fork+exec code

Hello Heikki, 01.12.2023 23:44, Heikki Linnakangas wrote: With memset(param, 0, sizeof(*param)); added at the beginning of save_backend_variables(), server starts successfully, but then `make check` fails with other Valgrind error. That's actually a pre-existing issue, I'm seeing that even

Re: processes stuck in shutdown following OOM/recovery

On Sat, Dec 2, 2023 at 2:18 PM Thomas Munro wrote: > On Fri, Dec 1, 2023 at 6:13 PM Justin Pryzby wrote: > > $ kill -9 2524495; sleep 0.05; pg_ctl -D ./pgdev.dat1 stop -m fast # > > 2524495 is a child's pid > > > This affects v15, and fails at ) but not its parent. > > Repro'd here. I had to

Re: [PATCH] plpython function causes server panic

I wrote: > The only readily-reachable error case in BeginInternalSubTransaction > is this specific one about IsInParallelMode, which was added later > than the original design and evidently with not a lot of thought or > testing. The comment for it speculates about whether we could get > rid of

Re: [PATCH] plpython function causes server panic

Andres Freund writes: > On 2023-12-01 20:04:15 -0500, Tom Lane wrote: >> Thanks for the report! I see the problem is that we're not expecting >> BeginInternalSubTransaction to fail. However, I'm not sure I like >> this solution, mainly because it's only covering a fraction of the >> problem.

Re: [PATCH] plpython function causes server panic

Hi, On 2023-12-01 20:04:15 -0500, Tom Lane wrote: > Hao Zhang writes: > > I found a problem when executing the plpython function: > > After the plpython function returns an error, in the same session, if we > > continue to execute > > plpython function, the server panic will be caused. > >

Re: processes stuck in shutdown following OOM/recovery

On Fri, Dec 1, 2023 at 6:13 PM Justin Pryzby wrote: > $ kill -9 2524495; sleep 0.05; pg_ctl -D ./pgdev.dat1 stop -m fast # 2524495 > is a child's pid > This affects v15, and fails at 7ff23c6d27 but not its parent. Repro'd here. I had to make the sleep shorter on my system. Looking...

Re: [PATCH] plpython function causes server panic

Hao Zhang writes: > I found a problem when executing the plpython function: > After the plpython function returns an error, in the same session, if we > continue to execute > plpython function, the server panic will be caused. Thanks for the report! I see the problem is that we're not expecting

Re: logical decoding and replication of sequences, take 2

On 12/1/23 12:08, Hayato Kuroda (Fujitsu) wrote: > Dear Tomas, > >> I did some micro-benchmarking today, trying to identify cases where this >> would cause unexpected problems, either due to having to maintain all >> the relfilenodes, or due to having to do hash lookups for every sequence >>

Re: logical decoding and replication of sequences, take 2

On 11/30/23 12:56, Amit Kapila wrote: > On Thu, Nov 30, 2023 at 5:28 AM Tomas Vondra > wrote: >> >> 3) "bad case" - small transactions that generate a lot of relfilenodes >> >> select alter_sequence(); >> >> where the function is defined like this (I did create 1000 sequences >> before the

Re: [Proposal] global sequence implemented by snowflake ID

On 12/1/23 07:15, Amit Kapila wrote: > On Thu, Nov 30, 2023 at 5:21 PM Tomas Vondra > wrote: >> >> On 11/30/23 11:56, Amit Kapila wrote: >> >>> >>> This was the key point that I wanted to discuss or hear opinions >>> about. So, if we wish to have some sort of global sequences then it is >>> not

Re: Refactoring backend fork+exec code

"Tristan Partin" writes: > On Fri Dec 1, 2023 at 2:44 PM CST, Heikki Linnakangas wrote: >> Committed a fix with memset(). I'm not sure what our policy with >> backpatching this kind of issues is. This goes back to all supported >> versions, but given the lack of complaints, I chose to not

Re: Emitting JSON to file using COPY TO

On Fri, Dec 01, 2023 at 02:28:55PM -0500, Joe Conway wrote: > I did a quick PoC patch (attached) -- if there interest and no hard > objections I would like to get it up to speed for the January commitfest. Cool. I would expect there to be interest, given all the other JSON support that has been

Re: pgsql: meson: docs: Add {html,man} targets, rename install-doc-*

Hi, On 2023-12-01 15:55:29 -0600, Tristan Partin wrote: > Commits look fine to me, but I hate the new target names... You shouldn't ever need to use them anywhere - that's what the alias is for... Happy to go another route if you have a suggestion. > > +for name, v in

Re: Clean up some signal usage mainly related to Windows

On Wed Jul 12, 2023 at 9:35 AM CDT, Tristan Partin wrote: On Wed Jul 12, 2023 at 9:31 AM CDT, Peter Eisentraut wrote: > On 12.07.23 16:23, Tristan Partin wrote: > > It has come to my attention that STDOUT_FILENO might not be portable and > > fileno(3) isn't marked as signal-safe, so I have just

Re: [RFC] Clang plugin for catching suspicious typedef casting

On Fri Aug 4, 2023 at 5:47 AM CDT, Dmitry Dolgov wrote: > On Thu, Aug 03, 2023 at 12:23:52PM -0500, Tristan Partin wrote: > > This is the first I am learning about clang plugins. Interesting concept. > Did you give any thought to using libclang instead of a compiler plugin? I > am kind of doing

Re: pgsql: meson: docs: Add {html,man} targets, rename install-doc-*

Commits look fine to me, but I hate the new target names... Luckily, I just use plain ninja, so I don't interact with that. +for name, v in targets_info_byname.items(): +if len(targets_info_byname[name]) > 1: My only comment is that you could reverse the logic and save yourself

Re: [PATCH] pg_convert improvement

On Mon, Nov 27, 2023 at 08:11:06AM +0100, Drouvot, Bertrand wrote: > + PG_RETURN_BYTEA_P(string); I looked around to see whether there was some sort of project policy about returning arguments without copying them, but the only strict rule I see is to avoid scribbling on argument data

Re: Improving asan/ubsan support

On Fri Dec 1, 2023 at 3:00 AM CST, Alexander Lakhin wrote: [ this thread separated from [1] as the discussion focus shifted ] H Andres, 29.11.2023 22:39, Andres Freund wrote: >> I use the following: >> ASAN_OPTIONS=detect_leaks=0:abort_on_error=1:print_stacktrace=1:\ >>

Re: Refactoring backend fork+exec code

On Fri Dec 1, 2023 at 2:44 PM CST, Heikki Linnakangas wrote: On 01/12/2023 16:00, Alexander Lakhin wrote: > Maybe you could look at issues with running `make check` under Valgrind > when server built with CPPFLAGS="-DUSE_VALGRIND -DEXEC_BACKEND": > # +++ regress check in src/test/regress +++ > #

Re: Refactoring backend fork+exec code

On 01/12/2023 16:00, Alexander Lakhin wrote: Maybe you could look at issues with running `make check` under Valgrind when server built with CPPFLAGS="-DUSE_VALGRIND -DEXEC_BACKEND": # +++ regress check in src/test/regress +++ # initializing database system by copying initdb template # postmaster

Re: Dumped SQL failed to execute with ERROR "GROUP BY position -1 is not in select list"

Haotian Chen writes: > postgres=# create view v1 as select * from t1 group by a,b,-1::int; > CREATE VIEW Hmm, surely that is a contrived case? > After exploring codes, I suppose we should treat operator plus constant > as -'nnn'::typename instead of const, my patch just did this by handling >

Re: Building PosgresSQL with LLVM fails on Solaris 11.4

Hi, On 2023-12-01 17:02:25 +, Sacha Hottinger wrote: > Compiling PostgreSQL 13.13 with option –with-llvm fails with Developer Studio > 12.6 as well as with gcc 13.2.0. > I have installed the developer/llvm/clang" + "developer/llvm/clang-build pkgs > (13.0.1). Uh, huh. I did not expect that

Re: Emitting JSON to file using COPY TO

On 11/29/23 10:32, Davin Shearer wrote: Thanks for the responses everyone. I worked around the issue using the `psql -tc` method as Filip described. I think it would be great to support writing JSON using COPY TO at some point so I can emit JSON to files using a PostgreSQL function directly.

Re: Fix bogus Asserts in calc_non_nestloop_required_outer

I have reviewed the changes and it looks fine. The new status of this patch is: Ready for Committer

Re: A wrong comment about search_indexed_tlist_for_var

Alvaro Herrera writes: > On 2023-Dec-01, Richard Guo wrote: >> However, this cross-check will also be performed in non-debug builds >> ever since commit 867be9c07, which converts this check from Asserts to >> test-and-elog. The commit message there also says: >> Committed separately with the

Re: meson: Stop using deprecated way getting path of files

On Fri Dec 1, 2023 at 12:07 PM CST, Tom Lane wrote: "Tristan Partin" writes: > On the buildfarm page[0], it would be nice if more than just the > compiler versions were stated. It would be nice to have all > build/runtime dependencies listed. By and large, we've attempted to address such

Re: meson: Stop using deprecated way getting path of files

"Tristan Partin" writes: > On the buildfarm page[0], it would be nice if more than just the > compiler versions were stated. It would be nice to have all > build/runtime dependencies listed. By and large, we've attempted to address such concerns by extending the configure script to emit info

Re: pg_upgrade and logical replication

On Fri, 1 Dec 2023 at 10:57, Peter Smith wrote: > > Here are review comments for patch v21-0001 > > == > src/bin/pg_upgrade/check.c > > 1. check_old_cluster_subscription_state > > +/* > + * check_old_cluster_subscription_state() > + * > + * Verify that each of the subscriptions has all their

Building PosgresSQL with LLVM fails on Solaris 11.4

Hi all Compiling PostgreSQL 13.13 with option –with-llvm fails with Developer Studio 12.6 as well as with gcc 13.2.0. I have installed the developer/llvm/clang" + "developer/llvm/clang-build pkgs (13.0.1). - It works without the llvm option - I have also tried it with 16.1 – no success either

Re: Remove unnecessary includes of system headers in header files

Peter Eisentraut writes: > I noticed that some header files included system header files for no > apparent reason, so I did some digging and found out that in a few cases > the original reason has disappeared. So I propose the attached patches > to remove the unnecessary includes. Seems

Re: Refactoring backend fork+exec code

On Fri Dec 1, 2023 at 6:10 AM CST, Heikki Linnakangas wrote: On 30/11/2023 20:44, Tristan Partin wrote: > Patches 1-3 seem committable as-is. Thanks for the review! I'm focusing on patches 1-3 now, and will come back to the rest after committing 1-3. There was one test failure with

Re: meson: Stop using deprecated way getting path of files

On Thu Nov 30, 2023 at 3:46 PM CST, Andrew Dunstan wrote: On 2023-11-30 Th 16:00, Tristan Partin wrote: > On Wed Nov 29, 2023 at 1:42 PM CST, Andres Freund wrote: >> Hi, >> >> On 2023-11-29 13:11:23 -0600, Tristan Partin wrote: >> > What is our limiting factor on bumping the minimum Meson

Re: Synchronizing slots from primary to standby

On Fri, Dec 1, 2023 at 5:40 PM Nisha Moond wrote: > > Review for v41 patch. > > 1. > == > src/backend/utils/misc/postgresql.conf.sample > > +#enable_syncslot = on # enables slot synchronization on the physical > standby from the primary > > enable_syncslot is disabled by default, so, it

Re: Materialized view in Postgres from the variables rather than SQL query results

Hi David, Thanks for replying back. Already did that but haven’t received anything yet. On Fri, Dec 1, 2023 at 7:40 PM David G. Johnston wrote: > This mailing list is for discussing the development of patches to the > PostgreSQL code base. Please send your request for help to a more >

Re: EXCLUDE COLLATE in CREATE/ALTER TABLE document

shihao zhong wrote: > Thanks for your comments, a new version is attached. In this hunk: @@ -1097,8 +1097,8 @@ WITH ( MODULUS numeric_literal, REM method index_method. The operators are required to be commutative. Each exclude_element - can optionally specify

Re: Various small doc improvements; plpgsql, schemas, permissions, oidvector

Hello, Thank you all for your help. I won't be able to submit new patches based on reviews for 2 weeks. On Thu, 30 Nov 2023 16:02:28 +0530 Shubham Khanna wrote: > -v7-0012-Explain-role-management.patch > > + The managment of most database objects is by way of granting some > role > > Here

Re: Refactoring backend fork+exec code

Hello Heikki, 01.12.2023 15:10, Heikki Linnakangas wrote: Attached is new version of these patches. For easier review, I made the new refactorings compared in a new commit 0003. I will squash that before pushing, but this makes it easier to see what changed. Barring any new feedback or

Re: Materialized view in Postgres from the variables rather than SQL query results

This mailing list is for discussing the development of patches to the PostgreSQL code base. Please send your request for help to a more appropriate list - specifically the -general list. David J. On Thursday, November 30, 2023, Nurul Karim Rafi wrote: > I have a stored procedure in Postgres.

Re: Postgres Partitions Limitations (5.11.2.3)

On Thu, Nov 30, 2023 at 10:29 PM Laurenz Albe wrote: > > On Thu, 2023-11-30 at 19:22 +0530, Ashutosh Bapat wrote: > > May be attach the patch to hackers thread (this) as well? > > If you want, sure. I thought it was good enough if the thread > is accessible via the commitfest app. The addition

Re: pgsql: Clean up role created in new subscription test.

> On 1 Dec 2023, at 13:19, Alvaro Herrera wrote: > > Isn't it simpler to use DROP OWNED BY in 0001? I suppose it is, I kind of like the explicit drops but we do use OWNED BY quite a lot in the regression tests so changed to that in the attached v5. -- Daniel Gustafsson

Re: A wrong comment about search_indexed_tlist_for_var

On 2023-Dec-01, Richard Guo wrote: > However, this cross-check will also be performed in non-debug builds > ever since commit 867be9c07, which converts this check from Asserts to > test-and-elog. The commit message there also says: > > Committed separately with the idea that eventually

Re: pgsql: Clean up role created in new subscription test.

Isn't it simpler to use DROP OWNED BY in 0001? -- Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/ "Hay que recordar que la existencia en el cosmos, y particularmente la elaboración de civilizaciones dentro de él no son, por desgracia, nada idílicas" (Ijon Tichy)

Re: Synchronizing slots from primary to standby

Review for v41 patch. 1. == src/backend/utils/misc/postgresql.conf.sample +#enable_syncslot = on # enables slot synchronization on the physical standby from the primary enable_syncslot is disabled by default, so, it should be 'off' here. ~~~ 2. IIUC, the slotsyncworker's connection to the

Re: Refactoring backend fork+exec code

On 30/11/2023 20:44, Tristan Partin wrote: Patches 1-3 seem committable as-is. Thanks for the review! I'm focusing on patches 1-3 now, and will come back to the rest after committing 1-3. There was one test failure with EXEC_BACKEND from patch 2, in 'test_shm_mq'. In

Re: pgsql: Clean up role created in new subscription test.

> On 1 Dec 2023, at 12:37, Heikki Linnakangas wrote: > > On 01/12/2023 13:22, Daniel Gustafsson wrote: >>> On 8 Nov 2023, at 13:32, Alvaro Herrera wrote: >>> I suppose you're just thinking of using PQexec() or whatever, run one >>> query with sufficient ORDER BY, save the result, and at the end

Re: pgsql: Clean up role created in new subscription test.

On 01/12/2023 13:22, Daniel Gustafsson wrote: On 8 Nov 2023, at 13:32, Alvaro Herrera wrote: I suppose you're just thinking of using PQexec() or whatever, run one query with sufficient ORDER BY, save the result, and at the end of the test run just run the same query and compare that they are

Re: pgsql: Clean up role created in new subscription test.

> On 8 Nov 2023, at 13:32, Alvaro Herrera wrote: > I suppose you're just thinking of using PQexec() or whatever, run one > query with sufficient ORDER BY, save the result, and at the end of the > test run just run the same query and compare that they are cell-by-cell > identical? This sounds a

Re: Fix a wrong comment in setrefs.c

On Tue, Nov 28, 2023 at 8:19 PM Heikki Linnakangas wrote: > On 03/11/2023 08:10, Richard Guo wrote: > > On Tue, Sep 26, 2023 at 9:51 AM Richard Guo > > wrote: > > On Tue, Sep 26, 2023 at 5:45 AM Tom Lane > > wrote: > > > >

Re: Synchronizing slots from primary to standby

Hi, On 12/1/23 12:06 PM, Amit Kapila wrote: On Wed, Nov 29, 2023 at 3:24 PM Drouvot, Bertrand wrote: I think I'm fine with documenting the fact that the user should not change the failover value. But if he does change it (because at the end nothing prevents it to do so) then I think the

A wrong comment about search_indexed_tlist_for_var

The comment of search_indexed_tlist_for_var says: * In debugging builds, we cross-check the varnullingrels of the subplan * output Var based on nrm_match. However, this cross-check will also be performed in non-debug builds ever since commit 867be9c07, which converts this check from Asserts to

RE: logical decoding and replication of sequences, take 2

Dear Tomas, > I did some micro-benchmarking today, trying to identify cases where this > would cause unexpected problems, either due to having to maintain all > the relfilenodes, or due to having to do hash lookups for every sequence > change. But I think it's fine, mostly ... > I did also

Re: Synchronizing slots from primary to standby

On Wed, Nov 29, 2023 at 3:24 PM Drouvot, Bertrand wrote: > > On 11/29/23 6:58 AM, Zhijie Hou (Fujitsu) wrote: > > On Tuesday, November 28, 2023 8:07 PM Drouvot, Bertrand > > wrote: > > > > Hi, > > > >> On 11/27/23 9:57 AM, Zhijie Hou (Fujitsu) wrote: > >>> On Monday, November 27, 2023 4:51 PM

Re: Synchronizing slots from primary to standby

Hi, On 12/1/23 4:19 AM, shveta malik wrote: On Thu, Nov 30, 2023 at 5:37 PM Ajin Cherian wrote: 1. In my opinion, the second message "aborting the wait...moving to the next slot" does not hold much value. There might not even be a "next slot", there might be just one slot. I think the first

RE: Remove unnecessary includes of system headers in header files

Hi Peter, I have reviewed the patches and also RUN the command, 'make check-world'. It is working fine. All test cases are passed successfully. Thanks and Regards, Shubham Khanna. -Original Message- From: Peter Eisentraut Sent: Friday, December 1, 2023 1:24 PM To: pgsql-hackers

Re: Synchronizing slots from primary to standby

On Fri, Dec 1, 2023 at 3:43 PM Drouvot, Bertrand wrote: > > Hi, > > On 11/30/23 1:06 PM, Ajin Cherian wrote: > > On Wed, Nov 29, 2023 at 8:17 PM Zhijie Hou (Fujitsu) > > > > 3. If creation of a slot on the standby fails for one slot because a > > slot of the same name exists, then thereafter no

Re: Synchronizing slots from primary to standby

Hi, On 11/30/23 1:06 PM, Ajin Cherian wrote: On Wed, Nov 29, 2023 at 8:17 PM Zhijie Hou (Fujitsu) 3. If creation of a slot on the standby fails for one slot because a slot of the same name exists, then thereafter no new sync slots are created on standby. Is this expected? I do see that

Re: Is this a problem in GenericXLogFinish()?

On Thu, Nov 30, 2023 at 4:30 PM Alexander Lakhin wrote: > > 30.11.2023 10:28, Hayato Kuroda (Fujitsu) wrote: > > Again, good catch. Here is my analysis and fix patch. > > I think it is sufficient to add an initialization for writebuf. > > I agree with the change. > Pushed! -- With Regards,

Re: pg_upgrade and logical replication

On Fri, Dec 1, 2023 at 10:57 AM Peter Smith wrote: > > Here are review comments for patch v21-0001 > > > 2. > In this function there are a couple of errors written to the > "subs_invalid.txt" file: > > + fprintf(script, "replication origin is missing for database:\"%s\" > subscription:\"%s\"\n",

Re: Show WAL write and fsync stats in pg_stat_io

Hi, Thanks for all the feedback. I am sharing the new version of the patchset. Current status of the patchset is: - IOOBJECT_WAL / IOCONTEXT_NORMAL / read, write, fsync stats and their tests are added. - IOOBJECT_WAL / IOCONTEXT_INIT stats and their tests are added. - Documentation is updated. -

Improving asan/ubsan support

[ this thread separated from [1] as the discussion focus shifted ] H Andres, 29.11.2023 22:39, Andres Freund wrote: I use the following: ASAN_OPTIONS=detect_leaks=0:abort_on_error=1:print_stacktrace=1:\ disable_coredump=0:strict_string_checks=1:check_initialization_order=1:\

Re: PostgreSql: Canceled on conflict out to old pivot

Thanks guys for the valuable info. The key take away for me is clear: keep transactions short under all circumstances. Cheers, Eduard Am Fr., 1. Dez. 2023 um 01:31 Uhr schrieb Andres Freund : > Hi, > > On 2023-11-30 18:51:35 -0500, Tom Lane wrote: > > On what grounds do you assert that?

Re: Show WAL write and fsync stats in pg_stat_io

Hi, On Wed, 8 Nov 2023 at 10:34, Bharath Rupireddy wrote: > > Is there any plan to account for WAL read stats in the WALRead() > function which will cover walsenders i.e. WAL read by logical and > streaming replication, WAL read by pg_walinspect and so on? I see the > patch already covers WAL

Re: Proposal: Filter irrelevant change before reassemble transactions during logical decoding

> This is just an immature idea. I haven't started to implement it yet. > Maybe it was designed this way because there > are key factors that I didn't consider. So I want to hear everyone's > opinions, especially the designers of logic decoding. Attached is the patch I used to implement this

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

On Thu, Nov 30, 2023 at 6:49 PM Amit Kapila wrote: > > On Wed, Nov 29, 2023 at 7:33 AM Hayato Kuroda (Fujitsu) > wrote: > > > > > > Actually, we do not expect that it won't input NULL. IIUC all of slots > > > > have > > > > slot_name, and subquery uses its name. But will it be kept forever? I