Re: another autovacuum scheduling thread

2025-10-13 Thread Robert Haas
On Fri, Oct 10, 2025 at 6:00 PM Jeremy Schneider wrote: > The spectacular failures I've seen with autovac usually come down to > things like too much sleeping (cost_delay) or too few workers, where > better ordering would be nice but probably wouldn't fix any real > problems leading to the spectac

Re: Add notification on BEGIN ATOMIC SQL functions using temp relations

2025-10-13 Thread Jim Jones
Hi Tom, Thanks for the review and thorough feedback. On 10/8/25 22:35, Tom Lane wrote: > I think the right way to make this work is to look through the > array of ObjectAddresses that ProcedureCreate builds to store > into pg_depend, because that is by definition the authoritative > info about wh

[PATCH] Fix incorrect fprintf usage in log_error FRONTEND path

2025-10-13 Thread Bryan Green
Hi, I noticed that the log_error() utility function has a bug in its FRONTEND code path. The function uses fprintf() with a va_list argument, but fprintf() expects individual arguments. This should be vfprintf() instead. The relevant code: #else fprintf(stderr, fmt, ap); #endif Should be: #

Re: Clarification on Role Access Rights to Table Indexes

2025-10-13 Thread Jeff Davis
On Mon, 2025-10-13 at 17:21 -0400, Tom Lane wrote: > I don't think so.  Even AccessShareLock is enough to block another > session trying to acquire AccessExclusiveLock, and then not only > have you DoS'd that session, but everything else trying to access > the table will queue up behind the AccessE

Re: pgstattuple: Use streaming read API in pgstatindex functions

2025-10-13 Thread Nazir Bilal Yavuz
Hi, On Mon, 13 Oct 2025 at 11:42, Xuneng Zhou wrote: > > Here is patch v3. The comments have been added, and the extra scope > ({}) has been removed as suggested. Thanks, the code looks good to me! The benchmark results are nice. I have a quick question about the setup, if you are benchmarking

Re: doc: Improve description of io_combine_limit and io_max_combine_limit GUCs

2025-10-13 Thread Chao Li
> On Oct 13, 2025, at 19:33, Karina Litskevich > wrote: > > On Thu, Oct 9, 2025 at 6:05 AM Chao Li wrote: >> >> So, this patch looks good to me. Please any committer takes care of this >> patch. >> > > Thank you for the review! > > Could you please change the status of the commitfest ent

Re: speedup COPY TO for partitioned table.

2025-10-13 Thread Masahiko Sawada
On Fri, Oct 10, 2025 at 3:04 AM Álvaro Herrera wrote: > > On 2025-Oct-10, jian he wrote: > > > On Fri, Oct 10, 2025 at 6:03 AM Masahiko Sawada > > wrote: > > > > Yes, but I think it's more consistent given that we use the > > > parentheses in all other places in copyto.c. > > > > If you look at

Re: Fixed a typo in comment in compress_lz4.c

2025-10-13 Thread Daniel Gustafsson
> On 13 Oct 2025, at 23:25, Chao Li wrote: > Fixed a typo: "iff" -> "if" that I found while reviewing the other patch. iff is shorthand for "if and only if", so it is correct here. That being said, it's also the cause of many typofix suggstions so it's clearly not widely known anymore and we tr

Re: Fixed a typo in comment in compress_lz4.c

2025-10-13 Thread Chao Li
> On Oct 14, 2025, at 05:31, Daniel Gustafsson wrote: > >> On 13 Oct 2025, at 23:25, Chao Li wrote: > >> Fixed a typo: "iff" -> "if" that I found while reviewing the other patch. > > iff is shorthand for "if and only if", so it is correct here. That being > said, > it's also the cause of m

Upgrade Debian CI image from Bookworm to Trixie

2025-10-13 Thread Nazir Bilal Yavuz
Hi, Debian Trixie CI images are generated now [1], so the attached patch uses them with the following changes: (copying from commit message) - detect_stack_use_after_return=0 option is added to the ASAN_OPTIONS because ASAN uses a "shadow stack" to track stack variable lifetimes and this con

