Re: Parallel workers stats in pg_stat_database

On Wed, Oct 02, 2024 at 11:12:37AM +0200, Benoit Lobréau wrote: > My collegues and I had a discussion about what could be done to improve > parallelism observability in PostgreSQL [0]. We thought about several > places to do it for several use cases. > > Guillaume Lelarge worked on pg_stat_stateme

Re: general purpose array_sort

Hi Junwang, On Wed, Oct 2, 2024 at 11:46 PM Junwang Zhao wrote: > On Wed, Oct 2, 2024 at 9:51 AM jian he wrote: > > > > > > > > > > + typentry = (TypeCacheEntry *) fcinfo->flinfo->fn_extra; > > > > + if (typentry == NULL || typentry->type_id != elmtyp) > > > > + { > > > > + typentry = lookup_typ

CI warnings test for 32 bit, and faster headerscheck

Hi, Today when "adder" choked on a compiler warning, I was annoyed that CI knew about that[1] but didn't turn red because only the CompilerWarnings task fails on warnings and it doesn't test 32 bit. So here's a patch for that. Tom has already fixed that in master, but my branch with this change t

Re: long-standing data loss bug in initial sync of logical replication

On Mon, 30 Sept 2024 at 16:56, Hayato Kuroda (Fujitsu) wrote: > > > 2. > > Similarly with above, the relcache won't be invalidated when ALTER > > PUBLICATION > > OWNER TO is executed. This means that privilage checks may be ignored if the > > entry > > is still valid. Not sure, but is there a poss

Re: long-standing data loss bug in initial sync of logical replication

> > I have addressed the comment for 0002 patch and attached the patches. > > Also, I have moved the tests in the 0002 to 0001 patch. > > Thanks for updating the patch. 0002 patch seems to remove cache invalidations > from publication_invalidation_cb(). Related with it, I found an issue and had >

Re: Address the -Wuse-after-free warning in ATExecAttachPartition()

On Tue, Oct 1, 2024 at 3:23 PM Amit Langote wrote: > Hi, > > On Mon, Jul 8, 2024 at 7:08 PM Junwang Zhao wrote: > > On Mon, Jul 8, 2024 at 3:22 PM Nitin Jadhav > > wrote: > > > > > > In [1], Andres reported a -Wuse-after-free bug in the > > > ATExecAttachPartition() function. I've created a pat

Re: Using per-transaction memory contexts for storing decoded tuples

On Wed, Oct 2, 2024 at 9:42 PM Hayato Kuroda (Fujitsu) wrote: > > Dear Sawada-san, Amit, > > > > So, decoding a large transaction with many smaller allocations can > > > have ~2.2% overhead with a smaller block size (say 8Kb vs 8MB). In > > > real workloads, we will have fewer such large transacti

Re: Return pg_control from pg_backup_stop().

On Wed, Oct 02, 2024 at 09:03:27AM +, David Steele wrote: > On 10/2/24 10:11, Michael Paquier wrote: >> Hmm, okay. There is also a slight impact for BASE_BACKUP, requiring >> basebackup_progress_wait_wal_archive() and do_pg_backup_stop() to be >> called earlier when sending the main data direc

RE: Using per-transaction memory contexts for storing decoded tuples

Dear Sawada-san, Amit, > > So, decoding a large transaction with many smaller allocations can > > have ~2.2% overhead with a smaller block size (say 8Kb vs 8MB). In > > real workloads, we will have fewer such large transactions or a mix of > > small and large transactions. That will make the overh

Re: On disable_cost

On Thu, 2024-10-03 at 11:44 +1300, David Rowley wrote: > 2) Laurenz is misunderstanding what "Disabled Nodes" means. It has > nothing to do with other Paths which were considered and rejected. It > might be better named as "Disabled Degree". It tracks how many plan > nodes below and including this

Re: Add on_error and log_verbosity options to file_fdw

