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
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
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:
#
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
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
> 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
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
> 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
> 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
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
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
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
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
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
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
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
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
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: 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
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
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
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 |
> +
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.
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
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
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
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
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"
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
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?
> 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.
>
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
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”
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'
>> 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:
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
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
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.
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
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
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
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
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
> 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
> 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
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
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
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
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: 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
50 matches
Mail list logo