Re: Add notification on BEGIN ATOMIC SQL functions using temp relations

2025-10-13 Thread Jim Jones
On 10/13/25 17:16, Jim Jones wrote: > PFA a first attempt to address your points. Oops... wrong files. Sorry. PFA the correct version. JimFrom 5e538c3cab1db93ffdff821007b900d1ffd60e39 Mon Sep 17 00:00:00 2001 From: Jim Jones Date: Mon, 13 Oct 2025 13:48:08 +0200 Subject: [PATCH v5 1/2] Refacto

Re: Fix overflow of nbatch

2025-10-13 Thread Melanie Plageman
On Thu, Oct 9, 2025 at 7:36 PM Tomas Vondra wrote: > > 1) A couple comments adjusted. It feels a bit too audacious to correct > comments written by native speaker, but it seems cleaner to me like this. I attached a patch with a few more suggested adjustments (0003). The more substantive tweaks ar

Re: pgstattuple: Use streaming read API in pgstatindex functions

2025-10-13 Thread wenhui qiu
Hi Xuneng Zhou > - /* Unlock and release buffer */ > - LockBuffer(buffer, BUFFER_LOCK_UNLOCK); > - ReleaseBuffer(buffer); > + UnlockReleaseBuffer(buffer); > } Thanks for your patch! Just to nitpick a bit — I think this comment is worth keeping, even though the function name already conveys its

Re: [PATCH] Add tests for Bitmapset

2025-10-13 Thread David Rowley
On Thu, 9 Oct 2025 at 16:47, Michael Paquier wrote: > Yes, a comment can be adapted here. Sounds good to me. Thanks. v3 looks good to me. David

Re: Clarification on Role Access Rights to Table Indexes

2025-10-13 Thread Nathan Bossart
On Fri, Oct 10, 2025 at 11:31:03AM -0700, Jeff Davis wrote: > On Fri, 2025-10-10 at 11:26 -0500, Nathan Bossart wrote: >> After sleeping on it, I still think this is the right call.  In any >> case, I've spent way too much time on this stuff, so I plan to commit >> the attached soon. > > I'm OK wi

Re: pgstattuple: Use streaming read API in pgstatindex functions

2025-10-13 Thread Nazir Bilal Yavuz
Hi, Thank you for working on this! On Mon, 13 Oct 2025 at 06:20, Xuneng Zhou wrote: > > Fix indentation issue in v1. I did not look at the benchmarks, so here are my code comments. - I would avoid creating a new scope for the streaming read. While it makes the streaming code easier to interpre

Re: Adding some error context for lock wait failures

2025-10-13 Thread Tom Lane
I wrote: > Yeah. I see that errfinish does FreeErrorDataContents in the > non-ERROR code path, but of course that does nothing for random > leakages during error processing. I'm tempted to have it do > MemoryContextReset(ErrorContext) if we are at stack depth zero. > That'd be unsafe during neste

RE: [PROPOSAL] Termination of Background Workers for ALTER/DROP DATABASE

2025-10-13 Thread Hayato Kuroda (Fujitsu)
Dear Michael, > > Sorry for posting many times. I noticed that CountOtherDBBackends() can be > called > > while creating the database. Should we also mention and test the case? > > How would you test that? A bgworker would not be able to connect to > the database that's being created. > Sorry f

Re: Thoughts on a "global" client configuration?

2025-10-13 Thread Christoph Berg
Re: Robert Haas > My theory is that they'll be even less impressed if they try to use a > supposedly-compatible library and it breaks a bunch of stuff, but I > wonder what Christoph Berg (cc'd) thinks. It would also hinder adoption of PG in more places. There are currently thousands of software pr

Re: POC: enable logical decoding when wal_level = 'replica' without a server restart

2025-10-13 Thread Masahiko Sawada
On Tue, Oct 7, 2025 at 4:39 AM shveta malik wrote: > > On Tue, Oct 7, 2025 at 9:36 AM Amit Kapila wrote: > > > > On Fri, Oct 3, 2025 at 12:43 AM Masahiko Sawada > > wrote: > > > > > > On Wed, Oct 1, 2025 at 10:48 AM Amit Kapila > > > wrote: > > > > > > > > The other point to consider is that

