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

2023-02-13 Thread Amit Langote
On Mon, Feb 13, 2023 at 5:07 Justin Pryzby wrote: > On Tue, Nov 29, 2022 at 10:37:56PM +0900, Amit Langote wrote: > > 0002 contains changes that has to do with changing how we access > > checkAsUser in some foreign table planning/execution code sites. > > Thought it might be better to describe it

Re: Rework LogicalOutputPluginWriterUpdateProgress

2023-02-13 Thread Amit Kapila
On Sat, Feb 11, 2023 at 2:34 AM Andres Freund wrote: > > On 2023-02-09 11:21:41 +0530, Amit Kapila wrote: > > On Thu, Feb 9, 2023 at 1:33 AM Andres Freund wrote: > > > > > > Hacking on a rough prototype how I think this should rather look, I had a > > > few > > > questions / remarks: > > > > > >

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

2023-02-13 Thread Peter Smith
Here are my review comments for the v34 patch. == src/backend/replication/logical/worker.c +/* The last time we send a feedback message */ +static TimestampTz send_time = 0; + IMO this is a bad variable name. When this variable was changed to be global it ought to have been renamed. The nam

Re: Reconcile stats in find_tabstat_entry() and get rid of PgStat_BackendFunctionEntry

2023-02-13 Thread Drouvot, Bertrand
Hi, On 2/13/23 8:40 AM, Kyotaro Horiguchi wrote: At Mon, 13 Feb 2023 08:09:50 +0100, "Drouvot, Bertrand" wrote in I think this is useful beyond being able to generate those functions with macros. The fact that we had to deal with transactional code in pgstatfuncs.c meant that a lot of the rel

Get rid of PgStat_BackendFunctionEntry

2023-02-13 Thread Drouvot, Bertrand
Hi hackers, Please find attached a patch proposal to $SUBJECT: it would allow to simplify even more the work done to generate pg_stat_get_xact*() functions with Macros. This is a follow up for [1] where it has been suggested to get rid of PgStat_BackendFunctionEntry in a separate patch. Looking

Re: Support logical replication of DDLs

