Re: [PoC] Reducing planning time when tables have many partitions

2023-11-17 Thread Tom Lane
John Naylor writes: > On Sat, Nov 18, 2023 at 4:04 AM Alena Rybakina > wrote: >> All diff files have already been added to >> v21-0002-Introduce-indexes-for-RestrictInfo patch. > Unfortunately, the patch tester is too smart for its own good, and > will try to apply .diff files as well. Yeah

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

2023-11-17 Thread Andres Freund
Hi, On 2023-11-17 17:54:43 -0800, Andres Freund wrote: > On 2023-11-17 15:36:25 +0100, Tomas Vondra wrote: > > Overall, this looks, walks and quacks like a cache invalidation issue, > > likely a missing invalidation somewhere in the ALTER PUBLICATION code. I can confirm that something is broken

Re: [PoC] Reducing planning time when tables have many partitions

2023-11-17 Thread John Naylor
On Sat, Nov 18, 2023 at 4:04 AM Alena Rybakina wrote: > > All diff files have already been added to > v21-0002-Introduce-indexes-for-RestrictInfo patch. Unfortunately, the patch tester is too smart for its own good, and will try to apply .diff files as well. Since

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

2023-11-17 Thread Andres Freund
Hi, On 2023-11-17 15:36:25 +0100, Tomas Vondra wrote: > It seems there's a long-standing data loss issue related to the initial > sync of tables in the built-in logical replication (publications etc.). :( > Overall, this looks, walks and quacks like a cache invalidation issue, > likely a

Re: Change GUC hashtable to use simplehash?

2023-11-17 Thread Andres Freund
Hi, On 2023-11-17 16:01:31 -0800, Jeff Davis wrote: > On Fri, 2023-11-17 at 15:27 -0800, Andres Freund wrote: > > At > > first I thought that that's largely because you aren't using > > SH_STORE_HASH. > > I might want to use that in the search_path cache, then. The lookup > wasn't showing up much

Re: On non-Windows, hard depend on uselocale(3)

2023-11-17 Thread Andres Freund
Hi, On 2023-11-17 08:57:47 +1300, Thomas Munro wrote: > I also had a go[3] at doing it with static inlined functions, to avoid > creating a load of new exported functions and associated function call > overheads. It worked fine, except on Windows: I needed a global > variable PGTYPESclocale that

Re: Change GUC hashtable to use simplehash?

2023-11-17 Thread Jeff Davis
On Fri, 2023-11-17 at 15:27 -0800, Andres Freund wrote: > At > first I thought that that's largely because you aren't using > SH_STORE_HASH. I might want to use that in the search_path cache, then. The lookup wasn't showing up much in the profile the last I checked, but I'll take a second look.

Re: Add pg_basetype() function to obtain a DOMAIN base type

2023-11-17 Thread jian he
On Thu, Sep 28, 2023 at 11:56 AM Alexander Korotkov wrote: > > The one thing triggering my perfectionism is that the patch does two > syscache lookups instead of one. In order to fit into one syscache > lookup we could add "bool missing_ok" argument to > getBaseTypeAndTypmod(). However,

Re: Change GUC hashtable to use simplehash?

2023-11-17 Thread Andres Freund
Hi, On 2023-11-17 14:08:56 -0800, Jeff Davis wrote: > On Fri, 2023-11-17 at 17:04 -0500, Tom Lane wrote: > > I can't imagine wanting to convert *every* hashtable in the system > > to simplehash; the added code bloat would be unreasonable.  So yeah, > > I think we'll have two mechanisms

Re: Emit fewer vacuum records by reaping removable tuples during pruning