Re: [PATCH] Fix incorrect fprintf usage in log_error FRONTEND path

2025-10-13 Thread Bryan Green
On 10/13/25 13:16, Bryan Green wrote: On 10/13/25 13:01, Tom Lane wrote: Bryan Green writes: On 10/13/25 10:48, Tom Lane wrote: Huh, that certainly appears broken, but isn't the non-FRONTEND case equally broken?  write_stderr() doesn't expect a va_list either.  I wonder if this code is ever r

Re: Use streaming read I/O in BRIN vacuuming

2025-10-13 Thread Nazir Bilal Yavuz
Hi, On Thu, 9 Oct 2025 at 02:03, Masahiko Sawada wrote: > > Thank you for proposing the patch! I've reviewed the patch and have > some comments: > > + stream = read_stream_begin_relation(READ_STREAM_MAINTENANCE | > + READ_STREAM_FULL | > +

Re: Logical Replication of sequences

2025-10-13 Thread shveta malik
Please find few initial comments for 002: 1) Patch commit msg says: "This patch adds support for a new SQL command: ALTER SUBSCRIPTION ... REFRESH SEQUENCES This command update the sequence entries present in the pg_subscription_rel catalog table with the INIT state to trigger resynchronization.

Visibility bug in tuple lock

2025-10-13 Thread Jasper Smit
Hi, We found a bug in heap_lock_tuple(). It seems to be unsafe to follow the t_ctid pointer of updated tuples, in the presence of aborted tuples. Vacuum can mark aborted tuples LP_UNUSED, even if there are still visible tuples that have a t_ctid pointing to them. We would like to address this i

Re: Add RESPECT/IGNORE NULLS and FROM FIRST/LAST options

2025-10-13 Thread Álvaro Herrera
On 2025-Oct-13, Tatsuo Ishii wrote: > Thanks for the review. In addition to the point, I added an assertion > which is called by all other window function API. Also added check to > the return value of get_func_name() because it could return NULL. V2 > patch attached. Hmm, this change made me rea

Re: IO in wrong state on riscv64

2025-10-13 Thread Thomas Munro
On Sun, Oct 12, 2025 at 2:00 AM Alexander Lakhin wrote: > I've managed to reproduce it using qemu-system-riscv64 with Debian trixie Huh, that's interesting. What is the host architecture? When I saw that error myself and wondered about memory order, I dismissed the idea of trying with qemu, fig

Re: Clarification on Role Access Rights to Table Indexes

2025-10-13 Thread Jeff Davis
On Wed, 2025-09-24 at 12:13 -0400, Tom Lane wrote: > Don't we do that intentionally, to make sure someone can't cause DOS > on a table they have no privileges on? Is this only a problem for strong locks (ShareLock or greater)? Strong locks are a problem when you have a pattern like a long running

Re: VACUUM (PARALLEL) option processing not using DefElem the way it was intended

2025-10-13 Thread Álvaro Herrera
On 2025-Oct-08, Masahiko Sawada wrote: > I noticed we're currently hardcoding the "BUFFER_USAGE_LIMIT" option > name in the error message: > > ereport(ERROR, > (errcode(ERRCODE_INVALID_PARAMETER_VALUE), > errmsg("BUFFER_USAGE_LIMIT option must be 0 or between %d kB > and %d kB"

Re: Question about InvalidatePossiblyObsoleteSlot()

2025-10-13 Thread suyu.cmj
Hi, Thanks for your detailed reply. > I think that we could get rid of the initial_restart_lsn and just use > s->data.restart_lsn here (while keeping initial xmin ones to preserve the > intent of 818fefd8fd4 for those). I agree with your proposed change. > Your concern is only about the restart_lsn

Re: What is the build strategy between make and meson?

2025-10-13 Thread Tom Lane
I wrote: > Chao Li writes: >> The other problem I encountered is that, when unicode map files are >> regenerated, “make” won’t auto rebuild corresponding .o and lib files, but >> ninja does. That means “Makefile” has something to fix. But given ninja >> works, should “Makefile” still be fixed?

