Re: [PATCH] Add a inline function to eliminate duplicate code

2022-08-05 Thread Junwang Zhao
Any more reviews? On Tue, Aug 2, 2022 at 9:24 PM mahendrakar s wrote: > > Patch is looking good to me. > > Thanks, > Mahendrakar. > > On Tue, 2 Aug 2022 at 16:57, Junwang Zhao wrote: >> >> abstract the logic of `scankey change attribute num to index col >> number` to

Re: Support logical replication of DDLs

2022-08-05 Thread Peter Smith
Hi Hou-san, here are my review comments for the patch v15-0001: == 1. Commit Message CREATE/ALTER/DROP TABLE (*) At first, I thought "(*)" looks like a SQL syntax element. SUGGESTION: CREATE/ALTER/DROP TABLE - - Note #1, Note #2 ... Note #1 – blah blah Note #2 – yada yada == 2.

Re: [Proposal] Fully WAL logged CREATE DATABASE - No Checkpoints

2022-08-05 Thread Dilip Kumar
On Fri, Aug 5, 2022 at 10:43 AM Dilip Kumar wrote: > > Yeah maybe it is not necessary to close as these unowned smgr will > automatically get closed on the transaction end. Actually the > previous person of the patch had both these comments fixed. The > reason for explicitly closing it is that

Re: collate not support Unicode Variation Selector

2022-08-05 Thread Kyotaro Horiguchi
At Thu, 4 Aug 2022 19:01:33 +0900, 荒井元成 wrote in > Thank you for your reply. > > SQLServer supports Unicode Variation Selector, so I would like PostgreSQL to > support them as well. I studied the code a bit further, then found that simple comparison can ignore selectors by using

Re: making relfilenodes 56 bits

2022-08-05 Thread Dilip Kumar
On Thu, Aug 4, 2022 at 5:01 PM Dilip Kumar wrote: > > On Sat, Jul 30, 2022 at 1:59 AM Robert Haas wrote: > > > One solution to all this is to do as Dilip proposes here: for system > > relations, keep assigning the OID as the initial relfilenumber. > > Actually, we really only need to do this for

Re: Patch to address creation of PgStat* contexts with null parent context

2022-08-05 Thread Kyotaro Horiguchi
At Thu, 04 Aug 2022 13:12:32 -0400, Reid Thompson wrote in > On Fri, 2022-07-29 at 11:53 +0900, Kyotaro Horiguchi wrote: > > > > That makes the memorycontext-tree structure unstable because > > CacheMemoryContext can be created on-the-fly. > > > > Honestly I don't like to call

Re: Patch to address creation of PgStat* contexts with null parent context

2022-08-05 Thread Kyotaro Horiguchi
At Fri, 05 Aug 2022 17:22:38 +0900 (JST), Kyotaro Horiguchi wrote in > Thus I thought that we may let pgstat_initialize() promptly allocate > the memory. > > Does it make sense? > > > About the patch, I had something like the attached in my mind. I haven't fully checked, but this change

RE: Perform streaming logical transactions by background workers and parallel apply

