Re: ExecRTCheckPerms() and many prunable partitions (checkAsUser)

2023-02-19 Thread Amit Langote
On Fri, Feb 17, 2023 at 9:02 PM Alvaro Herrera wrote: > On 2022-Dec-11, Amit Langote wrote: > > While staring at the build_simple_rel() bit mentioned above, I > > realized that this code fails to set userid correctly in the > > inheritance parent rels that are child relations of subquery parent >

Re: improving user.c error messages

2023-02-19 Thread Peter Eisentraut
On 07.02.23 21:10, Nathan Bossart wrote: ERROR: permission denied to use replication slots DETAIL: You must have REPLICATION privilege. adding the operation to the end seems less awkward (i.e., "You must have REPLICATION privilege to use replication slots."). I don't think

Re: Missing free_var() at end of accum_sum_final()?

2023-02-19 Thread Michael Paquier
On Sun, Feb 19, 2023 at 11:55:38PM +0100, Joel Jacobson wrote: > To fix, a init_var() in alloc_var() is needed when we will use the > fixed_buf instead of allocating, > since alloc_var() could be called in a loop for existing values, > like in sqrt_var() for instance. > > Attached new version

Re: Add index scan progress to pg_stat_progress_vacuum

2023-02-19 Thread Masahiko Sawada
On Sat, Feb 18, 2023 at 11:46 AM Imseih (AWS), Sami wrote: > > Thanks for the review! > > >+ > >+ ParallelVacuumFinish > >+ Waiting for parallel vacuum workers to finish index > >vacuum. > >+ > > >This change is out-of-date. > > That was an oversight.

RE: Allow logical replication to copy tables in binary format

2023-02-19 Thread Hayato Kuroda (Fujitsu)
Dear Melih, Thank you for updating the patch. Before reviewing, I found that cfbot have not accepted v8 patch [1]. IIUC src/psql/describe.c has been modified in v8, but src/test/regress/expected/subscription.out has not been changed accordingly. [1]:

Re: pg_upgrade and logical replication

2023-02-19 Thread Julien Rouhaud
On Mon, Feb 20, 2023 at 11:07:42AM +0530, Amit Kapila wrote: > On Sun, Feb 19, 2023 at 5:31 AM Julien Rouhaud wrote: > > > > > > > > > > I might be missing something, but what could go wrong if pg_upgrade > > > > could emit > > > > a bunch of commands like: > > > > > > > > ALTER SUBSCRIPTION

Add support for unit "B" to pg_size_pretty()

2023-02-19 Thread Peter Eisentraut
This patch adds support for the unit "B" to pg_size_pretty(). This makes it consistent with the units support in GUC. (pg_size_pretty() only supports "bytes", but GUC only supports "B". -- I opted against adding support for "bytes" to GUC.)From 605ada8dc38b35aa70a281a7cacab65ce89f42be Mon Sep

RE: [Proposal] Add foreign-server health checks infrastructure

2023-02-19 Thread Hayato Kuroda (Fujitsu)
Dear Peter, Thank you for reviewing! Latest version can be seen in [1]. > 1. > PQcanConnCheck seemed like a strange API name. Maybe it can have the > same prefix as the other? > > e.g. > > - PQconnCheck() > - PGconnCheckSupported() > > or > > - PQconnCheck() > - PGconnCheckable() > I choose

RE: [Proposal] Add foreign-server health checks infrastructure

2023-02-19 Thread Hayato Kuroda (Fujitsu)
Dear Katsuragi-san, Thank you for reviewing! PSA new version. > 0001: > Extending pqSocketPoll seems to be a better way because we can > avoid having multiple similar functions. I also would like to hear > horiguchi-san's opinion whether this matches his expectation. > Improvements of

RE: Is psSocketPoll doing the right thing?

2023-02-19 Thread Hayato Kuroda (Fujitsu)
Dear Horiguchi-san, My comment may be no longer needed, but I can +1 to your opinion. > > On 2023/02/09 11:50, Kyotaro Horiguchi wrote: > > > Hello. > > > While looking a patch, I found that pqSocketPoll passes through the > > > result from poll(2) to the caller and throws away revents. If I > >