2023-11-17 Thread Melanie Plageman
On Mon, Nov 13, 2023 at 5:28 PM Melanie Plageman wrote: > When there are no indexes on the relation, we can set would-be dead > items LP_UNUSED and remove them during pruning. This saves us a vacuum > WAL record, reducing WAL volume (and time spent writing and syncing > WAL). ... > Note that (on

Re: On non-Windows, hard depend on uselocale(3)

2023-11-17 Thread Tom Lane
I wrote: > I've not reviewed this closely, but I did try it on mamba's host. > It compiles and passes regression testing, but I see two warnings: > common.c: In function 'PGTYPESsprintf': > common.c:120:2: warning: function 'PGTYPESsprintf' might be a candidate for > 'gnu_printf' format

Re: Change GUC hashtable to use simplehash?

2023-11-17 Thread Jeff Davis
On Fri, 2023-11-17 at 14:08 -0800, Andres Freund wrote: > I think this would be a completely fair thing to port over - whether > it's > worth it I don't quite know, but I'd not be against it on principle > or such. Right now I don't think it offers much. I'll see if I can solve the case-folding

Re: Change GUC hashtable to use simplehash?

2023-11-17 Thread Andres Freund
Hi, On 2023-11-17 17:04:04 -0500, Tom Lane wrote: > Jeff Davis writes: > > On Fri, 2023-11-17 at 13:22 -0800, Gurjeet Singh wrote: > >> But your argument of a nicer API might make a case for the patch. > > > Yeah, that's what I was thinking. simplehash is newer and has a nicer > > API, so if we

Re: Change GUC hashtable to use simplehash?

2023-11-17 Thread Jeff Davis
On Fri, 2023-11-17 at 17:04 -0500, Tom Lane wrote: > I can't imagine wanting to convert *every* hashtable in the system > to simplehash; the added code bloat would be unreasonable.  So yeah, > I think we'll have two mechanisms indefinitely.  That's not to say > that we might not rewrite hsearch. 

Re: Change GUC hashtable to use simplehash?

2023-11-17 Thread Andres Freund
Hi, On 2023-11-17 13:44:21 -0800, Jeff Davis wrote: > On Fri, 2023-11-17 at 13:22 -0800, Gurjeet Singh wrote: > > This is not a comment on the patch itself, but since GUC operations > > are not typically considered performance or space sensitive, I don't think that's quite right - we have a lot

Re: Change GUC hashtable to use simplehash?

2023-11-17 Thread Tom Lane
Jeff Davis writes: > On Fri, 2023-11-17 at 13:22 -0800, Gurjeet Singh wrote: >> But your argument of a nicer API might make a case for the patch. > Yeah, that's what I was thinking. simplehash is newer and has a nicer > API, so if we like it and want to move more code over, this is one > step.

Re: Change GUC hashtable to use simplehash?

2023-11-17 Thread Jeff Davis
On Fri, 2023-11-17 at 13:22 -0800, Gurjeet Singh wrote: > This is not a comment on the patch itself, but since GUC operations > are not typically considered performance or space sensitive, A "SET search_path" clause on a CREATE FUNCTION is a case for better performance in guc.c, because it

Re: Lifetime of commit timestamps

2023-11-17 Thread Bruce Momjian
On Fri, Nov 17, 2023 at 01:20:46PM -0800, Andres Freund wrote: > Hi, > > On 2023-11-17 15:39:14 -0300, Euler Taveira wrote: > > On Mon, Nov 13, 2023, at 9:47 PM, Bruce Momjian wrote: > > > Is this documentation change still relevant? > > > > I think so. AFAICS nothing changed. Unless you read

Re: Change GUC hashtable to use simplehash?

2023-11-17 Thread Gurjeet Singh
On Fri, Nov 17, 2023 at 11:02 AM Jeff Davis wrote: > > I had briefly experimented changing the hash table in guc.c to use > simplehash. It didn't offer any measurable speedup, but the API is > slightly nicer. > > I thought I'd post the patch in case others thought this was a good > direction or

Re: Permute underscore separated components of columns before fuzzy matching

2023-11-17 Thread Tom Lane
Mikhail Gribkov writes: > Honestly I'm not entirely sure fixing only two switched words is worth the > effort, but the declared goal is clearly achieved. > I think the patch is good to go, although you need to fix code formatting. I took a brief look at this. I concur that we shouldn't need to

Re: simplehash: preserve consistency in case of OOM

2023-11-17 Thread Andres Freund
On 2023-11-17 13:00:19 -0800, Jeff Davis wrote: > Please tell me if you think the use of simplehash for a search_path > cache is the wrong tool for the job. No, seems fine. I just was curious - as you said, the older existing users won't ever care about this case.

Re: Lifetime of commit timestamps

2023-11-17 Thread Andres Freund
Hi, On 2023-11-17 15:39:14 -0300, Euler Taveira wrote: > On Mon, Nov 13, 2023, at 9:47 PM, Bruce Momjian wrote: > > Is this documentation change still relevant? > > I think so. AFAICS nothing changed. Unless you read the source code, it is not > clear that VACUUM removes the information for

Re: should check collations when creating partitioned index

2023-11-17 Thread Jeff Davis
On Fri, 2023-11-17 at 15:18 -0500, Tom Lane wrote: > You keep harping on this idea that we are only concerned with > equality, > but I think you are wrong.  We expect a btree index to provide > ordering > not only equality, and this example definitely is a btree index. > > Possibly, with a great

Re: lazy_scan_heap() should release lock on buffer before vacuuming FSM

2023-11-17 Thread Andres Freund
Hi, On 2023-11-15 16:21:45 -0500, Melanie Plageman wrote: > On Tue, Nov 14, 2023 at 7:15 PM Andres Freund wrote: > > On 2023-11-14 07:46:10 -0500, Melanie Plageman wrote: > > > > FreeSpaceMapVacuumRange()'s comment says: > > > > * As above, but assume that only heap pages between start and

Re: Lifetime of commit timestamps

2023-11-17 Thread Bruce Momjian
On Fri, Nov 17, 2023 at 03:39:14PM -0300, Euler Taveira wrote: > On Mon, Nov 13, 2023, at 9:47 PM, Bruce Momjian wrote: > > Is this documentation change still relevant? > > > I think so. AFAICS nothing changed. Unless you read the source code, it is not > clear that VACUUM removes the

Re: simplehash: preserve consistency in case of OOM

2023-11-17 Thread Gurjeet Singh
On Fri, Nov 17, 2023 at 12:13 PM Andres Freund wrote: > > On 2023-11-17 10:42:54 -0800, Jeff Davis wrote: > > Right now, if allocation fails while growing a hashtable, it's left in > > an inconsistent state and can't be used again. +1 to the patch. > I'm not against allowing this - but I am

Re: simplehash: preserve consistency in case of OOM

2023-11-17 Thread Jeff Davis
On Fri, 2023-11-17 at 12:13 -0800, Andres Freund wrote: > On 2023-11-17 10:42:54 -0800, Jeff Davis wrote: > > Right now, if allocation fails while growing a hashtable, it's left > > in > > an inconsistent state and can't be used again. > > I'm not against allowing this - but I am curious, in

Re: ResourceOwner refactoring

2023-11-17 Thread Andres Freund
Hi, On 2023-11-17 12:44:41 -0800, Andres Freund wrote: > On 2023-11-07 13:28:28 +0200, Heikki Linnakangas wrote: > > I feel pretty good about this overall. Barring objections or new cfbot > > failures, I will commit this in the next few days. > > I am working on rebasing the AIO patch over this.

Re: ResourceOwner refactoring

2023-11-17 Thread Andres Freund
Hi, On 2023-11-07 13:28:28 +0200, Heikki Linnakangas wrote: > I feel pretty good about this overall. Barring objections or new cfbot > failures, I will commit this in the next few days. I am working on rebasing the AIO patch over this. I think I found a crash that's unrelated to AIO. #4

Re: Recovering from detoast-related catcache invalidations

2023-11-17 Thread Tom Lane
I wrote: > In bug #18163 [1], Alexander proved the misgivings I had in [2] > about catcache detoasting being possibly unsafe: > ... > Attached is a POC patch for fixing this. The cfbot pointed out that this needed a rebase. No substantive changes. regards, tom lane diff

Re: should check collations when creating partitioned index

2023-11-17 Thread Tom Lane
Jeff Davis writes: > In the patch, you check for an exact collation match. Considering this > case only depends on equality, I think it would be correct if the > requirement was that (a) both collations are deterministic; or (b) the > collations match exactly. You keep harping on this idea that

Re: simplehash: preserve consistency in case of OOM

2023-11-17 Thread Andres Freund
Hi, On 2023-11-17 10:42:54 -0800, Jeff Davis wrote: > Right now, if allocation fails while growing a hashtable, it's left in > an inconsistent state and can't be used again. I'm not against allowing this - but I am curious, in which use cases is this useful? > @@ -446,10 +459,11 @@

Re: should check collations when creating partitioned index

2023-11-17 Thread Jeff Davis
On Mon, 2023-11-13 at 10:24 +0100, Peter Eisentraut wrote: > create table t1 (a int, b text) partition by hash (b); > create table t1a partition of t1 for values with (modulus 2, > remainder 0); > create table t1b partition of t1 for values with (modulus 2, > remainder 1); > create unique index i1

Re: Wrong rows estimations with joins of CTEs slows queries by more than factor 500

2023-11-17 Thread Tom Lane
Richard Guo writes: > On Fri, Nov 17, 2023 at 11:38 AM Tom Lane wrote: >> That line of argument also leads to the conclusion that it'd be >> okay to expose info about the ordering of the CTE result to the >> upper planner. This patch doesn't do that, and I'm not sufficiently >> excited about

Re: Why do indexes and sorts use the database collation?

2023-11-17 Thread Jeff Davis
On Mon, 2023-11-13 at 14:12 -0800, Andres Freund wrote: > Why on earth are we solving this by having multiple pg_collation > entries for > exactly the same collation, instead of normalizing the collation-name > during > lookup by adding the relevant encoding name if not explicitly > specified?  It

Re: Schema variables - new implementation for Postgres 15

2023-11-17 Thread Dmitry Dolgov
> On Wed, Aug 23, 2023 at 04:02:44PM +0200, Pavel Stehule wrote: > NameListToString is already buildin function. Do you think NamesFromList? > > This is my oversight - there is just `+extern List *NamesFromList(List > *names); ` line, but sure - it should be in 0002 patch > > fixed now Right,

Change GUC hashtable to use simplehash?

2023-11-17 Thread Jeff Davis
I had briefly experimented changing the hash table in guc.c to use simplehash. It didn't offer any measurable speedup, but the API is slightly nicer. I thought I'd post the patch in case others thought this was a good direction or nice cleanup. -- Jeff Davis PostgreSQL Contributor Team - AWS

Re: meson documentation build open issues

2023-11-17 Thread Andres Freund
Hi, On 2023-11-14 16:30:24 -0800, Andres Freund wrote: > On 2023-11-14 16:22:31 -0800, Andres Freund wrote: > > > v2-0004-meson-Add-world-target.patch > > > > > > AFAICT, this world target doesn't include the man target. (Again, this > > > would all work better if we added "man" to "docs".) > >

Re: Allow tests to pass in OpenSSL FIPS mode

2023-11-17 Thread Peter Eisentraut
On 15.11.23 21:29, Tom Lane wrote: Daniel Gustafsson writes: Since the 3DES/DES deprecations aren't limited to FIPS, do we want to do anything for pgcrypto where we have DES/3DES encryption? Maybe a doc patch which mentions the deprecation with a link to the SP could be in order? A docs

simplehash: preserve consistency in case of OOM

2023-11-17 Thread Jeff Davis
Right now, if allocation fails while growing a hashtable, it's left in an inconsistent state and can't be used again. Patch attached. -- Jeff Davis PostgreSQL Contributor Team - AWS From 82068d744f668039de7249854bc42eead4e77ebc Mon Sep 17 00:00:00 2001 From: Jeff Davis Date: Fri, 17 Nov

Re: mxid_age() and age(xid) appear undocumented

2023-11-17 Thread Bruce Momjian
On Mon, Nov 13, 2023 at 05:32:24PM -0800, Andres Freund wrote: > Hi, > > On 2023-11-13 17:00:43 -0800, Peter Geoghegan wrote: > > On Mon, Nov 13, 2023 at 4:43 PM Bruce Momjian wrote: > > > I looked into this and all the 4-byte xid functions are marked as > > > deprecated for the 8-byte variants.

Re: Lifetime of commit timestamps

2023-11-17 Thread Euler Taveira
On Mon, Nov 13, 2023, at 9:47 PM, Bruce Momjian wrote: > Is this documentation change still relevant? I think so. AFAICS nothing changed. Unless you read the source code, it is not clear that VACUUM removes the information for frozen tuples. They are decoupled (but executed in the same routine

Re: On non-Windows, hard depend on uselocale(3)

2023-11-17 Thread Tom Lane
Thomas Munro writes: > On Thu, Nov 16, 2023 at 12:06 PM Tom Lane wrote: >> Thomas Munro writes: >>> Perhaps we could use snprintf_l() and strtod_l() where available. >>> They're not standard, but they are obvious extensions that NetBSD and >>> Windows have, and those are the two systems for

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

2023-11-17 Thread Tomas Vondra
Hi, It seems there's a long-standing data loss issue related to the initial sync of tables in the built-in logical replication (publications etc.). I can reproduce it fairly reliably, but I haven't figured out all the details yet and I'm a bit out of ideas, so I'm sharing what I know with the

Re: POC: Extension for adding distributed tracing - pg_tracing

2023-11-17 Thread Aleksander Alekseev
Hi, Thanks for the updated patch! > Some small changes, mostly around making tests less flaky: > - Removed the top_span and nested_level in the span output, those were > mostly used for debugging > - More tests around Parse span in nested queries > - Remove explicit queryId in the tests ``` +--

Re: SLRU optimization - configurable buffer pool and partitioning the SLRU lock

2023-11-17 Thread Alvaro Herrera
On 2023-Nov-17, Dilip Kumar wrote: > On Thu, Nov 16, 2023 at 3:11 PM Alvaro Herrera > wrote: > > > > I just noticed that 0003 does some changes to > > TransactionGroupUpdateXidStatus() that haven't been adequately > > explained AFAICS. How do you know that these changes are safe? > > IMHO

Re: SLRU optimization - configurable buffer pool and partitioning the SLRU lock

2023-11-17 Thread Alvaro Herrera
In SlruSharedData, a new comment is added that starts: "Instead of global counter we maintain a bank-wise lru counter because ..." You don't need to explain what we don't do. Just explain what we do do. So remove the words "Instead of a global counter" from there, because they offer no wisdom.

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

2023-11-17 Thread Amul Sul
On Thu, Nov 16, 2023 at 7:05 PM Amul Sul wrote: > On Thu, Nov 16, 2023 at 2:50 AM Peter Eisentraut > wrote: > >> On 15.11.23 13:26, Amul Sul wrote: >> > Question: Why are you using AT_PASS_ADD_OTHERCONSTR? I don't know >> if >> > it's right or wrong, but if you have a specific reason,

Re: Synchronizing slots from primary to standby

2023-11-17 Thread Drouvot, Bertrand
Hi, On 11/17/23 2:46 AM, Zhijie Hou (Fujitsu) wrote: On Tuesday, November 14, 2023 10:27 PM Drouvot, Bertrand wrote: On 11/13/23 2:57 PM, Zhijie Hou (Fujitsu) wrote: On Friday, November 10, 2023 4:16 PM Drouvot, Bertrand wrote: Yeah good point, agree to just error out in all the case

Re: Synchronizing slots from primary to standby

2023-11-17 Thread Amit Kapila
On Thu, Nov 16, 2023 at 5:34 PM shveta malik wrote: > > PFA v35. > Review v35-0002* == 1. As quoted in the commit message, > If a logical slot is invalidated on the primary, slot on the standby is also invalidated. If a logical slot on the primary is valid but is invalidated on the

Re: Synchronizing slots from primary to standby

2023-11-17 Thread Drouvot, Bertrand
Hi, On 11/16/23 1:03 PM, shveta malik wrote: On Thu, Nov 16, 2023 at 3:43 PM Amit Kapila wrote: PFA v35. It has below changes: Thanks for the update! 6) shutdown the slotsync worker on promotion. + /* +* Shutdown the slot sync workers to prevent potential conflicts between +*

Re: Relation bulk write facility

2023-11-17 Thread Heikki Linnakangas
On 19/09/2023 17:13, Heikki Linnakangas wrote: The attached patch centralizes that pattern to a new bulk writing facility, and changes all those AMs to use it. Here's a new rebased version of the patch. This includes fixes to the pageinspect regression test. They were explained in the commit

Re: Hide exposed impl detail of wchar.c

2023-11-17 Thread John Naylor
On Fri, Nov 17, 2023 at 5:54 AM Nathan Bossart wrote: > > It looks like is_valid_ascii() was originally added to pg_wchar.h so that > it could easily be used elsewhere [0] [1], but that doesn't seem to have > happened yet. > > Would moving this definition to a separate header file be a viable

Re: trying again to get incremental backup

2023-11-17 Thread Alvaro Herrera
I made a pass over pg_combinebackup for NLS. I propose the attached patch. -- Álvaro HerreraBreisgau, Deutschland — https://www.EnterpriseDB.com/ "Right now the sectors on the hard disk run clockwise, but I heard a rumor that you can squeeze 0.2% more throughput by running them

Re: remaining sql/json patches

2023-11-17 Thread Alvaro Herrera
On 2023-Nov-17, Amit Langote wrote: > On Fri, Nov 17, 2023 at 4:27 PM jian he wrote: > > some enum declaration, ending element need an extra comma? > > Didn't know about the convention to have that comma, but I can see it > is present in most enum definitions. It's new. See commit

Re: remaining sql/json patches

2023-11-17 Thread Amit Langote
On Fri, Nov 17, 2023 at 4:27 PM jian he wrote: > hi. > minor issues. Thanks for checking. > In transformJsonFuncExpr(ParseState *pstate, JsonFuncExpr *func) > func.behavior->on_empty->location and > func.behavior->on_error->location are correct. > but in ExecInitJsonExpr,

Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

2023-11-17 Thread Andrei Zubkov
A little fix in "level_tracking" tests after merge. -- regards, Andrei Zubkov Postgres Professional From ed7531ba471061346922bbcb00d92738f6515a3f Mon Sep 17 00:00:00 2001 From: Andrei Zubkov Date: Fri, 17 Nov 2023 11:27:20 +0300 Subject: [PATCH 1/2] pg_stat_statements tests: Add NOT NULL