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

2022-09-27 Thread kuroda.hay...@fujitsu.com
Dear Önder: Thank you for updating patch! Your documentation seems OK, and I could not find any other places to be added Followings are my comments. 01 relation.c - general Many files are newly included. I was not sure but some codes related with planner may be able to move to

Re: Extend win32 error codes to errno mapping in win32error.c

2022-09-27 Thread Michael Paquier
On Wed, Sep 28, 2022 at 11:14:53AM +0530, Bharath Rupireddy wrote: > IMO, we can add mapping for just ERROR_INVALID_NAME which is an > obvious error code and easy to hit, leaving others. There are actually > many win32 error codes that may get hit in our code base and actually > mapping everything

Re: [PATCH] Add peer authentication TAP test

2022-09-27 Thread Michael Paquier
On Fri, Aug 26, 2022 at 10:43:43AM +0200, Drouvot, Bertrand wrote: > During the work in [1] we created a new TAP test to test the SYSTEM_USER > behavior with peer authentication. > > It turns out that there is currently no TAP test for the peer > authentication, so we think (thanks Michael for

Re: Fix some newly modified tab-complete changes

2022-09-27 Thread Kyotaro Horiguchi
At Wed, 28 Sep 2022 14:14:01 +1000, Peter Smith wrote in > On Tue, Sep 27, 2022 at 8:28 PM shiy.f...@fujitsu.com > wrote: > > > > Hi hackers, > > > > I saw a problem when using tab-complete for "GRANT", "TABLES IN SCHEMA" > > should > > be "ALL TABLES IN SCHEMA" in the following case. > > > >

Re: longfin and tamandua aren't too happy but I'm not sure why

2022-09-27 Thread Thomas Munro
On Wed, Sep 28, 2022 at 6:15 PM Tom Lane wrote: > There may be a larger conversation to be had here about how > much our CI infrastructure should be detecting. There seems > to be a depressingly large gap between what that found and > what the buildfarm is finding --- not only in portability >

Re: Extend win32 error codes to errno mapping in win32error.c

2022-09-27 Thread Bharath Rupireddy
On Wed, Sep 28, 2022 at 10:10 AM Michael Paquier wrote: > > One important thing, in my opinion, when it comes to updating this > table, is that it could be better to report the original error number > if errno can be somewhat confusing for the mapping. Returning errno = e instead of EINVAL in

Re: longfin and tamandua aren't too happy but I'm not sure why

2022-09-27 Thread Tom Lane
Dilip Kumar writes: > wrasse is also failing with a bus error, Yeah. At this point I think it's time to call for this patch to get reverted. It should get tested *off line* on some non-Intel, non-64-bit, alignment-picky architectures before the rest of us have to deal with it any more. There

Re: longfin and tamandua aren't too happy but I'm not sure why

2022-09-27 Thread Dilip Kumar
wrasse is also failing with a bus error, but I cannot get the stack trace. So it seems it is hitting some alignment issues during startup [1]. Is it possible to get the backtrace or lineno? [1] 2022-09-28 03:19:26.228 CEST [180:4] LOG: redo starts at 0/30FE9D8 2022-09-28 03:19:27.674 CEST

Re: Avoid memory leaks during base backups

2022-09-27 Thread Bharath Rupireddy
On Wed, Sep 28, 2022 at 10:19 AM Tom Lane wrote: > > Bharath Rupireddy writes: > > I had the same opinion. Here's what I think - for backup functions, we > > can have the new memory context child of TopMemoryContext and for > > perform_base_backup(), we can have the memory context child of > >

Re: Avoid memory leaks during base backups

2022-09-27 Thread Tom Lane
Bharath Rupireddy writes: > I had the same opinion. Here's what I think - for backup functions, we > can have the new memory context child of TopMemoryContext and for > perform_base_backup(), we can have the memory context child of > CurrentMemoryContext. With PG_TRY()-PG_FINALLY()-PG_END_TRY(),

RE: [small patch] Change datatype of ParallelMessagePending from "volatile bool" to "volatile sig_atomic_t"

2022-09-27 Thread kuroda.hay...@fujitsu.com
Dear Michael, > Yeah, at least as of the cancel callback psql_cancel_callback() that > handle_sigint() would call on SIGINT as this is set by psql. So it > does not seem right to use a boolean rather than a sig_atomic_t in > this case, as well. PSA fix patch. Note that

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

2022-09-27 Thread Bharath Rupireddy
On Wed, Sep 28, 2022 at 12:32 AM Thomas Munro wrote: > > On Wed, Sep 28, 2022 at 1:03 AM Bharath Rupireddy > wrote:> > > On Tue, Sep 27, 2022 at 3:01 PM Thomas Munro wrote: > > > Something like the attached. > > > > Isn't it also better to add notes in win32pread.c and win32pwrite.c > > about

Re: Extend win32 error codes to errno mapping in win32error.c

2022-09-27 Thread Michael Paquier
On Tue, Sep 27, 2022 at 03:23:04PM +0530, Bharath Rupireddy wrote: > The failure occurs in dosmaperr() in win32error.c due to an unmapped > errno for win32 error code. The error code 123 i.e. ERROR_INVALID_NAME > says "The file name, directory name, or volume label syntax is > incorrect." [2], the

Re: Avoid memory leaks during base backups

2022-09-27 Thread Bharath Rupireddy
On Wed, Sep 28, 2022 at 9:46 AM Michael Paquier wrote: > > On Tue, Sep 27, 2022 at 05:32:26PM +0900, Kyotaro Horiguchi wrote: > > At Tue, 27 Sep 2022 11:33:56 +0530, Bharath Rupireddy > > wrote in > > > On Mon, Sep 26, 2022 at 7:34 PM Tom Lane wrote: > > > > This ... seems like inventing your

Re: Avoid memory leaks during base backups

2022-09-27 Thread Michael Paquier
On Tue, Sep 27, 2022 at 05:32:26PM +0900, Kyotaro Horiguchi wrote: > At Tue, 27 Sep 2022 11:33:56 +0530, Bharath Rupireddy > wrote in > > On Mon, Sep 26, 2022 at 7:34 PM Tom Lane wrote: > > > This ... seems like inventing your own shape of wheel. The > > > normal mechanism for preventing this

Re: Fix some newly modified tab-complete changes

2022-09-27 Thread Peter Smith
On Tue, Sep 27, 2022 at 8:28 PM shiy.f...@fujitsu.com wrote: > > Hi hackers, > > I saw a problem when using tab-complete for "GRANT", "TABLES IN SCHEMA" should > be "ALL TABLES IN SCHEMA" in the following case. > > postgres=# grant all on > ALL FUNCTIONS IN SCHEMA DATABASE

Re: longfin and tamandua aren't too happy but I'm not sure why

2022-09-27 Thread Dilip Kumar
On Wed, Sep 28, 2022 at 2:59 AM Tom Lane wrote: > > ... also, lapwing's not too happy [1]. The alter_table test > expects this to yield zero rows, but it doesn't: By looking at regression diff as shown below, it seems that we are able to get the relfilenode from the Oid using

Re: Refactor backup related code (was: Is it correct to say, "invalid data in file \"%s\"", BACKUP_LABEL_FILE in do_pg_backup_stop?)

2022-09-27 Thread Kyotaro Horiguchi
At Wed, 28 Sep 2022 10:09:39 +0900, Michael Paquier wrote in > On Tue, Sep 27, 2022 at 03:11:54PM +0530, Bharath Rupireddy wrote: > > On Tue, Sep 27, 2022 at 2:20 PM Kyotaro Horiguchi > > wrote: > >> If this is still unacceptable, I propose to change the comment. (I > >> found that the

Re: making relfilenodes 56 bits

2022-09-27 Thread Thomas Munro
Hi Dilip, I am very happy to see these commits. Here's some belated review for the tombstone-removal patch. > v7-0004-Don-t-delay-removing-Tombstone-file-until-next.patch More things you can remove: * sync_unlinkfiletag in struct SyncOps * the macro UNLINKS_PER_ABSORB * global variable

Re: [PoC] Improve dead tuple storage for lazy vacuum

2022-09-27 Thread Masahiko Sawada
On Fri, Sep 23, 2022 at 12:11 AM John Naylor wrote: > > > On Thu, Sep 22, 2022 at 11:46 AM John Naylor > wrote: > > One thing I want to try soon is storing fewer than 16/32 etc entries, so > > that the whole node fits comfortably inside a power-of-two allocation. That > > would allow us to

Re: Insertion Sort Improvements

2022-09-27 Thread John Naylor
On Tue, Sep 27, 2022 at 4:39 PM Benjamin Coutu wrote: > > Getting back to improvements for small sort runs, it might make sense to consider using in-register based sorting via sorting networks for very small runs. > It looks like some of the commercial DBMSs do this very successfully. "Looks

Re: A doubt about a newly added errdetail

2022-09-27 Thread Amit Kapila
On Tue, Sep 27, 2022 at 6:12 PM Alvaro Herrera wrote: > > While reading this code, I noticed that function expr_allowed_in_node() > has a very strange API: it doesn't have any return convention at all > other than "if we didn't modify errdetail_str then all is good". I was > tempted to add an

Re: SYSTEM_USER reserved word implementation

2022-09-27 Thread Michael Paquier
On Tue, Sep 27, 2022 at 03:38:49PM -0700, Jacob Champion wrote: > On 9/26/22 06:29, Drouvot, Bertrand wrote: > Since there are only internal clients to the API, I'd argue this makes > more sense as an Assert(authn_id != NULL), but I don't think it's a > dealbreaker. Using an assert() looks like a

Re: Strip -mmacosx-version-min options from plperl build

2022-09-27 Thread Andres Freund
Hi, On 2022-08-30 09:35:51 -0400, Andrew Dunstan wrote: > On 2022-08-26 Fr 16:25, Andres Freund wrote: > > On 2022-08-26 16:00:31 -0400, Tom Lane wrote: > >> Andrew Dunstan writes: > >>> On 2022-08-26 Fr 12:11, Tom Lane wrote: > And if that doesn't help, try -Wl,--export-all-symbols > >>>

Re: Assign TupleTableSlot->tts_tableOid duplicated in tale AM.

2022-09-27 Thread Peter Geoghegan
On Tue, Sep 20, 2022 at 11:51 PM Wenchao Zhang wrote: > We can get the two assigned values are same by reading codes. Maybe we should > remove one? > > What's more, we find that maybe we assign slot->tts_tableOid in outer > interface like table_scan_getnextslot may be better and more friendly

meson vs mingw, plpython, readline and other fun

2022-09-27 Thread Andres Freund
Hi, Melih mentioned on IM that while he could build postgres with meson on windows w/ mingw, the tests didn't run. Issues: - The bit computing PATH to the temporary installation for running tests only dealt with backward slashes in paths on windows, because that's what python.org python

Re: GUC tables - use designated initializers

2022-09-27 Thread Peter Smith
On Wed, Sep 28, 2022 at 2:21 AM Tom Lane wrote: > > Peter Smith writes: > > Enums index a number of the GUC tables. This all relies on the > > elements being carefully arranged to be in the same order as those > > enums. There are comments to say what enum index belongs to each table > >

Re: [small patch] Change datatype of ParallelMessagePending from "volatile bool" to "volatile sig_atomic_t"

2022-09-27 Thread Michael Paquier
On Tue, Sep 27, 2022 at 01:38:26AM +, kuroda.hay...@fujitsu.com wrote: > Maybe you mentioned about sigint_interrupt_enabled, > and it also seems to be modified in the signal handler. Yeah, at least as of the cancel callback psql_cancel_callback() that handle_sigint() would call on SIGINT as

Re: [PATCH] Log details for client certificate failures

2022-09-27 Thread Masahiko Sawada
On Wed, Sep 28, 2022 at 4:44 AM Jacob Champion wrote: > > On Tue, Sep 27, 2022 at 1:51 AM Masahiko Sawada wrote: > > I think we can fix it by the attached patch but I'd like to discuss > > whether it's worth fixing it. > > Whoops. So every time it's changed, we leak a little postmaster memory?

Re: Refactor backup related code (was: Is it correct to say, "invalid data in file \"%s\"", BACKUP_LABEL_FILE in do_pg_backup_stop?)

2022-09-27 Thread Michael Paquier
On Tue, Sep 27, 2022 at 03:11:54PM +0530, Bharath Rupireddy wrote: > On Tue, Sep 27, 2022 at 2:20 PM Kyotaro Horiguchi > wrote: >> If this is still unacceptable, I propose to change the comment. (I >> found that the previous patch forgets about do_pg_backup_stop()) >> >> - * It fills in

Re: Adding a clang-format file

2022-09-27 Thread Peter Geoghegan
On Tue, Sep 27, 2022 at 5:35 AM Aleksander Alekseev wrote: > Personally I don't have anything against the idea. TimescaleDB uses > clang-format to mimic pgindent and it works quite well. One problem > worth mentioning though is that the clang-format file is dependent on > the particular version

Re: GUC values - recommended way to declare the C variables?

2022-09-27 Thread Peter Smith
On Tue, Sep 27, 2022 at 11:07 AM Peter Smith wrote: > ... > I will try to post a patch in the new few days to address (per your > suggestions) some of the variables that I am more familiar with. > PSA a small patch to tidy a few of the GUC C variables - adding comments and removing unnecessary

Re: Allow foreign keys to reference a superset of unique columns

2022-09-27 Thread Kaiting Chen
> So the broader point I'm trying to make is that, as I understand it, > indexes backing foreign key constraints is an implementation detail. > The SQL standard details the behavior of foreign key constraints > regardless of implementation details like a backing index. That means > that the

Re: Allow foreign keys to reference a superset of unique columns

2022-09-27 Thread Kaiting Chen
>>> Another example is that I think the idea is only well-defined when >>> the subset column(s) are a primary key, or at least all marked NOT NULL. >>> Otherwise they're not as unique as you're claiming. But then the FK >>> constraint really has to be dependent on a PK constraint not just an >>>

Re: SYSTEM_USER reserved word implementation

2022-09-27 Thread Jacob Champion
On 9/26/22 06:29, Drouvot, Bertrand wrote: > Please find attached V4 taking care of Jacob's previous comments. > + /* > + * InitializeSystemUser should already be called once we are sure that > + * authn_id is not NULL (means auth_method is actually valid). > + * But keep the

Re: Allow foreign keys to reference a superset of unique columns

2022-09-27 Thread Tom Lane
Kaiting Chen writes: >> Another example is that I think the idea is only well-defined when >> the subset column(s) are a primary key, or at least all marked NOT NULL. >> Otherwise they're not as unique as you're claiming. But then the FK >> constraint really has to be dependent on a PK

Re: Allow foreign keys to reference a superset of unique columns

2022-09-27 Thread Kaiting Chen
> Could we create this additional unique constraint implicitly, when using > FOREIGN KEY ... REFERENCES on a superset of unique columns? This would > make it easier to use, but still give proper information_schema output. Currently, a foreign key declared where the referenced columns have only a

Re: Allow foreign keys to reference a superset of unique columns

2022-09-27 Thread Kaiting Chen
> As I was reading through the email chain I had this thought: could you > get the same benefit (or 90% of it anyway) by instead allowing the > creation of a uniqueness constraint that contains more columns than > the index backing it? So long as the index backing it still guaranteed > the

Re: longfin and tamandua aren't too happy but I'm not sure why

2022-09-27 Thread Tom Lane
Robert Haas writes: > On Tue, Sep 27, 2022 at 4:50 PM Tom Lane wrote: >> There is a second problem that I am going to hold your feet to the >> fire about: >> (lldb) p sizeof(SharedInvalidationMessage) >> (unsigned long) $1 = 24 > Also, I don't really know what problem you think it's going to

Re: [PoC] Federated Authn/z with OAUTHBEARER

2022-09-27 Thread Jacob Champion
On Mon, Sep 26, 2022 at 6:39 PM Andrey Chudnovsky wrote: > For the providing token directly, that would be primarily used for > scenarios where the same party controls both the server and the client > side wrapper. > I.e. The client knows how to get a token for a particular principal > and

Re: Allow foreign keys to reference a superset of unique columns

2022-09-27 Thread Kaiting Chen
> For one example of where the semantics get fuzzy, it's not > very clear how the extra-baggage columns ought to participate in > CASCADE updates. Currently, if we have >CREATE TABLE foo (a integer PRIMARY KEY, b integer); > then an update that changes only foo.b doesn't need to update >

Re: longfin and tamandua aren't too happy but I'm not sure why

2022-09-27 Thread Tom Lane
... also, lapwing's not too happy [1]. The alter_table test expects this to yield zero rows, but it doesn't: SELECT m.* FROM filenode_mapping m LEFT JOIN pg_class c ON c.oid = m.oid WHERE c.oid IS NOT NULL OR m.mapped_oid IS NOT NULL; I've reproduced that symptom in a 32-bit FreeBSD VM

Re: longfin and tamandua aren't too happy but I'm not sure why

2022-09-27 Thread Robert Haas
On Tue, Sep 27, 2022 at 4:50 PM Tom Lane wrote: > I wrote: > > * frame #0: 0x00010a36af8c postgres`ParseCommitRecord(info='\x80', > > xlrec=0x7fa0678a8090, parsed=0x7ff7b5c50e78) at xactdesc.c:102:30 > > Okay, so the problem is this: by widening RelFileNumber to 64 bits, > you have

Re: longfin and tamandua aren't too happy but I'm not sure why

2022-09-27 Thread Tom Lane
I wrote: > * frame #0: 0x00010a36af8c postgres`ParseCommitRecord(info='\x80', > xlrec=0x7fa0678a8090, parsed=0x7ff7b5c50e78) at xactdesc.c:102:30 Okay, so the problem is this: by widening RelFileNumber to 64 bits, you have increased the alignment requirement of struct

Re: longfin and tamandua aren't too happy but I'm not sure why

2022-09-27 Thread Tom Lane
Justin Pryzby writes: > On Tue, Sep 27, 2022 at 02:55:18PM -0400, Robert Haas wrote: >> Both animals are running with -fsanitize=alignment and it's not >> difficult to believe that the commit mentioned above could have >> introduced an alignment problem where we didn't have one before, but >>

Re: Convert *GetDatum() and DatumGet*() macros to inline functions

2022-09-27 Thread Tom Lane
Peter Eisentraut writes: > On 26.09.22 19:34, Tom Lane wrote: >> I think we can do this while still having reasonable type-safety >> by adding AssertVariableIsOfTypeMacro() checks to the macros. > (I had looked into AssertVariableIsOfTypeMacro() for an earlier variant > of this patch, before I

Re: longfin and tamandua aren't too happy but I'm not sure why

2022-09-27 Thread Justin Pryzby
On Tue, Sep 27, 2022 at 02:55:18PM -0400, Robert Haas wrote: > Both animals are running with -fsanitize=alignment and it's not > difficult to believe that the commit mentioned above could have > introduced an alignment problem where we didn't have one before, but > without a stack backtrace I

Re: [PATCH] Log details for client certificate failures

2022-09-27 Thread Jacob Champion
On Tue, Sep 27, 2022 at 1:51 AM Masahiko Sawada wrote: > I think we can fix it by the attached patch but I'd like to discuss > whether it's worth fixing it. Whoops. So every time it's changed, we leak a little postmaster memory? Your patch looks good to me and I see no reason not to fix it.

Re: Convert *GetDatum() and DatumGet*() macros to inline functions

2022-09-27 Thread Peter Eisentraut
On 26.09.22 19:34, Tom Lane wrote: I think we can do this while still having reasonable type-safety by adding AssertVariableIsOfTypeMacro() checks to the macros. An advantage of that solution is that we verify that the code will be safe for a 32-bit build even in 64-bit builds. (Of course, it's

Re: pgsql: Increase width of RelFileNumbers from 32 bits to 56 bits.

2022-09-27 Thread Justin Pryzby
On Tue, Sep 27, 2022 at 03:12:56PM -0400, Robert Haas wrote: > On Tue, Sep 27, 2022 at 2:51 PM Justin Pryzby wrote: > > This seems to be breaking cfbot: > > https://cirrus-ci.com/github/postgresql-cfbot/postgresql > > > > For example: > > https://cirrus-ci.com/task/6720256776339456 > > OK, so it

Re: pgsql: Increase width of RelFileNumbers from 32 bits to 56 bits.

2022-09-27 Thread Robert Haas
On Tue, Sep 27, 2022 at 2:51 PM Justin Pryzby wrote: > This seems to be breaking cfbot: > https://cirrus-ci.com/github/postgresql-cfbot/postgresql > > For example: > https://cirrus-ci.com/task/6720256776339456 OK, so it looks like the pg_buffercache test is failing there. But it doesn't fail for

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

2022-09-27 Thread Thomas Munro
On Wed, Sep 28, 2022 at 1:03 AM Bharath Rupireddy wrote:> > On Tue, Sep 27, 2022 at 3:01 PM Thomas Munro wrote: > > Something like the attached. > > Isn't it also better to add notes in win32pread.c and win32pwrite.c > about the callers doing lseek(SEEK_SET) if they wish to and Windows >

longfin and tamandua aren't too happy but I'm not sure why

2022-09-27 Thread Robert Haas
Hi, longfin and tamandua recently begun failing like this, quite possibly as a result of 05d4cbf9b6ba708858984b01ca0fc56d59d4ec7c: +++ regress check in contrib/test_decoding +++ test ddl ... FAILED (test process exited with exit code 2) 3276 ms (all other tests in

Re: pg_stat_bgwriter.buffers_backend is pretty meaningless (and more?)

2022-09-27 Thread Melanie Plageman
v30 attached rebased and pgstat_io_ops.c builds with meson now also, I tested with pgstat_report_stat() only flushing when forced and tests still pass From 153b06200b36891c9e07df526a86dbd913e36e3e Mon Sep 17 00:00:00 2001 From: Melanie Plageman Date: Thu, 11 Aug 2022 18:28:50 -0400 Subject:

Re: pgsql: Fix perltidy breaking perlcritic

2022-09-27 Thread Dagfinn Ilmari Mannsåker
[resending to -hackers instead of -committers] Andrew Dunstan writes: > On Fri, Sep 9, 2022 at 10:44 PM John Naylor > wrote: > >> On Fri, Sep 9, 2022 at 3:32 AM Andrew Dunstan wrote: >> >> > A better way do do this IMNSHO is to put the eval in a block on its own >> along with the no critic

Re: making relfilenodes 56 bits

2022-09-27 Thread Robert Haas
On Tue, Sep 27, 2022 at 2:33 AM Dilip Kumar wrote: > Looks fine to me. OK, committed. I also committed the 0002 patch with some wordsmithing, and I removed a < 0 test an an unsigned value because my compiler complained about it. 0001 turned out to make headerscheck sad, so I just pushed a fix

Re: Backends stalled in 'startup' state

2022-09-27 Thread Ashwin Agrawal
On Thu, Sep 15, 2022 at 4:42 PM Ashwin Agrawal wrote: > > We recently saw many backends (close to max_connection limit) get stalled > in 'startup' in one of the production environments for Greenplum (fork of > PostgreSQL). Tracing the reason, it was found all the tuples created by > bootstrap

Re: Temporary file access API

2022-09-27 Thread John Morris
* So I think that code simplification and easy adoption of in-memory data changes (such as encryption or compression) are two rather distinct goals. admit that I'm running out of ideas how to develop a framework that'd be useful for both. I’m also wondering about code simplification vs a more

Re: GUC tables - use designated initializers

2022-09-27 Thread Tom Lane
Peter Smith writes: > Enums index a number of the GUC tables. This all relies on the > elements being carefully arranged to be in the same order as those > enums. There are comments to say what enum index belongs to each table > element. > But why not use designated initializers to enforce what

Re: kerberos/001_auth test fails on arm CPU darwin

2022-09-27 Thread Nazir Bilal Yavuz
Hi, Thanks for the reviews! On 9/27/2022 5:21 AM, Tom Lane wrote: Andres Freund writes: Maybe we should rely on PATH, rather than hardcoding OS dependent locations? My suggestion to consult krb5-config first was meant to allow PATH to influence the results. However, if that doesn't work,

Re: DROP OWNED BY is broken on master branch.

2022-09-27 Thread Robert Haas
On Tue, Sep 27, 2022 at 2:53 AM Rushabh Lathia wrote: > Yes, I was also thinking to avoid the duplicate logic but couldn't found > a way. I did the quick testing with the patch, and reported test is working > fine. But "make check" is failing with few failures. Oh, woops. There was a dumb

Re: XID formatting and SLRU refactorings (was: Add 64-bit XIDs into PostgreSQL 15)

2022-09-27 Thread Hamid Akhtar
On Tue, 26 Jul 2022 at 21:35, Simon Riggs wrote: > On Fri, 15 Jul 2022 at 12:29, Aleksander Alekseev > wrote: > > Personally I didn't expect that > > merging patches in this thread would take that long. They are in > > "Ready for Committer" state for a long time now and there are no known > >

Re: Differentiate MERGE queries with different structures

2022-09-27 Thread bt22nakamorit
2022-09-27 17:48 に Alvaro Herrera さんは書きました: Pushed, thanks. The code and the tests went fine on my environment. Thank you Alvaro for your help, and thank you Julien for your review! Best, Tatsuhiro Nakamori

Re: An attempt to avoid locally-committed-but-not-replicated-to-standby-transactions in synchronous replication

2022-09-27 Thread Bharath Rupireddy
On Tue, Aug 9, 2022 at 2:16 PM Bharath Rupireddy wrote: > > I've explained the problem with the current HA setup with synchronous > replication upthread at [1]. Let me reiterate it here once again. > > When a query is cancelled (a simple stroke of CTRL+C or > pg_cancel_backend() call) while the

Re: [RFC] building postgres with meson - v13

2022-09-27 Thread Peter Eisentraut
On 26.09.22 18:35, Andres Freund wrote: 9f5be26c1215 meson: Add docs for building with meson I do like the overall layout of this. The "Supported Platforms" section should be moved back to near the end of the chapter. I don't see a reason to move it forward, at least none that is related to

Re: A doubt about a newly added errdetail

2022-09-27 Thread Alvaro Herrera
While reading this code, I noticed that function expr_allowed_in_node() has a very strange API: it doesn't have any return convention at all other than "if we didn't modify errdetail_str then all is good". I was tempted to add an "Assert(*errdetail_msg == NULL)" at the start of it, just to make

Re: Adding a clang-format file

2022-09-27 Thread Aleksander Alekseev
Hi Peter, > I really don't see any real problem with making something available, > without changing any official project guidelines. It's commonplace to > provide a clang-format file these days. Personally I don't have anything against the idea. TimescaleDB uses clang-format to mimic pgindent

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

2022-09-27 Thread houzj.f...@fujitsu.com
On Tuesday, September 27, 2022 2:32 PM Kuroda, Hayato/黒田 隼人 > > Dear Wang, > > Followings are comments for your patchset. Thanks for the comments. > > 0001 > > > 01. launcher.c - logicalrep_worker_stop_internal() > > ``` > + > + Assert(LWLockHeldByMe(LogicalRepWorkerLock)); > +

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

2022-09-27 Thread houzj.f...@fujitsu.com
On Monday, September 26, 2022 6:58 PM Amit Kapila wrote: > > On Mon, Sep 26, 2022 at 8:41 AM wangw.f...@fujitsu.com > wrote: > > > > On Thur, Sep 22, 2022 at 18:12 PM Amit Kapila > wrote: > > > > > 3. > > > ApplyWorkerMain() > > > { > > > ... > > > ... > > > + > > > + if (server_version >=

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

2022-09-27 Thread Bharath Rupireddy
On Tue, Sep 27, 2022 at 3:01 PM Thomas Munro wrote: > > Something like the attached. Isn't it also better to add notes in win32pread.c and win32pwrite.c about the callers doing lseek(SEEK_SET) if they wish to and Windows implementations changing file position? We can also add a TODO item about

Re: has_privs_of_role vs. is_member_of_role, redux

2022-09-27 Thread Robert Haas
On Tue, Sep 27, 2022 at 2:05 AM Wolfgang Walther wrote: > I'm just saying WITH SET FALSE should take away more of the things you > can do (all the ownership things) to a point where it's safe to GRANT .. > WITH INHERIT TRUE, SET FALSE and still be useful for pre-defined or > privilege-container

Re: Add common function ReplicationOriginName.

2022-09-27 Thread Aleksander Alekseev
Hi Peter, > PSA patch v3 to combine the different replication origin name > formatting in a single function ReplicationOriginNameForLogicalRep as > suggested. LGTM except for minor issues with the formatting; fixed. > I expect you can find more like just this if you look harder than I did.

Re: Add more docs for pg_surgery?

2022-09-27 Thread Zhang Mingli
Regards, Zhang Mingli On Sep 27, 2022, 00:47 +0800, Ashutosh Sharma , wrote: > > And further it looks like you are doing an > experiment on undamaged relation which is not recommended as > documented. Yeah. >  If the relation would have been damaged, you probably may > not be able to access it. >

Re: kerberos/001_auth test fails on arm CPU darwin

2022-09-27 Thread Peter Eisentraut
On 27.09.22 03:37, Andres Freund wrote: Maybe we should rely on PATH, rather than hardcoding OS dependent locations? Or at least fall back to seach binaries in PATH? Seems pretty odd to hardcode all these locations without a way to influence it from outside the test. Homebrew intentionally

Fix some newly modified tab-complete changes

2022-09-27 Thread shiy.f...@fujitsu.com
Hi hackers, I saw a problem when using tab-complete for "GRANT", "TABLES IN SCHEMA" should be "ALL TABLES IN SCHEMA" in the following case. postgres=# grant all on ALL FUNCTIONS IN SCHEMA DATABASE FUNCTION PARAMETER SCHEMA

Re: A doubt about a newly added errdetail

2022-09-27 Thread Alvaro Herrera
On 2022-Sep-27, houzj.f...@fujitsu.com wrote: > Thanks for reviewing! > > Just in case I misunderstand, it seems you mean the message style[1] is OK, > right ? > > [1] > errdetail("Column list cannot be specified for a partitioned table when %s is > false.", >

Extend win32 error codes to errno mapping in win32error.c

2022-09-27 Thread Bharath Rupireddy
Hi, I recently faced an issue on windows where one of the tests was failing with 'unrecognized win32 error code: 123', see [1]. I figured out that it was due to a wrong file name being sent to open() system call (this error is of my own making and I fixed it for that thread). The failure occurs

Re: Refactor backup related code (was: Is it correct to say, "invalid data in file \"%s\"", BACKUP_LABEL_FILE in do_pg_backup_stop?)

2022-09-27 Thread Bharath Rupireddy
On Tue, Sep 27, 2022 at 2:20 PM Kyotaro Horiguchi wrote: > > > -1 from me. We have the function context and the structure name there > > to represent that the parameter name 'state' is actually 'backup > > state'. I don't think we gain anything here. Whenever the BackupState > > is used away from

Re: Insertion Sort Improvements

2022-09-27 Thread Benjamin Coutu
Getting back to improvements for small sort runs, it might make sense to consider using in-register based sorting via sorting networks for very small runs. This talk is specific to database sorting and illustrates how such an approach can be vectorized:

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

2022-09-27 Thread Thomas Munro
On Tue, Sep 27, 2022 at 6:43 PM Bharath Rupireddy wrote: > On Tue, Sep 27, 2022 at 3:08 AM Thomas Munro wrote: > > > > I don't think so, that's an extra kernel call. I think I'll just have > > to revert part of my recent change that removed the pg_ prefix from > > those function names in our

RE: A doubt about a newly added errdetail

2022-09-27 Thread houzj.f...@fujitsu.com
On Tuesday, September 27, 2022 3:21 PM Alvaro Herrera wrote: > > On 2022-Sep-27, Kyotaro Horiguchi wrote: > > > By the way, this is not an issue caused by the proposed patch, I see > > the following message in the patch. > > > > -errdetail("Column list

Re: Fast COPY FROM based on batch insert

2022-09-27 Thread Etsuro Fujita
On Wed, Aug 10, 2022 at 5:30 PM Etsuro Fujita wrote: > On Wed, Aug 10, 2022 at 1:06 AM Zhihong Yu wrote: > > + /* If any rows were inserted, run AFTER ROW INSERT triggers. */ > > ... > > + for (i = 0; i < inserted; i++) > > + { > > +

Re: [PATCH] Log details for client certificate failures

2022-09-27 Thread Masahiko Sawada
Hi, On Tue, Sep 13, 2022 at 11:11 PM Peter Eisentraut wrote: > > On 09.09.22 00:32, Jacob Champion wrote: > > On Thu, Jul 28, 2022 at 9:19 AM Jacob Champion > > wrote: > >> On Thu, Jul 21, 2022 at 4:29 PM Jacob Champion > >> wrote: > >>> v4 attempts to fix this by letting the check hooks

Re: Refactor backup related code (was: Is it correct to say, "invalid data in file \"%s\"", BACKUP_LABEL_FILE in do_pg_backup_stop?)

2022-09-27 Thread Kyotaro Horiguchi
At Tue, 27 Sep 2022 14:03:24 +0530, Bharath Rupireddy wrote in > On Tue, Sep 27, 2022 at 1:54 PM Kyotaro Horiguchi > wrote: > > What do you think about this? > > -1 from me. We have the function context and the structure name there > to represent that the parameter name 'state' is actually

Re: Differentiate MERGE queries with different structures

2022-09-27 Thread Alvaro Herrera
Pushed, thanks. -- Álvaro HerreraBreisgau, Deutschland — https://www.EnterpriseDB.com/ It does it in a really, really complicated way why does it need to be complicated? Because it's MakeMaker.

Re: Fast COPY FROM based on batch insert

2022-09-27 Thread Etsuro Fujita
On Tue, Aug 23, 2022 at 2:58 PM Andrey Lepikhov wrote: > On 22/8/2022 11:44, Etsuro Fujita wrote: > > I think the latter is more consistent with the existing error context > > information when in CopyMultiInsertBufferFlush(). Actually, I thought > > this too, and I think this would be useful

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

2022-09-27 Thread Peter Smith
Here are my review comments for the HEAD_v11-0001 patch: == 1. General - Another related bug? In [1] Hou-san wrote: For another case you mentioned (via_root used when publishing child) CREATE PUBLICATION pub1 for TABLE parent; CREATE PUBLICATION pub2 for TABLE child with

Re: Refactor backup related code (was: Is it correct to say, "invalid data in file \"%s\"", BACKUP_LABEL_FILE in do_pg_backup_stop?)

2022-09-27 Thread Bharath Rupireddy
On Tue, Sep 27, 2022 at 1:54 PM Kyotaro Horiguchi wrote: > > This commit introduced BackupState struct. The comment of > do_pg_backup_start says that: > > > * It fills in backup_state with the information required for the backup, > > And the parameters are: > > > do_pg_backup_start(const char

Re: Avoid memory leaks during base backups

2022-09-27 Thread Kyotaro Horiguchi
At Tue, 27 Sep 2022 11:33:56 +0530, Bharath Rupireddy wrote in > On Mon, Sep 26, 2022 at 7:34 PM Tom Lane wrote: > > This ... seems like inventing your own shape of wheel. The > > normal mechanism for preventing this type of leak is to put the > > allocations in a memory context that can be

Re: Refactor backup related code (was: Is it correct to say, "invalid data in file \"%s\"", BACKUP_LABEL_FILE in do_pg_backup_stop?)

2022-09-27 Thread Kyotaro Horiguchi
This commit introduced BackupState struct. The comment of do_pg_backup_start says that: > * It fills in backup_state with the information required for the backup, And the parameters are: > do_pg_backup_start(const char *backupidstr, bool fast, List **tablespaces, >

Re: Allow foreign keys to reference a superset of unique columns

2022-09-27 Thread David Rowley
On Tue, 27 Sept 2022 at 06:08, James Coleman wrote: > > On Mon, Sep 26, 2022 at 9:59 AM Wolfgang Walther > wrote: > > So no need for me to distract this thread from $subject anymore. I think > > the idea of allowing to create unique constraints on a superset of the > > columns of an already

Re: Add last_vacuum_index_scans in pg_stat_all_tables

2022-09-27 Thread Alvaro Herrera
On 2022-Sep-16, Fujii Masao wrote: > Could you tell me why the number of index scans should be tracked for > each table? Instead, isn't it enough to have one global counter, to > check whether the current setting of maintenance_work_mem is sufficient > or not? That is, I'm thinking to have

Re: [PATCH] Improve amcheck to also check UNIQUE constraint in btree index.

2022-09-27 Thread Maxim Orlov
On Thu, 22 Sept 2022 at 18:13, Andres Freund wrote: > Due to the merge of the meson based build this patch needs to be > adjusted: https://cirrus-ci.com/build/6350479973154816 > > Looks like you need to add amcheck--1.3--1.4.sql to the list of files to be > installed and

Re: pg_rewind WAL segments deletion pitfall

2022-09-27 Thread Kyotaro Horiguchi
At Thu, 1 Sep 2022 13:33:09 +0200, Polina Bungina wrote in > Here is the new version of the patch that includes the changes you > suggested. It is smaller now but I doubt if it is as easy to understand as > it used to be. pg_rewind works in two steps. First it constructs file map which decides

Re: [RFC] building postgres with meson - v13

2022-09-27 Thread John Naylor
On Tue, Sep 27, 2022 at 2:06 AM Andres Freund wrote: > > On 2022-09-26 15:18:29 +0700, John Naylor wrote: > > Either way it doesn't exist on this machine. I was able to get a working > > build with > > > > /usr/local/Homebrew/Library/Homebrew/os/mac/pkgconfig > > Hm - what did you need this path

Re: A doubt about a newly added errdetail

2022-09-27 Thread Alvaro Herrera
On 2022-Sep-27, Kyotaro Horiguchi wrote: > By the way, this is not an issue caused by the proposed patch, I see > the following message in the patch. > > - errdetail("Column list cannot be used > for a partitioned table when %s is false.", > +

Re: DROP OWNED BY is broken on master branch.

2022-09-27 Thread Rushabh Lathia
On Mon, Sep 26, 2022 at 11:46 PM Robert Haas wrote: > On Mon, Sep 26, 2022 at 3:44 AM Rushabh Lathia > wrote: > > Commit 6566133c5f52771198aca07ed18f84519fac1be7 ensure that > > pg_auth_members.grantor is always valid. This commit did changes > > into shdepDropOwned() function and combined the

Re: HOT chain validation in verify_heapam()

2022-09-27 Thread Himanshu Upadhyaya
On Tue, Sep 27, 2022 at 1:35 AM Robert Haas wrote: > On Sat, Sep 24, 2022 at 8:45 AM Himanshu Upadhyaya > wrote: > > Here our objective is to validate if both Predecessor's xmin and current > Tuple's xmin are same then cmin of predecessor must be less than current > Tuple's cmin. In case when

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

2022-09-27 Thread kuroda.hay...@fujitsu.com
Dear Wang, Followings are comments for your patchset. 0001 01. launcher.c - logicalrep_worker_stop_internal() ``` + + Assert(LWLockHeldByMe(LogicalRepWorkerLock)); + ``` I think it should be Assert(LWLockHeldByMeInMode(LogicalRepWorkerLock, LW_SHARED)) because the lock is

Re: add checkpoint stats of snapshot and mapping files of pg_logical dir

2022-09-27 Thread Bharath Rupireddy
On Mon, Sep 26, 2022 at 3:21 AM Justin Pryzby wrote: > > > This patch needs an update. It is failing on Windows for the test case > > "33/238 postgresql:pg_upgrade / pg_upgrade/002_pg_upgrade with an > > "exit status 2" error. > > > > The details are available on the cfbot.cputue.org. > >

  1   2   >