2022-08-05 Thread shiy.f...@fujitsu.com
On Thu, Aug 4, 2022 2:36 PM Wang, Wei/王 威 wrote: > > I also did some other improvements based on the suggestions posted in this > thread. Attach the new patches. > Thanks for updating the patch. Here are some comments on v20-0001 patch. 1. +typedef struct ApplyBgworkerShared +{ +

Re: enable/disable broken for statement triggers on partitioned tables

2022-08-05 Thread Amit Langote
On Fri, Aug 5, 2022 at 6:58 PM Alvaro Herrera wrote: > OK, pushed. This soon caused buildfarm to show a failure due to > underspecified ORDER BY, so I just pushed a fix for that too. Thank you. -- Thanks, Amit Langote EDB: http://www.enterprisedb.com

Re: Use fadvise in wal replay

2022-08-05 Thread Bharath Rupireddy
On Thu, Aug 4, 2022 at 9:48 PM Andrey Borodin wrote: > > > On 18 Jul 2022, at 22:55, Robert Haas wrote: > > > > On Thu, Jun 23, 2022 at 5:49 AM Jakub Wartak > > wrote: I have a fundamental question on the overall idea - How beneficial it will be if the process that's reading the current WAL

Re: Checking pgwin32_is_junction() errors

2022-08-05 Thread Thomas Munro
On Thu, Aug 4, 2022 at 9:42 AM Thomas Munro wrote: > On Thu, Aug 4, 2022 at 9:28 AM Andrew Dunstan wrote: > > On 2022-08-01 Mo 16:06, Andrew Dunstan wrote: > > > I'll try it out on fairywren/drongo. > > > They are happy with patches 2, 3, and 4. > > Thanks for testing! > > If there are no

Re: [PATCH] Reuse Workers and Replication Slots during Logical Replication

2022-08-05 Thread Melih Mutlu
Hi Amit, >> Why after step 4, do you need to drop the replication slot? Won't just > >> clearing the required info from the catalog be sufficient? > > > > > > The replication slots that we read from the catalog will not be used for > anything else after we're done with syncing the table which the

Re: annoyance with .git-blame-ignore-revs

2022-08-05 Thread Alvaro Herrera
On 2022-Aug-05, Andrew Dunstan wrote: > let's just backpatch the file and be done with it. I can do that in a couple of hours. -- Álvaro HerreraBreisgau, Deutschland — https://www.EnterpriseDB.com/ "You don't solve a bad join with SELECT DISTINCT" #CupsOfFail

Re: postgres_fdw: batch inserts vs. before row triggers

2022-08-05 Thread Etsuro Fujita
On Wed, Aug 3, 2022 at 2:24 PM Etsuro Fujita wrote: > To fix, I modified postgresGetForeignModifyBatchSize() to disable > batch insert when there are any such constraints, like when there are > any AFTER ROW triggers on the foreign table. Attached is a patch for > that. > > If there are no

Re: Cleaning up historical portability baggage

2022-08-05 Thread Thomas Munro
I've now pushed all of these except the --disable-thread-safety one, which I'm still contemplating. So far all green on the farm (except known unrelated breakage). But that's just the same-day animals...

Use pg_pwritev_with_retry() instead of write() in dir_open_for_write() to avoid partial writes?

2022-08-05 Thread Bharath Rupireddy
Hi, I noticed that dir_open_for_write() in walmethods.c uses write() for WAL file initialization (note that this code is used by pg_receivewal and pg_basebackup) as opposed to core using pg_pwritev_with_retry() in XLogFileInitInternal() to avoid partial writes. Do we need to fix this? Thoughts?

Re: Cleaning up historical portability baggage

2022-08-05 Thread Thomas Munro
On Thu, Aug 4, 2022 at 2:30 PM Andres Freund wrote: > On 2022-08-03 21:52:04 -0400, Tom Lane wrote: > > Andres Freund writes: > > > Another potential cleanup is the fallback for strtoll/strtoull. > > > > +1, I suspect the alternate spellings are dead. > > Looks like that includes systems where

Re: annoyance with .git-blame-ignore-revs

2022-08-05 Thread Andrew Dunstan
On 2022-08-04 Th 20:35, Peter Geoghegan wrote: > On Mon, Jul 11, 2022 at 12:30 PM Alvaro Herrera > wrote: >> $ git blame configure >> fatal: could not open object name list: .git-blame-ignore-revs >> >> My first workaround was to add empty .git-blame-ignore-revs in all >> checkouts. This was

Re: enable/disable broken for statement triggers on partitioned tables

2022-08-05 Thread Alvaro Herrera
OK, pushed. This soon caused buildfarm to show a failure due to underspecified ORDER BY, so I just pushed a fix for that too. Thanks Simon for reporting the problem, and thanks Amit for the patch. -- Álvaro HerreraBreisgau, Deutschland — https://www.EnterpriseDB.com/ "Si quieres ser

RE: Data is copied twice when specifying both child and parent table in publication

2022-08-05 Thread wangw.f...@fujitsu.com
On Thur, Jul 28, 2022 at 17:17 PM Peter Smith wrote: > Here are some review comments for the HEAD_v7-0001 patch: Thanks for your comments. > 2. Commit message. > > 2a. > > If there are two publications that publish the parent table and the child > table > separately, and both specify the

Re: automatically generating node support functions

2022-08-05 Thread Amit Kapila
On Wed, Aug 3, 2022 at 7:16 PM Tom Lane wrote: > > Amit Kapila writes: > > I have a question related to commit 964d01ae90. Today, after getting > > the latest code, when I compiled it on my windows machine, it lead to > > a compilation error because the outfuncs.funcs.c was not regenerated. > >

Re: [PATCH] Use indexes on the subscriber when REPLICA IDENTITY is full on the publisher

2022-08-05 Thread Amit Kapila
On Fri, Jul 22, 2022 at 9:45 PM Önder Kalacı wrote: >> >> >> BTW, do we want to consider partial indexes for the scan in this >> context? I mean it may not have data of all rows so how that would be >> usable? >> > > As far as I can see, check_index_predicates() never picks a partial index for >

Expr. extended stats are skipped with equality operator

2022-08-05 Thread Danny Shemesh
Hey all ! I'm on a quest to help the planner (on pg14) use the best of several partial, expressional indices we have on some large tables (few TBs in size, billions of records). As we know, stats for expressions in partial indices aren't gathered by default - so I'm tinkering with expressional

Re: BTMaxItemSize seems to be subtly incorrect

2022-08-05 Thread Peter Geoghegan
On Thu, Aug 4, 2022 at 10:40 PM Peter Geoghegan wrote: > This very likely has something to do with the way nbtdedup.c uses > BTMaxItemSize(), which apparently won't work on these 32-bit systems > now. Update: I discovered that I can get the regression tests to fail (even on mainstream 64-bit

Re: annoyance with .git-blame-ignore-revs

2022-08-05 Thread Alvaro Herrera
On 2022-Aug-05, Alvaro Herrera wrote: > On 2022-Aug-05, Andrew Dunstan wrote: > > > let's just backpatch the file and be done with it. > > I can do that in a couple of hours. Done. Thanks! -- Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/ "La rebeldía es la

Re: Cleaning up historical portability baggage

2022-08-05 Thread Robert Haas
On Fri, Aug 5, 2022 at 10:48 AM Tom Lane wrote: > Hmm ... I agree with you that the end result could be nicer code, > but what's making it nicer is a pretty substantial amount of human > effort for each and every call site. Is anybody stepping forward > to put in that amount of work? > > My

Re: Expr. extended stats are skipped with equality operator

2022-08-05 Thread Justin Pryzby
On Fri, Aug 05, 2022 at 04:43:36PM +0300, Danny Shemesh wrote: > 2. Less important, just a minor note - feel free to ignore - although the > eq. operator above seems to be skipped when matching the ext. stats, I can > work around this by using a CASE expression (fiddle >

Re: [doc] fix a potential grammer mistake

2022-08-05 Thread Robert Treat
On Thu, Aug 4, 2022 at 10:32 AM Daniel Gustafsson wrote: > > On 4 Aug 2022, at 00:44, Junwang Zhao wrote: > > > Attachment is a patch with the "just" removed. > > I think this is a change for better, so I've pushed it. Thanks for the > contribution! > > Thanks! Robert Treat https://xzilla.net

Re: A proposal for shared memory based backup infrastructure

2022-08-05 Thread Robert Haas
On Sat, Jul 30, 2022 at 2:54 AM mahendrakar s wrote: > There might be security concerns if the backup started by one user can be > stopped by another user. > This is because the user who stops the backup will get the backup_label or > table space map file contents of other user. > Isn't this a

Re: pgsql: BRIN: mask BRIN_EVACUATE_PAGE for WAL consistency checking

2022-08-05 Thread Alvaro Herrera
On 2022-Aug-05, Alvaro Herrera wrote: > Add a test that tickles the case, as branch testing technology allows. One point here is that this confirms that the backpatched renaming alias for PostgreSQL::Test::Cluster is working well. Another is that, as far as I know, this is the going to be the

Re: Cleaning up historical portability baggage

2022-08-05 Thread Robert Haas
On Sat, Jul 23, 2022 at 8:23 PM Tom Lane wrote: > More generally, I'm not exactly convinced that changes like > this are a readability improvement: > > -#ifdef HAVE_SETSID > +#ifndef WIN32 > > I'd rather not have the code cluttered with a sea of > indistinguishable "#ifndef WIN32" tests when some

Re: Cleaning up historical portability baggage

2022-08-05 Thread Tom Lane
Robert Haas writes: > Overall, I don't think it's a great idea to keep all of these > HAVE_WHATEVER macros around if the configure tests are gone. It might > be necessary in the short term to make sure we don't regress the > readability of the code, but I think it would be better to come up >

Re: optimize lookups in snapshot [sub]xip arrays

2022-08-05 Thread Nathan Bossart
On Fri, Aug 05, 2022 at 11:02:15AM +0700, John Naylor wrote: > That is a good point. Maybe potential helpers in simd.h should only deal > specifically with vector registers, with it's users providing C fallbacks. > I don't have any good ideas of where to put the new function, though. I moved it

Re: Proposal: Support custom authentication methods using hooks

2022-08-05 Thread Stephen Frost
Greetings, * Andres Freund (and...@anarazel.de) wrote: > On 2022-08-03 17:21:58 -0400, Stephen Frost wrote: > > * Andres Freund (and...@anarazel.de) wrote: > > > On 2022-08-03 16:28:08 -0400, Stephen Frost wrote: > > > > Again, server-side only is not interesting and not a direction that > > > >

Re: PG 15 (and to a smaller degree 14) regression due to ExprEvalStep size

2022-08-05 Thread Andres Freund
Hi, I tried to look into some of the questions from Amit, but I have e.g. no idea what exactly the use of subtransactions tries to achieve - afaics 1a36bc9dba8 is the first patch to introduce needing to evaluate parts expressions in a subtransaction - but there's not a single comment explaining

Draft back-branch release notes are up

2022-08-05 Thread Tom Lane
... at https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=aab05919a685449826db986a921c1d8632d673e0 Please send corrections and comments by Sunday. regards, tom lane

Re: [Proposal] Fully WAL logged CREATE DATABASE - No Checkpoints

2022-08-05 Thread Andres Freund
Hi, On 2022-08-04 19:14:08 -0400, Tom Lane wrote: > Andres Freund writes: > > On 2022-08-04 18:05:25 -0400, Tom Lane wrote: > >> In any case, DROP DATABASE is far from the only place with a problem. > > > What other place has a database corrupting potential of this magnitude just > > because

Re: Cleaning up historical portability baggage

2022-08-05 Thread Andres Freund
Hi, On 2022-08-06 09:02:32 +1200, Thomas Munro wrote: > On Sat, Aug 6, 2022 at 12:01 AM Thomas Munro wrote: > > On Thu, Aug 4, 2022 at 2:30 PM Andres Freund wrote: > >> [strtoll cleanup patch] > > > > LGTM. This is just C99 stuff, and my scraped config data > > set agrees with your

Re: Expr. extended stats are skipped with equality operator

2022-08-05 Thread Tom Lane
Justin Pryzby writes: > A reproducer for this: > CREATE TABLE t1(x int[], y float); > INSERT INTO t1 SELECT array[1], a FROM generate_series(1,99)a; > CREATE STATISTICS s2 ON (CASE x[1] WHEN 1 THEN true ELSE false END), y FROM > t1; > ANALYZE t1; > explain analyze SELECT * FROM t1 WHERE CASE

Re: standby recovery fails (tablespace related) (tentative patch and discussion)

2022-08-05 Thread Alvaro Herrera
On 2022-Jul-30, Tom Lane wrote: > BTW, quite aside from stability, is it really necessary for this test to > be so freakin' slow? florican for instance reports > > [12:54:07] t/033_replay_tsp_drops.pl ok 117840 ms ( 0.01 usr > 0.00 sys + 8.72 cusr 5.41 csys = 14.14 CPU) > >

Re: Proposal to provide the facility to set binary format output for specific OID's per session

2022-08-05 Thread Justin Pryzby
On Tue, Jul 26, 2022 at 08:11:04AM -0400, Dave Cramer wrote: > Attached patch to correct these deficiencies. You sent a patch to be applied on top of the first patch, but cfbot doesn't know that, so it says the patch doesn't apply. http://cfbot.cputube.org/dave-cramer.html BTW, a previous

Re: optimize lookups in snapshot [sub]xip arrays

2022-08-05 Thread Andres Freund
Hi, On 2022-08-05 13:25:10 -0700, Nathan Bossart wrote: > I went ahead and renamed it to pg_lfind32() and switched it back to > returning the pointer. That felt the cleanest from the naming perspective, > but as Andres noted, it might not be as fast as just looking for the > presence of the

Re: BTMaxItemSize seems to be subtly incorrect

2022-08-05 Thread Peter Geoghegan
On Fri, Aug 5, 2022 at 10:13 AM Peter Geoghegan wrote: > Update: I discovered that I can get the regression tests to fail (even > on mainstream 64-bit platforms) by MAXALIGN()'ing the expression that > we assign to state->maxpostingsize at the top of _bt_dedup_pass(). Looks like this was nothing

Re: Hash index build performance tweak from sorting

2022-08-05 Thread David Zhang
On 2022-08-01 8:37 a.m., Simon Riggs wrote: Using the above test case, I'm getting a further 4-7% improvement on already committed code with the attached patch, which follows your proposal. I ran two test cases: for committed patch `hash_sort_by_hash.v3.patch`, I can see about 6 ~ 7%

Re: Cleaning up historical portability baggage

2022-08-05 Thread Thomas Munro
On Sat, Aug 6, 2022 at 12:01 AM Thomas Munro wrote: > On Thu, Aug 4, 2022 at 2:30 PM Andres Freund wrote: >> [strtoll cleanup patch] > > LGTM. This is just C99 stuff, and my scraped config data > set agrees with your observation. I found a couple of explicit references to these macros left in

Re: Cleaning up historical portability baggage

2022-08-05 Thread Andres Freund
Hi, On 2022-08-05 14:08:23 -0700, Andres Freund wrote: > Hah, I was about to push it. Thanks for catching these. Happy for you to push > this soon! Thanks. Next in my quest for reducing autoconf vs meson pg_config.h differences is GETTIMEOFDAY stuff. HAVE_GETTIMEOFDAY currently is only defined

Re: Cleaning up historical portability baggage

2022-08-05 Thread Thomas Munro
On Sat, Aug 6, 2022 at 12:03 PM Andres Freund wrote: > HAVE_GETTIMEOFDAY currently is only defined for mingw as the configure test is > gated to windows - that's somewhat weird imo. mingw has had it since at least > 2007. The attached patch makes the gettimeofday() fallback specific to msvc. +1

Re: [RFC] building postgres with meson -v

2022-08-05 Thread Andres Freund
Hi, On 2021-10-31 16:24:48 -0700, Andres Freund wrote: > - support for building docs. > I couldn't get dbtoepub work in a vpath style build, so I changed that > to also use pandoc. No idea if anybody uses the epub rules? combing through various FIXMEs in the meson patch I started to look

Remaining case where reltuples can become distorted across multiple VACUUM operations

2022-08-05 Thread Peter Geoghegan
My bugfix commit 74388a1a (which was pushed back in February) added heuristics to VACUUM's reltuples calculation/estimate. This prevented VACUUM from distorting our estimate of reltuples over time, across successive VACUUM operations run against the same table. The problem was that VACUUM could

Re: Use fadvise in wal replay

2022-08-05 Thread Andrey Borodin
Hi Bharath, thank you for the suggestion. > On 5 Aug 2022, at 16:02, Bharath Rupireddy > wrote: > > On Thu, Aug 4, 2022 at 9:48 PM Andrey Borodin wrote: >> >>> On 18 Jul 2022, at 22:55, Robert Haas wrote: >>> >>> On Thu, Jun 23, 2022 at 5:49 AM Jakub Wartak >>> wrote: > > I have a

Re: Cleaning up historical portability baggage

2022-08-05 Thread Tom Lane
Andres Freund writes: > I've renamed the file to win32gettimeofday now. I wonder if we should rename > files that are specific to msvc to indicate that? But that's for later. +1, but you didn't change the file's own comments containing its name. regards, tom lane

Re: Checking pgwin32_is_junction() errors

2022-08-05 Thread Thomas Munro
On Fri, Aug 5, 2022 at 9:17 PM Thomas Munro wrote: > Hmm, POSIX says st_link should contain the length of a symlink's > target path, so I suppose we should probably set that even though we > never consult it. Here's a version that does that. I also removed > the rest of the now redundant #ifdef

Re: Cleaning up historical portability baggage

2022-08-05 Thread Andres Freund
Hi, On 2022-08-03 14:25:01 +1200, Thomas Munro wrote: > It'd be good to find a new home for pg_get_user_name() and > pg_get_user_home_dir(), which really shouldn't be left in the now > bogusly named src/port/thread.c. Any suggestions? Leaving the name aside, the win32 handling of these