Re: pg_upgrade and logical replication

2023-02-19 Thread Amit Kapila
On Sun, Feb 19, 2023 at 5:31 AM Julien Rouhaud wrote: > > On Sat, Feb 18, 2023 at 04:12:52PM +0530, Amit Kapila wrote: > > > > > > > > > > > > > I also don't know if there is any other safe way for newly added > > > > tables apart from the above suggestion to create separate publications > > > >

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

2023-02-19 Thread Michael Paquier
On Fri, Feb 17, 2023 at 09:31:14AM -0800, Andres Freund wrote: > On 2023-02-17 16:19:46 +0900, Michael Paquier wrote: >> But it looks like I misunderstood what this quote meant compared to >> what v3 does. It is true that v3 sets iov_len and iov_base more than >> needed when writing sizes larger

Re: Add WAL read stats to pg_stat_wal

2023-02-19 Thread Kyotaro Horiguchi
At Thu, 16 Feb 2023 11:11:38 -0800, Andres Freund wrote in > I wonder if we should keep the checkpointer around for longer. If we have > checkpointer signal postmaster after it wrote the shutdown checkpoint, > postmaster could signal walsenders to shut down, and checkpointer could do > some

Re: DSA failed to allocate memory

2023-02-19 Thread Thomas Munro
On Fri, Jan 20, 2023 at 11:02 PM Thomas Munro wrote: > Yeah. I think the analysis looks good, but I'll do some testing next > week with the aim of getting it committed. Looks like it now needs > Meson changes, but I'll look after that as my penance. Here's an updated version that I'm

Re: Support logical replication of DDLs

2023-02-19 Thread Amit Kapila
On Sun, Feb 19, 2023 at 7:50 AM Jonathan S. Katz wrote: > > On 2/17/23 4:15 AM, Amit Kapila wrote: > > > This happens because of the function used in the index expression. > > Now, this is not the only thing, the replication can even fail during > > DDL replication when the function like above is

Re: Make set_ps_display faster and easier to use

2023-02-19 Thread David Rowley
On Fri, 17 Feb 2023 at 21:44, David Rowley wrote: > Updated patch attached. After making another couple of small adjustments, I've pushed this. Thanks for the review. David

Re: Refactor calculations to use instr_time

2023-02-19 Thread Kyotaro Horiguchi
At Fri, 17 Feb 2023 13:53:36 +0300, Nazir Bilal Yavuz wrote in > Thanks for the review. I updated the patch. WalUsageAccumDiff(, , ); - PendingWalStats.wal_records = diff.wal_records; - PendingWalStats.wal_fpi = diff.wal_fpi; - PendingWalStats.wal_bytes =

Re: Normalization of utility queries in pg_stat_statements

2023-02-19 Thread Michael Paquier
On Mon, Feb 20, 2023 at 11:32:23AM +0900, Michael Paquier wrote: > These last ones are staying around for a few more weeks, until the > middle of the next CF, I guess. After all this is done, the final > changes are very short, showing the effects of the normalization, as > of: > 6 files

Re: Normalization of utility queries in pg_stat_statements

2023-02-19 Thread Michael Paquier
On Fri, Feb 17, 2023 at 09:36:27AM +0100, Drouvot, Bertrand wrote: > On 2/17/23 3:35 AM, Michael Paquier wrote: >> 0001 looks quite committable at this stage, and that's independent on >> the rest. At the end this patch creates four new test files that are >> extended in the next patches:

RE: Time delayed LR (WAS Re: logical replication restrictions)

2023-02-19 Thread Hayato Kuroda (Fujitsu)
Dear Amit, Thank you for reviewing! PSA new version. > 1. > + > + The minimum delay for publisher sends data, in milliseconds > + > + > > It would probably be better to write it as "The minimum delay, in > milliseconds, by the publisher to send changes" Fixed. > 2. The