On Tue, Oct 1, 2024 at 11:34 PM Fujii Masao wrote: > > > > On 2024/10/02 9:27, Masahiko Sawada wrote: > > Sorry for being late in joining the review of this patch. Both 0001 > > and 0003 look good to me. I have two comments on the 0002 patch: > > Thanks for the review! > > > I think that while sca

Re: Enhance create subscription reference manual

> parameter in this case (it is an "optional" parameter, though). However, > when we refer to the stored catalog value, we should call it an option or > a property and calling it parameter is not suitable. Not sure. The stored catalog value of a subscription can be changed ALTER SUBSCRIPTION. In t

Re: Enhance create subscription reference manual

On Wed, 2 Oct 2024 17:09:42 -0700 "David G. Johnston" wrote: > On Wednesday, October 2, 2024, Peter Smith wrote: > > > ` > > - "subscription's failover option" > > > > Feels like this one refers to the stored catalog value, which is neither a > parameter nor an option but an attribute. Though

Re: bgwrite process is too lazy

Hi Tomas Thank you for explaining,If do not change this static parameter,Can refer to the parameter autovacuum_vacuum_cost_delay and lower the minimum value of the bgwrite_delay parameter to 2ms? Let bgwrite write more dirty pages to reduce the impact on performance when checkpoint occurs?Af

Re: Partitioned tables and [un]loggedness

On Thu, Sep 19, 2024 at 01:08:33PM +0900, Michael Paquier wrote: > I have applied 0001 for now to add ATT_PARTITIONED_TABLE. Attached is > the remaining piece. And the second piece is now applied as of e2bab2d79204. -- Michael signature.asc Description: PGP signature

Re: Enhance create subscription reference manual

On Thu, Oct 3, 2024 at 10:01 AM David G. Johnston wrote: > > On Wednesday, October 2, 2024, Peter Smith wrote: >> >> >> >> You can see how confused the current docs are because "failover" is >> called by both terms even within the same paragraph! [1] >> - "failover parameter specified in the subs

Re: Patch: Show queries of processes holding a lock

On Tue, 1 Oct 2024 at 21:04, Alexey Orlov wrote: > session 1: > CREATE TABLE foo (val integer); > INSERT INTO foo (val) VALUES (1); > BEGIN; > UPDATE foo SET val = 3; > > session 2: > BEGIN; > UPDATE TABLE foo SET val = 2; > > LOG: process 3133043 still waiting for ShareLock on transaction 758 > a

Re: Pgoutput not capturing the generated columns

On Thu, Oct 3, 2024 at 10:09 AM Peter Smith wrote: > > Hi Shubham, > > The different meanings of the terms "parameter" versus "option" were > discussed in a recent thread [1], and that has made me reconsider this > generated columns feature. > > Despite being in the PUBLICATION section "WITH ( pub

Re: [BUG FIX]Connection fails with whitespace after keepalives parameter value

On Wed, Oct 02, 2024 at 05:39:31PM -0400, Tom Lane wrote: > Interesting. This is unhappy about the space before a parameter name, > not the space after a parameter value, so it's a different issue. conninfo_uri_parse_options() parses the URI as a set of option/values, where conninfo_uri_parse_par

Re: pg_verifybackup: TAR format backup verification

I wrote: > Sadly, it seems adder[1] is even pickier than mamba: Nope, it was my testing error: I supposed that this patch only affected pg_combinebackup and pg_verifybackup, so I only recompiled those modules not the whole tree. But there's one more place with a json_manifest_per_file_callback ca

Re: On disable_cost

On Thu, 3 Oct 2024 at 03:04, Robert Haas wrote: > I don't think this will produce the right answer in all cases because > disabled node counts don't propagate across subquery levels. I see my patch didn't behave correctly when faced with a SubqueryScan as SubqueryScan does not use the "lefttree"

Re: Pgoutput not capturing the generated columns

Hi Shubham, The different meanings of the terms "parameter" versus "option" were discussed in a recent thread [1], and that has made me reconsider this generated columns feature. Despite being in the PUBLICATION section "WITH ( publication_parameter [= value] [, ... ] )", I think that 'publish_ge

Re: Enhance create subscription reference manual

On Wednesday, October 2, 2024, Peter Smith wrote: > ` > - "subscription's failover option" > Feels like this one refers to the stored catalog value, which is neither a parameter nor an option but an attribute. Though in some cases we are using “property” as well. An in this usage option sorta