Re: [PING] [PATCH v2] parallel pg_restore: avoid disk seeks when jumping short distance forward

2025-10-13 Thread Chao Li
> On Oct 14, 2025, at 10:44, Tom Lane wrote: > > This won't show the effect, because pg_dump will be able to go back > and insert data offsets into the dump's TOC, so pg_restore can just > seek to where the data is. See upthread discussion about what's > needed to provoke Dimitrios' problem. >

Re: Add LZ4 compression in pg_dump

2025-10-13 Thread Michael Paquier
On Sun, Oct 12, 2025 at 01:24:37PM -0400, Tom Lane wrote: > Michael Paquier writes: >> At the end I am finishing with the attached. I also saw an overlap >> with the addition of --jobs for the directory format vs not using the >> option, so I have removed the case where --jobs was not used in the

Why pg_dump overwrites dump file?

2025-10-13 Thread Chao Li
Hi Hacker, I noticed this problem while testing the other patch. When I do custom-format dump, if a target file exists, pg_dump will just go ahead overwrite the existing file; however, when I do directory dump, if a target dir exists, pg_dump will fail with an error “directory xxx is not empty”

A tidyup of pathkeys.c

2025-10-13 Thread David Rowley
When working on a5a68dd6d I noticed that truncate_useless_pathkeys() uses a bunch of different static helper functions that are mostly the same as each other. Most of them differ only in which field of PlannerInfo they look at. The attached aims to clean that up by making 2 reusable functions. I'

Re: Add RESPECT/IGNORE NULLS and FROM FIRST/LAST options

2025-10-13 Thread Tatsuo Ishii
>> Thank you for the report! >> >>> Coverity is not very happy with this patch. >>> It's complaining that the result of window_gettupleslot >>> is not checked, which seems valid: >>> >>> 1503{ >>> 1504if (fetch_tuple) >> CID 1666587:

Re: another autovacuum scheduling thread

2025-10-13 Thread Nathan Bossart
On Thu, Oct 09, 2025 at 11:13:48AM -0500, Nathan Bossart wrote: > On Thu, Oct 09, 2025 at 04:13:23PM +1300, David Rowley wrote: >> I think the best way to understand it is if you look at >> relation_needs_vacanalyze() and see how it calculates boolean values >> for boolean output params. So, instea

Re: doc: Improve description of io_combine_limit and io_max_combine_limit GUCs

2025-10-13 Thread Karina Litskevich
On Thu, Oct 9, 2025 at 6:05 AM Chao Li wrote: > > So, this patch looks good to me. Please any committer takes care of this > patch. > Thank you for the review! Could you please change the status of the commitfest entry [1] to Ready for Commiter if you believe it's ready? [1] https://commitfest

Re: [PATCH TEST] Fix logical replication setup in subscription test `t/009_matviews.pl`

2025-10-13 Thread Michael Paquier
On Mon, Oct 13, 2025 at 06:12:07PM +0530, Ashutosh Bapat wrote: > For now this makes sense. The arguments and the patches I am seeing do not really make sense here. > We could avoid running a full test, and save time and resources, if we > piggy back MV vs logical replication testing on 100_bugs.

Re: Logical Replication of sequences

2025-10-13 Thread shveta malik
On Mon, Oct 13, 2025 at 12:57 PM shveta malik wrote: > > Please find few initial comments for 002: > 7) Currently CREATE SUB makes the state 'i' for sequences in pg_subscription_rel while ALTER SUB REFRESH SEQ makes the state as 'd'. I think we do not need to maintain 2 different states here. We

Re: IO in wrong state on riscv64