Re: [Proposal] Add foreign-server health checks infrastructure

2023-02-19 Thread Peter Smith
Here is a code review only for patch v31-0001. == General Comment 1. PQcanConnCheck seemed like a strange API name. Maybe it can have the same prefix as the other? e.g. - PQconnCheck() - PGconnCheckSupported() or - PQconnCheck() - PGconnCheckable() == Commit Message 2.

Re: cfbot failures

2023-02-19 Thread Andres Freund
Hi, On 2023-02-19 19:08:41 -0600, Justin Pryzby wrote: > On 2023-02-11, Andres Freund wrote > 20230212004254.3lp22a7bpkcjo...@awork3.anarazel.de: > > The windows test failure is a transient issue independent of the patch > > (something went wrong with image permissions). > > That's happening

cfbot failures

2023-02-19 Thread Justin Pryzby
On 2023-02-11, Andres Freund wrote 20230212004254.3lp22a7bpkcjo...@awork3.anarazel.de: > The windows test failure is a transient issue independent of the patch > (something went wrong with image permissions). That's happening again since 3h ago.

Re: windows CI failing PMSignalState->PMChildFlags[slot] == PM_CHILD_ASSIGNED

2023-02-19 Thread Thomas Munro
On Mon, Feb 20, 2023 at 2:46 AM Andrew Dunstan wrote: > On 2023-02-17 Fr 19:27, Thomas Munro wrote: >> FWIW I have a thing I call bfbot for slurping up similar >> data from the build farm. It's not pretty enough for public >> consumption, but I do know that this assertion hasn't failed there, >>

Re: Missing free_var() at end of accum_sum_final()?

2023-02-19 Thread Joel Jacobson
On Sun, Feb 19, 2023, at 23:16, Joel Jacobson wrote: > Hi again, > > Ignore previous patch, new correct version attached, that also keeps > track of if the buf-field is in use or not. Ops! One more thinko, detected when trying fixed_buf[8], which caused a seg fault. To fix, a init_var() in

Re: Missing free_var() at end of accum_sum_final()?

2023-02-19 Thread Joel Jacobson
Hi again, Ignore previous patch, new correct version attached, that also keeps track of if the buf-field is in use or not. Seems like your idea gives a signifiant speed-up! On my machine, numeric_in() is 22% faster! I ran a benchmark with 100 tests measuring execution-time of numeric_in() with

Re: Wrong query results caused by loss of join quals

2023-02-19 Thread Tom Lane
Richard Guo writes: > ... As we can see, the join qual 't2.a = t2.b' disappears in the plan, and > that results in the wrong query results. Ugh. > I did some dig and here is what happened. Firstly both sides of qual > 't2.a = t2.b' could be nulled by the OJ t1/t2 and they are marked so in >

Re: Handle TEMP_CONFIG for pg_regress style tests in pg_regress.c

2023-02-19 Thread Andres Freund
Hi, On 2023-02-19 11:13:38 -0500, Tom Lane wrote: > Andrew Dunstan writes: > > On 2023-02-19 Su 02:25, Peter Eisentraut wrote: > >> On 18.02.23 21:26, Andres Freund wrote: > >>> My inclination is to move TEMP_CONFIG support from the Makefile to > >>> pg_regress.c. That way it's consistent across

Re: Missing free_var() at end of accum_sum_final()?

2023-02-19 Thread Joel Jacobson
On Fri, Feb 17, 2023, at 21:26, Andres Freund wrote: > Removing the free_var()s from numeric_add_opt_error() speeds it up from ~361ms > to ~338ms. I notice numeric_add_opt_error() is extern and declared in numeric.h, called from e.g. timestamp.c and jsonpath_exec.c. Is that a problem, i.e. is

Re: Add LZ4 compression in pg_dump

2023-02-19 Thread Justin Pryzby
Some little updates since I last checked: + * This file also includes the implementation when compression is none for + * both API's. => this comment is obsolete. s/deffer/infer/ ? or determine ? This typo occurs multiple times. currently this includes only ".gz" => remove this phase from the