Re: Enhance create subscription reference manual

On Wednesday, October 2, 2024, Peter Smith wrote: > > > You can see how confused the current docs are because "failover" is > called by both terms even within the same paragraph! [1] > - "failover parameter specified in the subscription" > - "subscription's failover option" > > ~~~ > > What to do

Re: Enhance create subscription reference manual

On Wed, Oct 2, 2024 at 6:17 PM Yugo NAGATA wrote: > > > > > I would like to hear opinions from native English speakers. > > +1 > Hi, You can look up the definitions of the words "option" versus "parameter". They do have different meanings, like: - An "option" is a modifier that affects how a co

Re: Add support to TLS 1.3 cipher suites and curves lists

On Wed, Oct 2, 2024 at 11:33 AM Daniel Gustafsson wrote: > > If I migrate a server to a different machine that doesn't support my > > groups, I don't know that this would give me enough information to fix > > the configuration. > > Fair point, how about something along the lines of: > > + er

Re: On disable_cost

On Thu, 3 Oct 2024 at 08:41, Alena Rybakina wrote: > I may have misunderstood your message, but disabled nodes number must > propagate up the tree, otherwise it will be incorrect. I think there are two misunderstandings on this thread: 1) You're misunderstanding what Laurenz is complaining abou

Re: [BUG FIX]Connection fails with whitespace after keepalives parameter value

Fujii Masao writes: > On 2024/10/02 11:35, Michael Paquier wrote: >> On Tue, Oct 01, 2024 at 12:29:15PM -0400, Tom Lane wrote: >>> I agree with Sasaki-san that useKeepalives seems rather bogus: almost >>> every other place in fe-connect.c uses pqParseIntParam rather than >>> calling strtol directl

Re: On disable_cost

On 02.10.2024 22:08, Laurenz Albe wrote: On Wed, 2024-10-02 at 21:31 +0300, Alena Rybakina wrote: Honestly, I like this patch. Before this patch, when disabling any algorithm in the optimizer, the cost increased significantly and I’m not sure that this was a reliable solution due to the fact th

Re: On disable_cost