2025-10-13 Thread Thomas Munro
On Mon, Oct 13, 2025 at 5:00 PM Alexander Lakhin wrote: > 13.10.2025 01:44, Thomas Munro wrote: > > On Sun, Oct 12, 2025 at 6:00 PM Alexander Lakhin > > wrote: > >> Please find those attached (gdb "disass/m pgaio_io_update_state" misses > >> the start of the function (but it's still disassembled

Re: Fixed a typo in comment in compress_lz4.c

2025-10-13 Thread Michael Paquier
On Mon, Oct 13, 2025 at 06:08:43PM -0400, Tom Lane wrote: > I disagree that it's not widely known. OneLook Dictionary Search [1] > defines it correctly as "Usually means: If and only if; equivalence" > and reports finding it in 35 of their underlying dictionaries, > including all the usual suspect

Re: [PATCH v1] parallel pg_restore: avoid disk seeks when jumping short distance forward

2025-10-13 Thread David G. Johnston
On Sunday, October 12, 2025, Chao Li wrote: > 2 - 0003 > ``` > /* > * LZ4 equivalent to feof() or gzeof(). Return true iff there is no > - * decompressed output in the overflow buffer and the end of the backing > file > ``` > > This doesn’t belong to the current patch. But “iff” seems a typo

Re: Incorrect version number given to sync_pgdata() in pg_combinebackup.c

2025-10-13 Thread Michael Paquier
On Fri, Oct 10, 2025 at 02:39:58PM +0800, Chao Li wrote: > Yeah, looks like a stupid bug. read_pg_version_file() has multiplied > 1 to version number. Thanks for the review. Applied this one down to v17. -- Michael signature.asc Description: PGP signature

Re: Fixed a typo in comment in compress_lz4.c

2025-10-13 Thread Chao Li
> On Oct 14, 2025, at 07:32, Michael Paquier wrote: > > On Mon, Oct 13, 2025 at 06:08:43PM -0400, Tom Lane wrote: >> I disagree that it's not widely known. OneLook Dictionary Search [1] >> defines it correctly as "Usually means: If and only if; equivalence" >> and reports finding it in 35 of t

Re: [PING] [PATCH v2] parallel pg_restore: avoid disk seeks when jumping short distance forward

2025-10-13 Thread Chao Li
> On Oct 14, 2025, at 05:07, Tom Lane wrote: > > > I've pushed the parts of that patch set that I thought were > uncontroversial. What's left is the business about increasing > DEFAULT_IO_BUFFER_SIZE and then adjusting the tests appropriately. > > So, v4-0001 attached is the previous v3-0002

Re: Fixed a typo in comment in compress_lz4.c

2025-10-13 Thread Tom Lane
Chao Li writes: > Look at this instance. The comment says: > * LZ4 equivalent to feof() or gzeof(). Return true iff there is no > * more buffered data and the end of the input file has been reached. > It just states when the function should return true. In this case, why “if” > is not good

Re: Thoughts on a "global" client configuration?

2025-10-13 Thread Jacob Champion
On Thu, Oct 9, 2025 at 5:28 AM Robert Haas wrote: > I'm never going to be a fan of the idea of changing libpq defaults > with any frequency, no matter what configuration options we have. If > we change those defaults with any regularity, I think it will cause a > lot of problems for a lot of peopl

Build failure with Meson >= 1.8.3 on Windows

2025-10-13 Thread Dave Page
I recently tried to update the version of Meson used in my winpgbuild project, which builds PostgreSQL for Windows, along with (almost) all the various dependencies. Something has changed in versions from 1.8.3 and above which causes the regression tests to fail: This successful run is with Meson

Re: [PROPOSAL] Termination of Background Workers for ALTER/DROP DATABASE

2025-10-13 Thread Michael Paquier
On Thu, Oct 09, 2025 at 02:28:56AM +, Hayato Kuroda (Fujitsu) wrote: > Sorry for posting many times. I noticed that CountOtherDBBackends() can be > called > while creating the database. Should we also mention and test the case? How would you test that? A bgworker would not be able to connect

Re: Thoughts on a "global" client configuration?

2025-10-13 Thread Christoph Berg
Re: Robert Haas > So you support calling it libpq.so.5 forever, no matter how much we change? I would say SONAME/ABI/API breakages are not a tool to promote better SSL settings. If we want to move to sslmode=verify-full by default, we should just do that. I don't see why adding extra ABI/API pain