Re: wrong query result due to wang plan

2023-02-19 Thread Tom Lane
I wrote: > So that leads to a conclusion that we could just forget the whole > thing and always use the syntactic qualscope here. I tried that > and it doesn't break any existing regression tests, which admittedly > doesn't prove a lot in this area :-(. Hmm, I thought it worked, but when I tried

Re: Handle TEMP_CONFIG for pg_regress style tests in pg_regress.c

2023-02-19 Thread Tom Lane
Andrew Dunstan writes: > On 2023-02-19 Su 02:25, Peter Eisentraut wrote: >> On 18.02.23 21:26, Andres Freund wrote: >>> My inclination is to move TEMP_CONFIG support from the Makefile to >>> pg_regress.c. That way it's consistent across the build tools and isn't >>> duplicated. >> I'm having a

Re: Ignoring BRIN for HOT updates (was: -udpates seems broken)

2023-02-19 Thread Tomas Vondra
Hi, On 2/19/23 02:03, Matthias van de Meent wrote: > On Thu, 16 Jun 2022 at 15:05, Tomas Vondra > wrote: >> >> I've pushed the revert. Let's try again for PG16. > > As we discussed in person at the developer meeting, here's a patch to > try again for PG16. > > It combines the committed patches

RE: Support logical replication of DDLs

2023-02-19 Thread houzj.f...@fujitsu.com
On Wed, Feb 15, 2023 at 13:57 PM Amit Kapila wrote: > On Fri, Feb 10, 2023 at 8:23 PM Masahiko Sawada > > wrote: > > > > On Thu, Feb 9, 2023 at 6:55 PM Ajin Cherian wrote: > > > > > (v67) > > > > I have some questions about adding the infrastructure for DDL deparsing. > > > > Apart from the

Re: Weird failure with latches in curculio on v15

2023-02-19 Thread Robert Haas
On Sun, Feb 19, 2023 at 2:45 AM Andres Freund wrote: > To me that seems even simpler? Nothing but the archiver is supposed to create > .done files and nothing is supposed to remove .ready files without archiver > having created the .done files. So the archiver process can scan > archive_status

Re: Handle TEMP_CONFIG for pg_regress style tests in pg_regress.c

2023-02-19 Thread Andrew Dunstan
On 2023-02-19 Su 02:25, Peter Eisentraut wrote: On 18.02.23 21:26, Andres Freund wrote: When building with meson, TEMP_CONFIG is supported for TAP tests, but doesn't do anything for regress/isolation. The reason for that is that meson's (and ninja's) architecture is to separate "build

Re: windows CI failing PMSignalState->PMChildFlags[slot] == PM_CHILD_ASSIGNED

2023-02-19 Thread Andrew Dunstan
On 2023-02-17 Fr 19:27, Thomas Munro wrote: FWIW I have a thing I call bfbot for slurping up similar data from the build farm. It's not pretty enough for public consumption, but I do know that this assertion hasn't failed there, except the cases I mentioned earlier, and a load of failures on

RE: Rework LogicalOutputPluginWriterUpdateProgress

2023-02-19 Thread wangw.f...@fujitsu.com
On Thur, Feb 14, 2023 at 2:03 AM Andres Freund wrote: > On 2023-02-13 14:06:57 +0530, Amit Kapila wrote: > > > > The patch calls update_progress in change_cb_wrapper and other > > > > wrappers which will miss the case of DDLs that generates a lot of data > > > > that is not processed by the

Re: Inconsistency in reporting checkpointer stats

2023-02-19 Thread Nitin Jadhav
> This doesn't pass the tests, because the regression tests weren't adjusted: > https://api.cirrus-ci.com/v1/artifact/task/5937624817336320/testrun/build/testrun/regress/regress/regression.diffs Thanks for sharing this. I have fixed this in the patch attached. >> IMO, there's no need for 2