I see; the merge join happened to be the preferred join path, so nothing had to be excluded. /* reset all parameters */ EXPLAIN (COSTS OFF) SELECT * FROM tab_a JOIN tab_b USING (id); QUERY PLAN ═ Merge Join Merge Cond: (tab_

Re: On disable_cost

On Wed, 2024-10-02 at 21:31 +0300, Alena Rybakina wrote: > Honestly, I like this patch. Before this patch, when disabling any algorithm > in the optimizer, the cost increased significantly and I’m not sure that this > was a reliable solution due to the fact that the cost even without disabling > ca

Re: On disable_cost

On Wed, 2024-10-02 at 21:13 +0300, Alena Rybakina wrote: > >   CREATE TABLE tab_a (id integer); > > > >   CREATE TABLE tab_b (id integer); > > > >   SET enable_nestloop = off; > >   SET enable_hashjoin = off; > > > >   EXPLAIN SELECT * FROM tab_a JOIN tab_b USING (id); > > > >  

Re: pg_verifybackup: TAR format backup verification

Robert Haas writes: > On Tue, Oct 1, 2024 at 1:32 PM Tom Lane wrote: >> Yes, mamba thinks this is OK. > Committed. Sadly, it seems adder[1] is even pickier than mamba: ../pgsql/src/backend/backup/basebackup_incremental.c: In function \342\200\230CreateIncrementalBackupInfo\342\200\231: ../pgs

Re: Add support to TLS 1.3 cipher suites and curves lists

> On 2 Oct 2024, at 19:16, Jacob Champion > wrote: > > On Wed, Sep 25, 2024 at 6:39 AM Daniel Gustafsson wrote: >> I can't recall specific bounds for supporting LibreSSL even being discussed, >> the support is also not documented as an official thing. Requiring TLS 1.3 >> APIs for supporting a

Re: On disable_cost

you can disable mergejoin, I think the output about this will appear. I did it and disabled nodes were displayed in the query explain: alena@postgres=#  CREATE TABLE tab_a (id integer); alena@postgres=#  CREATE TABLE tab_a (id integer); alena@postgres=#  CREATE TABLE tab_b (id integer); alena

Re: BUG #18641: Logical decoding of two-phase commit fails with TOASTed default values

Hi, Amit (adding hackers for discussion) >We forgot to set/unset the flag in functions >systable_beginscan_ordered and systable_endscan_ordered. BTW, Thank you for the clarification. >shouldn't this occur even without prepare transaction? If so, we need >to backpatch this till 14. Yes, it occur

Re: Patch: Show queries of processes holding a lock

On Tue, Oct 1, 2024 at 11:45 AM wenhui qiu wrote: > > Hi Alexey Orlov > Thank you for your work on this path,The lock information is recorded in > detail,Easy to trace the lock competition at that time there is a detailed > lock competition log，But I have a concern,Frequent calls to this fu

Re: On disable_cost

Hi! On 02.10.2024 21:04, Laurenz Albe wrote: I didn't want a running total, but maybe I misunderstood what a disabled node is; see below. If you see a join where two plans were disabled, that's useful information. I'm not sure if I follow what you mean here.  The patch will show "Disabled: tr

Re: On disable_cost

On Wed, 2024-10-02 at 10:08 -0400, Robert Haas wrote: > On Fri, Sep 27, 2024 at 4:42 AM Laurenz Albe wrote: > > 1. The "disabled nodes" are always displayed. > >     I'd be happier if it were only shown for COSTS ON, but I think it > >     would be best if they were only shown with VERBOSE ON. > >

Re: On disable_cost

On Wed, 2024-10-02 at 21:55 +1300, David Rowley wrote: > On Tue, 1 Oct 2024 at 06:17, Laurenz Albe wrote: > > Why did you change "Disabled" from an integer to a boolean? > > I just don't think "Disabled Nodes" is all that self-documenting and > I'm also unsure why the full integer value of disabl

Re: pg_parse_json() should not leak token copies on failure

On 2024-10-01 Tu 3:04 PM, Jacob Champion wrote: (Tom, thank you for the fixup in 75240f65!) Off-list, Andrew suggested an alternative approach to the one I'd been taking: let the client choose whether it wants to take token ownership to begin with. v3 adds a new flag (and associated API) which

Re: Add support to TLS 1.3 cipher suites and curves lists

On Wed, Sep 25, 2024 at 6:39 AM Daniel Gustafsson wrote: > I can't recall specific bounds for supporting LibreSSL even being discussed, > the support is also not documented as an official thing. Requiring TLS 1.3 > APIs for supporting a library in 2025 (when 18 ships) doesn't seem entirely > unre

Re: pg_trgm comparison bug on cross-architecture replication due to different char implementation

On Tue, Oct 1, 2024 at 8:57 PM Tom Lane wrote: > > Noah Misch writes: > > On Tue, Oct 01, 2024 at 11:55:48AM -0700, Masahiko Sawada wrote: > >> Considering that the population of database cluster signedness will > >> converge to signedness=true in the future, we can consider using > >> -fsigned-c

Re: Rename PageData to XLogPageData

On 02/10/2024 14:30, Peter Eisentraut wrote: I was fiddling a bit with making some Page-related APIs const-proof, which might involve changing something like "Page p" to "const PageData *p", but I was surprised that a type PageData exists but it's an unrelated type local to generic_xlog.c. Go

Re: bgwrite process is too lazy

On 10/2/24 17:02, Tony Wayne wrote: > > > On Wed, Oct 2, 2024 at 8:14 PM Laurenz Albe > wrote: > > On Wed, 2024-10-02 at 16:48 +0800, wenhui qiu wrote: > > Whenever I check the checkpoint information in a log, most dirty > pages are written by the ch

Re: bgwrite process is too lazy

On Wed, Oct 2, 2024 at 8:14 PM Laurenz Albe wrote: > On Wed, 2024-10-02 at 16:48 +0800, wenhui qiu wrote: > > Whenever I check the checkpoint information in a log, most dirty pages > are written by the checkpoint process > > That's exactly how it should be! > > is it because if bgwriter frequentl

Re: general purpose array_sort

On Wed, Oct 2, 2024 at 9:51 AM jian he wrote: > > > > > > > + typentry = (TypeCacheEntry *) fcinfo->flinfo->fn_extra; > > > + if (typentry == NULL || typentry->type_id != elmtyp) > > > + { > > > + typentry = lookup_type_cache(elmtyp, sort_asc ? TYPECACHE_LT_OPR : > > > TYPECACHE_GT_OPR); > > > + f

Re: bgwrite process is too lazy

On Wed, 2024-10-02 at 16:48 +0800, wenhui qiu wrote: > Whenever I check the checkpoint information in a log, most dirty pages are > written by the checkpoint process That's exactly how it should be! Yours, Laurenz Albe

Re: not null constraints, again

On 2024-Oct-02, Alvaro Herrera wrote: > On 2024-Oct-02, jian he wrote: > > > On Tue, Oct 1, 2024 at 11:20 PM Alvaro Herrera > > wrote: > > after v7, still not bullet-proof. as before, pg_dump/restore will fail > > for the following: > > > > drop table if exists t2, t2_0 > > create table t2 (a

Re: pg_verifybackup: TAR format backup verification

On Tue, Oct 1, 2024 at 1:32 PM Tom Lane wrote: > Yes, mamba thinks this is OK. Committed. -- Robert Haas EDB: http://www.enterprisedb.com

Re: On disable_cost

On Fri, Sep 27, 2024 at 4:42 AM Laurenz Albe wrote: > 1. The "disabled nodes" are always displayed. >I'd be happier if it were only shown for COSTS ON, but I think it >would be best if they were only shown with VERBOSE ON. > >After all, the messages are pretty verbose... I agree that

Re: On disable_cost

On Wed, Oct 2, 2024 at 4:55 AM David Rowley wrote: > On Tue, 1 Oct 2024 at 06:17, Laurenz Albe wrote: > > Why did you change "Disabled" from an integer to a boolean? > > I just don't think "Disabled Nodes" is all that self-documenting and > I'm also unsure why the full integer value of disabled_n

Re: not null constraints, again

On 2024-Oct-02, jian he wrote: > On Tue, Oct 1, 2024 at 11:20 PM Alvaro Herrera > wrote: > > > > On 2024-Oct-01, jian he wrote: > > > > > create table t2 (a int primary key constraint foo not null no inherit); > > > primary key cannot coexist with not-null no inherit? > > > here t2, pg_dump/res

Re: Allowing parallel-safe initplans

Le 12/04/2023 à 20:06, Robert Haas a écrit : There's only one existing test case that visibly changes plan with these changes. The new plan is clearly saner-looking than before, and testing with some data loaded into the table confirms that it is faster. I'm not sure if it's worth devising m

Rename PageData to XLogPageData

I was fiddling a bit with making some Page-related APIs const-proof, which might involve changing something like "Page p" to "const PageData *p", but I was surprised that a type PageData exists but it's an unrelated type local to generic_xlog.c. This patch renames that type to a more specific

Re: pg_basebackup and error messages dependent on the order of the arguments

>> My point was not so much about --compress but rather giving a good error >> message. >Right, and my point was that the issue is bigger than --compress. >For example, you get exactly the same misbehavior with >$ pg_basebackup --checkpoint=fast --format=t -d --pgdata=/var/tmp/dummy >pg_baseback

Re: Psql meta-command conninfo+

On 02.10.24 06:48, Hunaid Sohail wrote: > Should I revert to the v34 docs for Session User, or is it fine as is? What I tried to say is that the current description is a bit vague --- specially "Authenticated User". > Authenticated User: The name of the user returned by PQuser() > Session User

Re: [BUG FIX]Connection fails with whitespace after keepalives parameter value

On 2024/10/02 11:35, Michael Paquier wrote: On Tue, Oct 01, 2024 at 12:29:15PM -0400, Tom Lane wrote: Fujii Masao writes: Is a connection URL with whitespace, like "tcp:postgresql://localhost:5432/postgres?keepalives=1 & ...", considered valid? If not, the issue seems to be that ecpg adds

Re: Parallel workers stats in pg_stat_database

Hi, Thanks for your imput ! I will fix the doc as proposed and do the split as soon as I have time. On 10/1/24 09:27, Michael Paquier wrote: I'm less a fan of the addition for utilities because these are less common operations. My thought process was that in order to size max_parallel_worke

Re: Return pg_control from pg_backup_stop().

On 10/2/24 10:11, Michael Paquier wrote: On Fri, May 17, 2024 at 12:46:49PM +1000, David Steele wrote: The basic idea is to harden recovery by returning a copy of pg_control from pg_backup_stop() that has a flag set to prevent recovery if the backup_label file is missing. Instead of backup soft

Re: On disable_cost

On Tue, 1 Oct 2024 at 06:17, Laurenz Albe wrote: > Why did you change "Disabled" from an integer to a boolean? I just don't think "Disabled Nodes" is all that self-documenting and I'm also unsure why the full integer value of disabled_nodes is required over just displaying the boolean value of if

Re: Conflict Detection and Resolution

Hi, here are my code/test review comments for patch v14-0001. (the v14-0001 docs review was already previously posted) == src/backend/commands/subscriptioncmds.c parse_subscription_conflict_resolvers: 1. + /* Check if the conflict type already exists in the list */ + if (list_member(SeenTyp

Re: not null constraints, again

On Tue, Oct 1, 2024 at 11:20 PM Alvaro Herrera wrote: > > On 2024-Oct-01, jian he wrote: > > > create table t2 (a int primary key constraint foo not null no inherit); > > primary key cannot coexist with not-null no inherit? > > here t2, pg_dump/restore will fail. > > Yeah, this needs to throw an

Re: Enhance create subscription reference manual

On Wed, 02 Oct 2024 16:25:35 +0900 (JST) Tatsuo Ishii wrote: > > Also, the "public" parameter in CREATE PUBLICATION doc, > > You mean "publish"? Yes. Sorry for the typo. > > > "vacuum_index_cleanup" and > > "vacuum_truncate" storage parameters in CREATE TABLE doc might be also > > targets t

Re: Enhance create subscription reference manual

>> > I agreed with adding tag to "failover" since it is done in the >> > description on "slot_name" parameter. >> > >> > How about also rewrite it to "enable the failover option" rather than >> > simply >> > "enable the failover" to clarify that the parameter is refereed to. >> > >> > We could

Re: Return pg_control from pg_backup_stop().

On Fri, May 17, 2024 at 12:46:49PM +1000, David Steele wrote: > This is greatly simplified implementation of the patch proposed in [1] and > hopefully it addresses the concerns expressed there. Since the > implementation is quite different it seemed like a new thread was > appropriate, especially s