2023-02-13 Thread Alvaro Herrera
On 2023-Feb-06, Peter Smith wrote: > I thought this comment was analogous to another one from this same > patch 0001 (see seclabel.c), so the suggested change above was simply > to make the wording consistent. > > @@ -134,6 +134,9 @@ ExecSecLabelStmt(SecLabelStmt *stmt) > (errcode(ERRCODE_INVAL

Re: Consolidate ItemPointer to Datum conversion functions

2023-02-13 Thread Peter Eisentraut
On 09.02.23 09:33, Peter Eisentraut wrote: On 06.02.23 11:11, Heikki Linnakangas wrote: On 06/02/2023 11:54, Peter Eisentraut wrote: Instead of defining the same set of macros several times, define it once in an appropriate header file.  In passing, convert to inline functions. Looks good to

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

2023-02-13 Thread Kyotaro Horiguchi
At Mon, 13 Feb 2023 10:15:03 +0530, Bharath Rupireddy wrote in > On Sun, Feb 12, 2023 at 11:01 PM Andres Freund wrote: > > > > On 2023-02-12 19:59:00 +0530, Bharath Rupireddy wrote: > > > /* Prepare to write out a lot of copies of our zero buffer at once. > > > */ > > > for (i = 0;

Re: Killing off removed rels properly

2023-02-13 Thread Richard Guo
On Sat, Feb 11, 2023 at 4:50 AM Tom Lane wrote: > I think it's time to clean this up by removing the rel from the > planner data structures altogether. The attached passes check-world, > and if it does trigger any problems I would say that's a clear > sign of bugs elsewhere. +1. The patch look

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

2023-02-13 Thread Amit Kapila
On Fri, Feb 10, 2023 at 4:56 PM Takamichi Osumi (Fujitsu) wrote: > > On Friday, February 10, 2023 2:05 PM Friday, February 10, 2023 2:05 PM wrote: > > On Fri, Feb 10, 2023 at 10:11 AM Amit Kapila > > wrote: > > In the previous patch, we couldn't solve the > timeout of the publisher, when we condu

Re: Rework of collation code, extensibility

2023-02-13 Thread Peter Eisentraut
On 01.02.23 00:33, Jeff Davis wrote: On Tue, 2023-01-31 at 11:40 +0100, Peter Eisentraut wrote: I don't know to what extent this depends on the abbreviated key GUC discussion.  Does the rest of this patch set depend on this? The overall refactoring is not dependent logically on the GUC patch.

Re: psql - factor out echo code

2023-02-13 Thread Peter Eisentraut
On 01.12.22 08:27, Pavel Stehule wrote: st 30. 11. 2022 v 10:43 odesílatel Fabien COELHO > napsal: >> Now some of the output generated by test_extdepend gets a bit >> confusing: >> +-- QUERY: >> + >> + >> +-- QUERY: >> >> That'

Re: Helper functions for wait_for_catchup() in Cluster.pm

2023-02-13 Thread Alvaro Herrera
On 2023-Jan-26, Drouvot, Bertrand wrote: > Hi, > > On 1/26/23 10:42 AM, Alvaro Herrera wrote: > > On 2023-Jan-26, Drouvot, Bertrand wrote: > > > > > On 1/24/23 7:27 PM, Alvaro Herrera wrote: > > > > > > 1. I don't think wait_for_write_catchup is necessary, because > > > > calling wait_for_catch

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

2023-02-13 Thread shiy.f...@fujitsu.com
On Thu, Feb 2, 2023 4:34 PM Önder Kalacı wrote: > >> >> and if there's more >> than one candidate index pick any one. Additionally, we can allow >> disabling the use of an index scan for this particular case. If we are >> too worried about API change for allowing users to specify the index >> then

Re: Can we do something to help stop users mistakenly using force_parallel_mode?

2023-02-13 Thread David Rowley
On Sat, 11 Feb 2023 at 04:34, Andrew Dunstan wrote: > > On 2023-02-09 Th 15:25, David Rowley wrote: > Likely the hardest part to get right here is the new name. Can anyone > think of anything better than debug_parallel_query? > > > WFM Thanks for chipping in. I've attached a patch which fixes t

Making CopyFromStateData not internal anymore

2023-02-13 Thread Jelte Fennema
For Citus we'd like to hook into the way that GENERATED AS IDENTITY generates the next value. The way we had in mind was to replace T_NextValueExpr with a function call node. But doing that same for COPY seems only possible by manually changing the defexprs array of the CopyFromState. Sadly CopyFro

Re: [PATCH] Add pretty-printed XML output option

2023-02-13 Thread Jim Jones
On 13.02.23 02:15, Peter Smith wrote: Something is misbehaving. Using the latest HEAD, and before applying the v6 patch, 'make check' is working OK. But after applying the v6 patch, some XML regression tests are failing because the DETAIL part of the message indicating the wrong syntax position

Re: run pgindent on a regular basis / scripted manner

2023-02-13 Thread Andrew Dunstan
On 2023-02-12 Su 15:59, Justin Pryzby wrote: It seems like if pgindent knows about git, it ought to process only tracked files. Then, it wouldn't need to manually exclude generated files, and it wouldn't process vpath builds and who-knows-what else it finds in CWD. for vpath builds use an ex

Re: run pgindent on a regular basis / scripted manner

2023-02-13 Thread Andrew Dunstan
On 2023-02-12 Su 16:13, Tom Lane wrote: Andrew Dunstan writes: On 2023-02-12 Su 11:24, Tom Lane wrote: It seems like "indent the whole tree" is about to become a minority use-case. Maybe instead of continuing to privilege that case, we should say that it's invoked by some new switch like --a

Re: amvalidate(): cache lookup failed for operator class 123

2023-02-13 Thread Justin Pryzby
On Thu, May 13, 2021 at 12:01:22PM -0500, Justin Pryzby wrote: > postgres=# select amvalidate(123); > ERROR: cache lookup failed for operator class 123 > The usual expectation is that sql callable functions should return null rather > than hitting elog(). On Thu, May 13, 2021 at 2:22 PM Tom Lane

Re: run pgindent on a regular basis / scripted manner

2023-02-13 Thread Jelte Fennema
On Sun, 12 Feb 2023 at 15:16, Andrew Dunstan wrote: > I'm not sure how much more I really want to do here. Given the way pgindent > now processes command line arguments, maybe the best thing is for people to > use that. Use of git aliases can help. Something like these for example > > > [alias]

Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf

2023-02-13 Thread Pavel Luzanov
Hello, Playing with this patch, I did not see descriptive comments in pg_ident.conf. Does it make sense to reflect changes to the PG-USERNAME field in the pg_ident.conf.sample file? The same relates to the regexp supportin the DATABASE and USER fieldsof the pg_hba.conf.sample file(8fea8683

Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf

2023-02-13 Thread Jelte Fennema
On Mon, 13 Feb 2023 at 15:06, Pavel Luzanov wrote: > Does it make sense to reflect changes to the PG-USERNAME field in the > pg_ident.conf.sample file? > > The same relates to the regexp supportin the DATABASE and USER fieldsof > the pg_hba.conf.sample file(8fea8683). That definitely makes sense

[PATCH] Support SK_SEARCHNULL / SK_SEARCHNOTNULL for heap-only scans

2023-02-13 Thread Aleksander Alekseev
Hi hackers, A colleague of mine wanted to use a ScanKey with SK_SEARCHNULL flag for a heap-only scan (besides other ScanKeys) and discovered that the result differs from what he would expect. Turned out that this is currently not supported as it is explicitly stated in skey.h. Although several wo

Re: Helper functions for wait_for_catchup() in Cluster.pm

2023-02-13 Thread Drouvot, Bertrand
Hi, On 2/13/23 11:58 AM, Alvaro Herrera wrote: On 2023-Jan-26, Drouvot, Bertrand wrote: Hi, On 1/26/23 10:42 AM, Alvaro Herrera wrote: On 2023-Jan-26, Drouvot, Bertrand wrote: On 1/24/23 7:27 PM, Alvaro Herrera wrote: 1. I don't think wait_for_write_catchup is necessary, because calling

ERROR: permission info at index 1 ....

2023-02-13 Thread tender wang
Hi hackers, After a61b1f74823c commit, below query reports error: create table perm_test1(a int); create table perm_test2(b int); select subq.c0 from (select (select a from perm_test1 order by a limit 1) as c0, b as c1 from perm_test2 where false order by c0, c1) as subq where false; ERROR:

Re: [PATCH] Support SK_SEARCHNULL / SK_SEARCHNOTNULL for heap-only scans

2023-02-13 Thread Andres Freund
Hi, On 2023-02-13 17:59:13 +0300, Aleksander Alekseev wrote: > @@ -36,20 +36,36 @@ HeapKeyTest(HeapTuple tuple, TupleDesc tupdesc, int > nkeys, ScanKey keys) > boolisnull; > Datum test; > > - if (cur_key->sk_flags & SK_ISNULL) > -

Re: run pgindent on a regular basis / scripted manner

2023-02-13 Thread Andrew Dunstan
On 2023-02-13 Mo 09:02, Jelte Fennema wrote: On Sun, 12 Feb 2023 at 15:16, Andrew Dunstan wrote: I'm not sure how much more I really want to do here. Given the way pgindent now processes command line arguments, maybe the best thing is for people to use that. Use of git aliases can help. Some

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

2023-02-13 Thread Andres Freund
Hi, On 2023-02-11 05:44:47 +, Takamichi Osumi (Fujitsu) wrote: > On Saturday, February 11, 2023 11:10 AM Andres Freund > wrote: > > Has there been any discussion about whether this is actually best > > implemented on the client side? You could alternatively implement it on the > > sender. >

Re: Making Vars outer-join aware

2023-02-13 Thread Tom Lane
Richard Guo writes: > Thanks for the report! I've looked at it a little bit and traced down > to function have_unsafe_outer_join_ref(). The comment there says > * In practice, this test never finds a problem ... > This seems not correct as showed by the counterexample. Right. I'd noticed that

BRIN indexes vs. SK_SEARCHARRAY (and preprocessing scan keys)

2023-02-13 Thread Tomas Vondra
Hi, while experimenting with BRIN indexes after a couple FOSDEM discussions, I ran into the existing limitation that BRIN indexes don't handle array scan keys. So BRIN indexes can be used for conditions like WHERE a IN (1,2,3,4,5) but we essentially treat the values as individual scan keys,

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

2023-02-13 Thread Tom Lane
Amit Langote writes: > On Mon, Feb 13, 2023 at 5:07 Justin Pryzby wrote: >> That seems to add various elog()s which are hit frequently by sqlsmith: > Thanks for the report. I’ll take a look once I’m back at a computer in a > few days. Looks like we already have a diagnosis and fix [1]. I'll g

Re: proposal: psql: psql variable BACKEND_PID

2023-02-13 Thread Peter Eisentraut
On 09.02.23 10:11, Pavel Stehule wrote: first and main (for me) - I can use psql variables tab complete - just :B - it is significantly faster second - I can see all connection related information by \set third - there is not hook on reconnect in psql - so if you implement BACKEND_PID by self,

Re: ERROR: permission info at index 1 ....

2023-02-13 Thread Tom Lane
tender wang writes: >After a61b1f74823c commit, below query reports error: > create table perm_test1(a int); > create table perm_test2(b int); > select subq.c0 > from (select (select a from perm_test1 order by a limit 1) as c0, b as c1 > from perm_test2 where false order by c0, c1) as subq

Re: proposal: psql: psql variable BACKEND_PID

2023-02-13 Thread Pavel Stehule
po 13. 2. 2023 v 18:06 odesílatel Peter Eisentraut < peter.eisentr...@enterprisedb.com> napsal: > On 09.02.23 10:11, Pavel Stehule wrote: > > first and main (for me) - I can use psql variables tab complete - just > > :B - it is significantly faster > > second - I can see all connection related inf

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

2023-02-13 Thread Andres Freund
On 2023-02-13 18:33:34 +0900, Kyotaro Horiguchi wrote: > At Mon, 13 Feb 2023 10:15:03 +0530, Bharath Rupireddy > wrote in > > On Sun, Feb 12, 2023 at 11:01 PM Andres Freund wrote: > > > > > > On 2023-02-12 19:59:00 +0530, Bharath Rupireddy wrote: > > > > /* Prepare to write out a lot of c

Re: proposal: psql: psql variable BACKEND_PID

2023-02-13 Thread Andres Freund
Hi, On 2023-02-13 18:06:23 +0100, Peter Eisentraut wrote: > On 09.02.23 10:11, Pavel Stehule wrote: > > first and main (for me) - I can use psql variables tab complete - just > > :B - it is significantly faster > > second - I can see all connection related information by \set > > third - there is

Re: Force testing of query jumbling code in TAP tests

2023-02-13 Thread Andres Freund
Hi, On 2023-02-13 14:00:36 +0900, Michael Paquier wrote: > With this addition, the query jumbling gets covered at 95%~, while > https://coverage.postgresql.org/src/backend/nodes/index.html reports > currently 35%. > > Thoughts or comments? Shouldn't there at least be some basic verification of p

Re: proposal: psql: psql variable BACKEND_PID

2023-02-13 Thread Tom Lane
Andres Freund writes: > On 2023-02-13 18:06:23 +0100, Peter Eisentraut wrote: >> But what do you need the backend PID for in the first place? > For me it's using gdb, pidstat, strace, perf, ... > But for those %p in the PROMPTs is more useful. Indeed, because ... > E.g. I fire of a query, it's

Re: proposal: psql: psql variable BACKEND_PID

2023-02-13 Thread Andres Freund
Hi, On 2023-02-13 12:52:23 -0500, Tom Lane wrote: > Andres Freund writes: > > E.g. I fire of a query, it's slower than I'd like, I want to attach perf. Of > > course I can establish a separate connection, query pg_stat_activity there, > > and then perf. But that requires manually filtering pg_sta

Re: Rework LogicalOutputPluginWriterUpdateProgress

2023-02-13 Thread Andres Freund
Hi, On 2023-02-13 08:22:34 +0530, Amit Kapila wrote: > On Sat, Feb 11, 2023 at 3:04 AM Andres Freund wrote: > > > > > One difference I see with the patch is that I think we will end up > > > sending keepalive for empty prepared transactions even though we don't > > > skip sending begin/prepare me

Re: Rework LogicalOutputPluginWriterUpdateProgress

2023-02-13 Thread Andres Freund
Hi, 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 plugin. I think for that we either need > > > to call update_pro

Re: proposal: psql: psql variable BACKEND_PID

2023-02-13 Thread Pavel Stehule
po 13. 2. 2023 v 18:52 odesílatel Tom Lane napsal: > Andres Freund writes: > > On 2023-02-13 18:06:23 +0100, Peter Eisentraut wrote: > >> But what do you need the backend PID for in the first place? > > > For me it's using gdb, pidstat, strace, perf, ... > > But for those %p in the PROMPTs is mo

Re: run pgindent on a regular basis / scripted manner

2023-02-13 Thread Jelte Fennema
On Mon, 13 Feb 2023 at 17:47, Andrew Dunstan wrote: > OK, but I'd like to hear from more people about what they want. Experience > tells me that making assumptions about how people work is not a good idea. I > doubt anyone's work pattern is like mine. I don't want to implement an option > that

Re: ERROR: permission info at index 1 ....

2023-02-13 Thread Alvaro Herrera
On 2023-Feb-13, Tom Lane wrote: > tender wang writes: > >After a61b1f74823c commit, below query reports error: > > create table perm_test1(a int); > > create table perm_test2(b int); > > select subq.c0 > > from (select (select a from perm_test1 order by a limit 1) as c0, b as c1 > > from p

Re: Killing off removed rels properly

2023-02-13 Thread Tom Lane
Richard Guo writes: > On Sat, Feb 11, 2023 at 4:50 AM Tom Lane wrote: >> I think it's time to clean this up by removing the rel from the >> planner data structures altogether. The attached passes check-world, >> and if it does trigger any problems I would say that's a clear >> sign of bugs elsew

Adding "large" to PG_TEST_EXTRA

2023-02-13 Thread Andres Freund
Hi, I'm working on rebasing [1], my patch to make relation extension scale better. As part of that I'd like to add tests for relation extension. To be able to test the bulk write strategy path, we need to have a few backends concurrently load > 16MB files. It seems pretty clear that doing that o

Re: Adding "large" to PG_TEST_EXTRA

2023-02-13 Thread Stephen Frost
Greetings, * Andres Freund (and...@anarazel.de) wrote: > I'm working on rebasing [1], my patch to make relation extension scale > better. > > As part of that I'd like to add tests for relation extension. To be able to > test the bulk write strategy path, we need to have a few backends concurrentl

Re: Making Vars outer-join aware

2023-02-13 Thread Justin Pryzby
On Mon, Feb 13, 2023 at 03:33:15PM +0800, Richard Guo wrote: > On Mon, Feb 13, 2023 at 7:58 AM Justin Pryzby wrote: > > The patch broke this query: > > > > select from pg_inherits inner join information_schema.element_types > > right join (select from pg_constraint as sample_2) on true > > on fals

[PATCH] Align GSS and TLS error handling in PQconnectPoll()

2023-02-13 Thread Jacob Champion
Hi all, During the gssencmode CVE discussion, we noticed that PQconnectPoll() handles the error cases for TLS and GSS transport encryption slightly differently. After TLS fails, the connection handle is dead and future calls to PQconnectPoll() return immediately. But after GSS encryption fails, th

Re: ICU locale validation / canonicalization

2023-02-13 Thread Jeff Davis
On Fri, 2023-02-10 at 22:50 -0500, Robert Haas wrote: > The fact that you're figuring out how it all works from reading the > source code does not give me a warm feeling. Right. On the other hand, the behavior is quite well documented, it was just the keyword that was undocumented (or I didn't fin

Re: Adding "large" to PG_TEST_EXTRA

2023-02-13 Thread Tom Lane
Andres Freund writes: > As part of that I'd like to add tests for relation extension. To be able to > test the bulk write strategy path, we need to have a few backends concurrently > load > 16MB files. > It seems pretty clear that doing that on all buildfarm machines wouldn't be > nice / welcome.

Re: Adding "large" to PG_TEST_EXTRA

2023-02-13 Thread Andres Freund
Hi, On 2023-02-13 13:45:41 -0500, Stephen Frost wrote: > Are there existing tests that we should add into that set that you're > thinking of..? I've been working with the Kerberos tests and that's > definitely one that seems to fit this description... I think the kerberos tests are already opt-i

Re: Adding "large" to PG_TEST_EXTRA

2023-02-13 Thread Andres Freund
Hi, On 2023-02-13 13:54:59 -0500, Tom Lane wrote: > Bikeshedding a bit ... is "large" the right name? It's not awful but > I wonder if there is a better one I did wonder about that too. But didn't come up with something more poignant. > it seems like this class could eventually include tests t

Re: Adding "large" to PG_TEST_EXTRA

2023-02-13 Thread Stephen Frost
Greetings, * Andres Freund (and...@anarazel.de) wrote: > On 2023-02-13 13:45:41 -0500, Stephen Frost wrote: > > Are there existing tests that we should add into that set that you're > > thinking of..? I've been working with the Kerberos tests and that's > > definitely one that seems to fit this d

Re: Adding "large" to PG_TEST_EXTRA

2023-02-13 Thread Andres Freund
Hi, On 2023-02-13 14:15:24 -0500, Stephen Frost wrote: > * Andres Freund (and...@anarazel.de) wrote: > > On 2023-02-13 13:45:41 -0500, Stephen Frost wrote: > > > Are there existing tests that we should add into that set that you're > > > thinking of..? I've been working with the Kerberos tests an

Re: Adding "large" to PG_TEST_EXTRA

2023-02-13 Thread Tom Lane
Andres Freund writes: > On 2023-02-13 13:54:59 -0500, Tom Lane wrote: >> it seems like this class could eventually include tests that run a long time >> but don't necessarily eat disk space. "resource-intensive" is too long. > I'm not sure we'd want to combine time-intensive and disk-space-inten

Re: Adding "large" to PG_TEST_EXTRA

2023-02-13 Thread Andres Freund
On 2023-02-13 14:55:45 -0500, Tom Lane wrote: > bigdisk, bigcpu? Works for me. I'll probably just add bigdisk as part of adding a test for bulk relation extensions, mentioning in a comment that we might want bigcpu if we have a test for it?

Re: old_snapshot_threshold bottleneck on replica

2023-02-13 Thread Andres Freund
Hi, On 2023-01-24 10:46:28 -0500, Robert Haas wrote: > On Mon, Jan 23, 2023 at 9:40 AM Maxim Orlov wrote: > > One of our customers stumble onto a significant performance degradation > > while running multiple OLAP-like queries on a replica. > > After some investigation, it became clear that the

Re: proposal: psql: psql variable BACKEND_PID

2023-02-13 Thread Peter Eisentraut
On 13.02.23 18:33, Pavel Stehule wrote: In every real use case you can use pg_backend_pid(), but you need to write a complete name without tab complete, and you need to know so this function is available. BACKEND_PID is supported by  tab complete, and it is displayed in \set list and \? varia

Re: Adding "large" to PG_TEST_EXTRA

2023-02-13 Thread Tom Lane
Andres Freund writes: > On 2023-02-13 14:55:45 -0500, Tom Lane wrote: >> bigdisk, bigcpu? > Works for me. > I'll probably just add bigdisk as part of adding a test for bulk relation > extensions, mentioning in a comment that we might want bigcpu if we have a > test for it? No objection here.

Re: REASSIGN OWNED vs ALTER TABLE OWNER TO permission inconsistencies

2023-02-13 Thread Robert Haas
On Wed, Feb 8, 2023 at 5:49 AM Nazir Bilal Yavuz wrote: > My colleague Adam realized that when transferring ownership, 'REASSIGN > OWNED' command doesn't check 'CREATE privilege on the table's schema' on > new owner but 'ALTER TABLE OWNER TO' docs state that: Well, that sucks. > As you can see,

OID ordering dependency in pg_dump

2023-02-13 Thread Tom Lane
While starting to poke at the hashed-enum-partition-key problem recently discussed [1], I realized that pg_dump's flagInhAttrs() function has a logic issue: its loop changes state that will be inspected in other iterations of the loop, and there's no guarantee about the order in which related table

Re: Adding "large" to PG_TEST_EXTRA

2023-02-13 Thread Andrew Dunstan
On 2023-02-13 Mo 14:34, Andres Freund wrote: Hi, On 2023-02-13 14:15:24 -0500, Stephen Frost wrote: * Andres Freund (and...@anarazel.de) wrote: On 2023-02-13 13:45:41 -0500, Stephen Frost wrote: Are there existing tests that we should add into that set that you're thinking of..? I've been w

Re: Adding "large" to PG_TEST_EXTRA

2023-02-13 Thread Peter Smith
On Tue, Feb 14, 2023 at 5:42 AM Andres Freund wrote: > > Hi, > > I'm working on rebasing [1], my patch to make relation extension scale > better. > > As part of that I'd like to add tests for relation extension. To be able to > test the bulk write strategy path, we need to have a few backends conc

Re: recovery modules

2023-02-13 Thread Nathan Bossart
On Mon, Feb 13, 2023 at 04:37:10PM +0900, Michael Paquier wrote: > + basic_archive_context = data->context; > + if (CurrentMemoryContext == basic_archive_context) > + MemoryContextSwitchTo(TopMemoryContext); > + > + if (MemoryContextIsValid(basic_archive_context)) > + MemoryContex

[PATCH] Fix unbounded authentication exchanges during PQconnectPoll()

2023-02-13 Thread Jacob Champion
Hello, This is closely related to the prior conversation at [1]. There are a couple places in CONNECTION_AWAITING_RESPONSE where libpq will read a huge number of bytes from a server that we really should have hung up on. The attached patch adds a length check for the v2 error compatibility case,

pg_walinspect memory leaks

2023-02-13 Thread Peter Geoghegan
It looks like pg_walinspect's GetWALRecordsInfo() routine doesn't take sufficient care with memory management. It should avoid memory leaks of the kind that lead to OOMs whenever pg_get_wal_records_info_till_end_of_wal() has to return very many tuples. Right now it isn't that hard to make that happ

Re: Improve logging when using Huge Pages

2023-02-13 Thread Justin Pryzby
On Thu, Feb 09, 2023 at 11:29:09AM -0800, Nathan Bossart wrote: > On Thu, Feb 09, 2023 at 10:40:13AM +0100, Alvaro Herrera wrote: > > On 2023-Feb-08, Justin Pryzby wrote: > >> I don't think it makes sense to run postgres -C huge_pages_active, > >> however, so I see no issue that that would always r

Re: recovery modules

2023-02-13 Thread Andres Freund
Hi, On 2023-02-13 14:56:47 -0800, Nathan Bossart wrote: > On Mon, Feb 13, 2023 at 04:37:10PM +0900, Michael Paquier wrote: > > + basic_archive_context = data->context; > > + if (CurrentMemoryContext == basic_archive_context) > > + MemoryContextSwitchTo(TopMemoryContext); > > + > > + if

Re: Adding "large" to PG_TEST_EXTRA

2023-02-13 Thread Andres Freund
Hi, On 2023-02-14 09:26:47 +1100, Peter Smith wrote: > I've observed suggested test cases get rejected as being overkill, or > because they would add precious seconds to the test execution. OTOH, I > felt such tests would still help gain some additional percentages from > the "code coverage" stats

Re: Rework of collation code, extensibility

2023-02-13 Thread Jeff Davis
New version attached. Changes: * I moved the GUC patch to the end (so you can ignore it if it's not useful for review) * I cut out the pg_locale_internal.h rearrangement (at least for now, it might seem useful after the dust settles on the other changes). * I added a separate patch for pg_locale_d

Re: pglz compression performance, take two

2023-02-13 Thread Andres Freund
Hi, On 2023-02-08 11:16:47 +0100, Tomas Vondra wrote: > On 2/7/23 21:18, Andres Freund wrote: > > > > Independent of this failure, I'm worried about the cost/benefit analysis of > > a > > pglz change that changes this much at once. It's quite hard to review. > > > > I agree. > > I think I man

Re: pglz compression performance, take two

2023-02-13 Thread Andrey Borodin
On Mon, Feb 13, 2023 at 4:03 PM Andres Freund wrote: > > Due to the sanitizer changes, and this feedback, I'm marking the entry as > waiting on author. > Thanks Andres! Yes, I plan to make another attempt to refactor this patch on the weekend. If this attempt fails, I think we should just reject i

Re: Support for dumping extended statistics

2023-02-13 Thread Andres Freund
Hi, On 2023-02-01 04:38:17 +, Hari krishna Maddileti wrote: > Thanks for the update, I have attached the updated patch with > meson compatible and addressed warnings from make file too. This patch consistently crashes in CI: https://cirrus-ci.com/github/postgresql-cfbot/post

Re: pgbench: using prepared BEGIN statement in a pipeline could cause an error

2023-02-13 Thread Andres Freund
Hi, On 2023-02-08 13:10:40 +0100, Alvaro Herrera wrote: > On 2023-Feb-08, Alvaro Herrera wrote: > > > I propose instead the following: each command is prepared just before > > it's executed, as previously, and if we see a \startpipeline, then we > > prepare all commands starting with the one just

Buffer usage detailed by RelKind in EXPLAIN ANALYZE BUFFERS

2023-02-13 Thread Andrey Borodin
Hi hackers! I was asked to prototype a feature that helps to distinguish shared buffer usage between index reads and heap reads. Practically it looks like this: # explain (analyze,verbose,buffers) select nextval('s'); QUERY PLAN -

Re: Add the ability to limit the amount of memory that can be allocated to backends.

2023-02-13 Thread Andres Freund
Hi, On 2023-01-26 15:27:20 -0500, Reid Thompson wrote: > Yes, just a rebase. There is still work to be done per earlier in the > thread. The tests recently started to fail: https://cirrus-ci.com/github/postgresql-cfbot/postgresql/commitfest%2F42%2F3867 > I do want to follow up and note re pall

Re: Buffer usage detailed by RelKind in EXPLAIN ANALYZE BUFFERS

2023-02-13 Thread Andres Freund
Hi, On 2023-02-13 16:23:30 -0800, Andrey Borodin wrote: > But there are some caveats: > 1. Some more increments on hot paths. We have to add this tiny toll to > every single buffer hit, but it will be seldom of any use. Additionally, I bet it slows down EXPLAIN (ANALYZE, BUFFERS) noticeably. It's

Re: Make EXPLAIN generate a generic plan for a parameterized query

2023-02-13 Thread Andres Freund
Hi, On 2023-02-05 18:24:03 +0100, Laurenz Albe wrote: > Anyway, attached is v7 that tries to do it that way. This consistently fails on CI: https://cirrus-ci.com/github/postgresql-cfbot/postgresql/commitfest%2F42%2F3962 https://api.cirrus-ci.com/v1/artifact/task/5156723929907200/testrun/build/te

Re: Buffer usage detailed by RelKind in EXPLAIN ANALYZE BUFFERS

2023-02-13 Thread Andrey Borodin
On Mon, Feb 13, 2023 at 4:29 PM Andres Freund wrote: > > 1. Some more increments on hot paths. We have to add this tiny toll to > > every single buffer hit, but it will be seldom of any use. > > Additionally, I bet it slows down EXPLAIN (ANALYZE, BUFFERS) noticeably. It's > already quite expensive

Re: Inconsistency in reporting checkpointer stats

2023-02-13 Thread Andres Freund
On 2022-12-21 17:14:12 +0530, Nitin Jadhav wrote: > Kindly review and share your comments. 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

Re: Adding "large" to PG_TEST_EXTRA

2023-02-13 Thread Peter Smith
On Tue, Feb 14, 2023 at 10:44 AM Andres Freund wrote: > > Hi, > > On 2023-02-14 09:26:47 +1100, Peter Smith wrote: > > I've observed suggested test cases get rejected as being overkill, or > > because they would add precious seconds to the test execution. OTOH, I > > felt such tests would still he

Re: Buffer usage detailed by RelKind in EXPLAIN ANALYZE BUFFERS

2023-02-13 Thread Andres Freund
Hi, On 2023-02-13 16:36:25 -0800, Andrey Borodin wrote: > On Mon, Feb 13, 2023 at 4:29 PM Andres Freund wrote: > > > 1. Some more increments on hot paths. We have to add this tiny toll to > > > every single buffer hit, but it will be seldom of any use. > > > > Additionally, I bet it slows down EX

Re: Adding "large" to PG_TEST_EXTRA

2023-02-13 Thread Andres Freund
Hi, On 2023-02-14 11:38:06 +1100, Peter Smith wrote: > No, nothing specific in mind. But maybe like these: > - tests for causing obscure errors that would never otherwise be > reached without something deliberately designed to fail a certain way I think there's some cases around this that could b

Re: pg_walinspect memory leaks

2023-02-13 Thread Andres Freund
Hi, On 2023-02-13 15:22:02 -0800, Peter Geoghegan wrote: > More concretely, it looks like GetWALRecordInfo() calls > CStringGetTextDatum/cstring_to_text in a way that accumulates way too > much memory in ExprContext. Additionally, we leak two stringinfos for each record. > This could be avoided

Re: recovery modules

2023-02-13 Thread Nathan Bossart
On Mon, Feb 13, 2023 at 03:37:33PM -0800, Andres Freund wrote: > On 2023-02-13 14:56:47 -0800, Nathan Bossart wrote: >> On Mon, Feb 13, 2023 at 04:37:10PM +0900, Michael Paquier wrote: >> > + basic_archive_context = data->context; >> > + if (CurrentMemoryContext == basic_archive_context) >> > +

Re: recovery modules

2023-02-13 Thread Nathan Bossart
On Mon, Feb 13, 2023 at 04:55:58PM -0800, Nathan Bossart wrote: > Okay. I've added it back in v12 with the suggested adjustment for the > memory context stuff. Sorry for then noise, cfbot alerted me to a missing #include, which I've added in v13. -- Nathan Bossart Amazon Web Services: https://a

Re: Exit walsender before confirming remote flush in logical replication

2023-02-13 Thread Kyotaro Horiguchi
At Mon, 13 Feb 2023 08:27:01 +0530, Amit Kapila wrote in > On Mon, Feb 13, 2023 at 7:26 AM Kyotaro Horiguchi > wrote: > > > > IMHO I vaguely don't like that we lose a means to specify the default > > behavior here. And I'm not sure we definitely don't need other than > > flush and immedaite for

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

2023-02-13 Thread Andres Freund
Hi, On 2023-02-12 09:31:36 -0800, Andres Freund wrote: > Another thing: I think we should either avoid iterating over all the IOVs if > we don't need them, or, even better, initialize the array as a constant, once. I just tried to use pg_pwrite_zeros - and couldn't because it doesn't have an offs

Re: Move defaults toward ICU in 16?

2023-02-13 Thread Jeff Davis
On Thu, 2023-02-02 at 05:13 -0800, Jeff Davis wrote: > As a project, do we want to nudge users toward ICU as the collation > provider as the best practice going forward? One consideration here is security. Any vulnerability in ICU collation routines could easily become a vulnerability in Postgres.

Re: Exit walsender before confirming remote flush in logical replication

2023-02-13 Thread Andres Freund
On 2023-02-14 10:05:40 +0900, Kyotaro Horiguchi wrote: > What do you think about the need for explicitly specifying the > default? I'm fine with specifying the default using a single word, > such as WAIT_FOR_REMOTE_FLUSH. We obviously shouldn't force the option to be present. Why would we want to

Re: Fix GUC_NO_SHOW_ALL test scenario in 003_check_guc.pl

2023-02-13 Thread Kyotaro Horiguchi
At Mon, 13 Feb 2023 12:18:07 +0900, Michael Paquier wrote in > On Mon, Feb 13, 2023 at 11:27:58AM +0900, Kyotaro Horiguchi wrote: > > I think currently the output by --describe-config can be used only for > > consulting while editing a (possiblly broken) config file. Thus I > > think it's no us

Re: Support logical replication of DDLs

2023-02-13 Thread Peter Smith
FYI - the latest patch cannot be applied. See cfbot [1] -- [1] http://cfbot.cputube.org/patch_42_3595.log Kind Regards, Peter Smith. Fujitsu Australia

Re: A new strategy for pull-up correlated ANY_SUBLINK

2023-02-13 Thread Andy Fan
Hi All: Sorry for the delay. Once I saw Tom's reply on Nov 15, I tried his suggestion about "whether we need to restrict the optimization so that it doesn't occur if the subquery contains outer references falling outside available_rels. " quickly, I'm sure such a restriction can fix the bad

Re: Buffer usage detailed by RelKind in EXPLAIN ANALYZE BUFFERS

2023-02-13 Thread Andrey Borodin
On Mon, Feb 13, 2023 at 4:39 PM Andres Freund wrote: > > The problem I'm talking about is the increased overhead in InstrStopNode(), > due to BufferUsageAccumDiff() getting more expensive. > Thanks, now I understand the problem better. According to godbolt.com my patch doubles the number of instr

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

2023-02-13 Thread Kyotaro Horiguchi
At Mon, 13 Feb 2023 15:51:25 +0530, Amit Kapila wrote in > I think we can introduce a new variable as last_feedback_time in the > LogicalRepWorker structure and probably for the last_received, we can > last_lsn in MyLogicalRepWorker as that seems to be updated correctly. > I think it would be go

Re: Buffer usage detailed by RelKind in EXPLAIN ANALYZE BUFFERS

2023-02-13 Thread Andres Freund
Hi, On 2023-02-13 18:14:58 -0800, Andrey Borodin wrote: > On Mon, Feb 13, 2023 at 4:39 PM Andres Freund wrote: > > > > The problem I'm talking about is the increased overhead in InstrStopNode(), > > due to BufferUsageAccumDiff() getting more expensive. > > > > Thanks, now I understand the proble

Re: Some revises in adding sorting path

2023-02-13 Thread David Rowley
I looked at the three patches and have some thoughts: 0001: Does the newly added test have to be this complex? I think it might be better to just come up with the most simple test you can that uses an incremental sort. I really can't think why the test requires a FOR UPDATE, to test incremental

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

2023-02-13 Thread wangw.f...@fujitsu.com
On Thur, Feb 7, 2023 15:29 PM I wrote: > On Wed, Feb 1, 2023 20:07 PM Melih Mutlu wrote: > > Thanks for pointing out this review. I somehow skipped that, sorry. > > > > Please see attached patches. > > Thanks for updating the patch set. > Here are some comments. Hi, here are some more comments o

  1   2   >