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

2023-04-11 Thread David Rowley
On Wed, 12 Apr 2023 at 09:53, Tom Lane wrote: > > David Rowley writes: > > In preparation for when that's ticked off, I'd like to gather people's > > thoughts about if we should remove force_parallel_mode from v16? > > To clarify, you just mean removing

Re: ERROR messages in VACUUM's PARALLEL option

2023-04-11 Thread David Rowley
(On Wed, 12 Apr 2023 at 01:58, Tom Lane wrote: > > David Rowley writes: > > Over in [1], Horiguchisan mentioned a few things about VACUUM's new > > BUFFER_USAGE_LIMIT option. > > > 1) buffer_usage_limit in the ERROR messages should be consistently in > >

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

2023-04-11 Thread David Rowley
On Thu, 16 Feb 2023 at 00:05, David Rowley wrote: > I pushed the rename patch earlier. > > How should we go about making contact with the owners? After a quick round of making direct contact with the few remaining buildfarm machine owners which are still using force_parallel_mode, we&#x

Protecting allocator headers with Valgrind

2023-04-11 Thread David Rowley
Over on [1], Tom mentioned that we might want to rethink the decision to not protect chunk headers with Valgrind. That thread fixed a bug that was accessing array element -1, which effectively was reading the MemoryChunk at the start of the allocated chunk as an array element. I wrote a patch to

Re: An oversight in ExecInitAgg for grouping sets

2023-04-11 Thread David Rowley
On Mon, 9 Jan 2023 at 22:21, David Rowley wrote: > One extra thing I noticed was that I had to add a new > VALGRIND_MAKE_MEM_DEFINED in AllocSetAlloc when grabbing an item off > the freelist. I didn't quite manage to figure out why that's needed as > when we do AllocSetFr

ERROR messages in VACUUM's PARALLEL option

2023-04-11 Thread David Rowley
Over in [1], Horiguchisan mentioned a few things about VACUUM's new BUFFER_USAGE_LIMIT option. 1) buffer_usage_limit in the ERROR messages should be consistently in uppercase. 2) defGetString() already checks for opt->args == NULL and raises an ERROR when it is. I suspect that Melanie might have

Re: "an SQL" vs. "a SQL"

2023-04-10 Thread David Rowley
On Fri, 11 Jun 2021 at 13:44, David Rowley wrote: > Anyway, I'll set an alarm for this time next year so I can check on > how many inconsistencies have crept back in over the development > cycle. That alarm went off today. There seem to be only 3 "a SQL"s in the d

Re: Option to not use ringbuffer in VACUUM, using it in failsafe mode

2023-04-06 Thread David Rowley
On Fri, 7 Apr 2023 at 09:44, Melanie Plageman wrote: > Otherwise, LGTM. Thanks for looking. I've also taken Justin's comments about the README into account and fixed that part. I've pushed the patch after a little more adjusting. I added some text to the docs that mention larger VACUUM_BUFFER_

Re: Option to not use ringbuffer in VACUUM, using it in failsafe mode

2023-04-06 Thread David Rowley
On Fri, 7 Apr 2023 at 05:20, Melanie Plageman wrote: > > diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml > This still says that the default value is -1. Oops, I had this staged but didn't commit and formed the patch with "git diff master.." instead of "git diff master", so miss

Re: Option to not use ringbuffer in VACUUM, using it in failsafe mode

2023-04-06 Thread David Rowley
second On Thu, 6 Apr 2023 at 14:14, Melanie Plageman wrote: > > Attached is v14 which adds back in tests for the BUFFER_USAGE_LIMIT > option. I've spent quite a bit of time looking at this since you sent it. I've also made quite a few changes, mostly cosmetic ones, but there are a few things bel

Re: Option to not use ringbuffer in VACUUM, using it in failsafe mode

2023-04-05 Thread David Rowley
On Thu, 6 Apr 2023 at 13:14, David Rowley wrote: > Is it intended that VACUUM t1,t2; now share the same strategy? > Currently, in master, we'll allocate a new strategy for t2 after > vacuuming t1. Does this not mean we'll now leave fewer t1 pages in > shared_buffers be

Re: Option to not use ringbuffer in VACUUM, using it in failsafe mode

2023-04-05 Thread David Rowley
On Thu, 6 Apr 2023 at 12:42, Melanie Plageman wrote: > Attached is a v12 of the whole vacuum_buffer_usage_limit patch set which > includes a commit to fix the bug in master and a commit to move relevant > code from vacuum() up into ExecVacuum(). I'm still playing catch up to the moving of the pre

Re: Option to not use ringbuffer in VACUUM, using it in failsafe mode

2023-04-05 Thread David Rowley
On Wed, 5 Apr 2023 at 16:37, Peter Geoghegan wrote: > > On Tue, Apr 4, 2023 at 8:14 PM David Rowley wrote: > > 14. Not related to this patch, but why do we have half the vacuum > > related GUCs in vacuum.c and the other half in globals.c? I see > > vacuum_freeze_table_ag

Re: Negative cache entries for memoize

2023-04-05 Thread David Rowley
On Thu, 6 Apr 2023 at 03:12, Bruce Momjian wrote: > During two presentations, I was asked if negative cache entries were > created for cases where inner-side lookups returned no rows. > > It seems we don't do that. Has this been considered or is it planned? It does allow negative cache entries,

Re: Option to not use ringbuffer in VACUUM, using it in failsafe mode

2023-04-04 Thread David Rowley
On Wed, 5 Apr 2023 at 05:53, Melanie Plageman wrote: > Attached v10 addresses the review feedback below. Thanks. Here's another round on v10-0001: 1. The following documentation fragment does not seem to be aligned with the code: 16 GB. The minimum size is the lesser of 1/8 th

Re: Prefetch the next tuple's memory during seqscans

2023-04-03 Thread David Rowley
On Tue, 4 Apr 2023 at 07:47, Gregory Stark (as CFM) wrote: > The referenced patch was committed March 19th but there's been no > comment here. Is this patch likely to go ahead this release or should > I move it forward again? Thanks for the reminder on this. I have done some work on it but just

Re: Option to not use ringbuffer in VACUUM, using it in failsafe mode

2023-04-03 Thread David Rowley
On Tue, 4 Apr 2023 at 02:49, Melanie Plageman wrote: > v9 attached. I've made a pass on the v9-0001 patch only. Here's what I noted down: v9-0001: 1. In the documentation and comments, generally we always double-space after a period. I see quite often you're not following this. 2. Doc: We co

Re: Why enable_hashjoin Completely disables HashJoin

2023-04-03 Thread David Rowley
On Tue, 4 Apr 2023 at 11:18, Andres Freund wrote: > It sounds too hard compared to the gains, but another way could be to plan > with the relevant path generation hard disabled, and plan from scratch, with > additional scan types enabled, if we end up being unable to generate valid > plan. I thin

Re: Option to not use ringbuffer in VACUUM, using it in failsafe mode

2023-04-03 Thread David Rowley
I've now pushed up v8-0004. Can rebase the remaining 2 patches on top of master again and resend? On Mon, 3 Apr 2023 at 08:11, Melanie Plageman wrote: > I still have a few open questions: > - what the initial value of ring_size for autovacuum should be (see the > one remaining TODO in the code

Re: Option to not use ringbuffer in VACUUM, using it in failsafe mode

2023-04-02 Thread David Rowley
On Sat, 1 Apr 2023 at 13:24, Melanie Plageman wrote: > Your diff LGTM. > > Earlier upthread in [1], Bharath had mentioned in a review comment about > removing the global variables that he would have expected the analogous > global in analyze.c to also be removed (vac_strategy [and analyze.c also >

Re: Option to not use ringbuffer in VACUUM, using it in failsafe mode

2023-03-31 Thread David Rowley
On Sat, 1 Apr 2023 at 12:57, Melanie Plageman wrote: > I've attached v7 with that commit dropped and with support for parallel > vacuum workers to use the same number of buffers in their own Buffer > Access Strategy ring as the main vacuum phase did. I also updated the > docs to indicate that vacu

Re: Option to not use ringbuffer in VACUUM, using it in failsafe mode

2023-03-31 Thread David Rowley
On Sat, 1 Apr 2023 at 02:52, Melanie Plageman wrote: > There was one small typo keeping this from compiling. Also a repeated > word. I've fixed these. I also edited a bit of indentation and tweaked > some wording. Diff attached (to be applied on top of your diff). Thanks for fixing that mistake.

Re: Option to not use ringbuffer in VACUUM, using it in failsafe mode

2023-03-30 Thread David Rowley
On Mon, 20 Mar 2023 at 11:50, Melanie Plageman wrote: > Attached is an updated v6. I had a look over the v6-0001 patch. There are a few things I think could be done better: "Some operations will access a large number of pages at a time", does this really need "at a time"? I think it's more rele

Re: [RFC] Add jit deform_counter

2023-03-28 Thread David Rowley
On Sun, 12 Jun 2022 at 21:14, Dmitry Dolgov <9erthali...@gmail.com> wrote: > I've noticed that JIT performance counter generation_counter seems to include > actions, relevant for both jit_expressions and jit_tuple_deforming options. It > means one can't directly see what is the influence of jit_tup

Re: Some revises in adding sorting path

2023-03-28 Thread David Rowley
On Tue, 21 Feb 2023 at 22:02, Richard Guo wrote: > Looking at the codes now I have some concern that what we do in > create_ordered_paths for partial paths may have already been done in > generate_useful_gather_paths, especially when query_pathkeys is equal to > sort_pathkeys. Not sure if this is

Re: Refactoring SysCacheGetAttr to know when attr cannot be NULL

2023-03-24 Thread David Rowley
On Fri, 24 Mar 2023 at 20:31, Peter Eisentraut wrote: > > On 23.03.23 09:52, David Rowley wrote: > > One thing I thought about while looking is it stage 2 might do > > something similar for SearchSysCacheN. I then wondered if we're more > > likely to want to keep the

Re: Improve the performance of nested loop join in the case of partitioned inner table

2023-03-23 Thread David Rowley
On Thu, 23 Mar 2023 at 19:46, Alexandr Nikulin wrote: > I propose to slightly improve the performance of nested loop join in the case > of partitioned inner table. > As I see in the code, the backend looks for the partition of the inner table > each time after fetch a new row from the outer tabl

Re: Refactoring SysCacheGetAttr to know when attr cannot be NULL

2023-03-23 Thread David Rowley
On Tue, 14 Mar 2023 at 02:19, Daniel Gustafsson wrote: > Rebased v3 on top of recent conflicting ICU changes causing the patch to not > apply anymore. Also took another look around the tree to see if there were > missed callsites but found none new. I had a look at this. It generally seems like

Re: An oversight in ExecInitAgg for grouping sets

2023-03-22 Thread David Rowley
On Mon, 20 Mar 2023 at 19:18, Richard Guo wrote: > It occurred to me that this hasn't been applied. Should we add it to > the CF to not lose track of it? I have a git branch with it. That'll work for me personally as a reminder to come back to it during the v17 cycle. David

Re: Comment in preptlist.c

2023-03-22 Thread David Rowley
On Wed, 22 Mar 2023 at 20:40, Etsuro Fujita wrote: > Thanks for picking this up, David! Thanks for looking, Tom and Richard! And now it just clicked with me why Tom left this. Sorry for stepping on your toes here. David

Re: Comment in preptlist.c

2023-03-21 Thread David Rowley
On Tue, 21 Mar 2023 at 22:41, Etsuro Fujita wrote: > I think that “planner/rewriter” should be parser/rewriter. Attached > is a patch for that. Pushed. David

Re: Adjust Memoize hit_ratio calculation

2023-03-21 Thread David Rowley
On Tue, 21 Mar 2023 at 09:41, David Rowley wrote: > Because that's being changed in v16, I think it might also be a good > idea to fix the hit_ratio calculation problem reported by David > Johnston in [1]. In the attached, I've adjusted David's calculation > slight

Adjust Memoize hit_ratio calculation

2023-03-20 Thread David Rowley
Yesterday, in 785f70957, I adjusted the Memoize costing code to account for the size of the cache key when estimating how many cache entries can exist at once in the cache. That effectively makes Memoize a less likely choice as fewer entries will be expected to fit in work_mem now. Because that's

Re: Save a few bytes in pg_attribute

2023-03-20 Thread David Rowley
On Mon, 20 Mar 2023, 11:00 pm Peter Eisentraut, wrote: > After the discussion in [0] ff., I was looking around in pg_attribute > and noticed that we could possibly save 4 bytes. We could change both > attstattarget and attinhcount from int4 to int2, which together with > some reordering would sav

Re: Lock conflict

2023-03-19 Thread David Rowley
On Mon, 20 Mar 2023 at 14:58, 席冲(宜穆) wrote: > I think lock requested only check for conflict with already-held lock, if > there is no conflict, the lock should be granted. That would mean that stronger locks such as AEL might never be granted if there was never any moment when no other conflicti

Re: heapgettup refactoring

2023-03-19 Thread David Rowley
On Wed, 8 Feb 2023 at 15:09, David Rowley wrote: > Using the tests mentioned in [1], I tested out > remove_HeapScanDescData_rs_inited_field.patch. It's not looking very > promising at all. In light of the performance regression from removing the rs_inited field, let's just fo

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

2023-03-08 Thread David Rowley
On Thu, 9 Mar 2023 at 01:34, Alvaro Herrera wrote: > David, do you intend to continue to be involved in reviewing this one? Yes. I'm currently trying to make a few Bitmapset improvements which include the change made in this thread's 0001 patch over on [1]. For the main patch, I've been starting

Re: optimize several list functions with SIMD intrinsics

2023-03-07 Thread David Rowley
On Wed, 8 Mar 2023 at 13:25, Nathan Bossart wrote: > I've attached a work-in-progress patch that implements these optimizations > for both x86 and arm, and I will register this in the July commitfest. I'm > posting this a little early in order to gauge interest. Interesting and quite impressive

Re: Add support for unit "B" to pg_size_pretty()

2023-03-07 Thread David Rowley
On Wed, 8 Mar 2023 at 09:22, Peter Eisentraut wrote: > Ok, I have fixed the original documentation to that effect and > backpatched it. Thanks for fixing that. David

Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment

2023-03-07 Thread David Rowley
On Sun, 5 Mar 2023 at 13:21, Lukas Fittl wrote: > Alternatively (or in addition) we could consider showing the "ndistinct" > value that is calculated in cost_memoize_rescan - since that's the most > significant contributor to the cache hit ratio (and you can influence that > directly by improvi

Re: using memoize in in paralel query decreases performance

2023-03-07 Thread David Rowley
On Tue, 7 Mar 2023 at 22:09, Pavel Stehule wrote: > I can live with it. This is an analytical query and the performance is not > too important for us. I was surprised that the performance was about 25% > worse, and so the hit ratio was almost zero. I am thinking, but I am not sure > if the esti

Re: using memoize in in paralel query decreases performance

2023-03-07 Thread David Rowley
/On Tue, 7 Mar 2023 at 21:09, Pavel Stehule wrote: > > po 6. 3. 2023 v 22:52 odesílatel David Rowley napsal: >> I wonder if the additional work_mem required for Memoize is just doing >> something like causing kernel page cache evictions and leading to &g

Re: Making empty Bitmapsets always be NULL

2023-03-06 Thread David Rowley
On Sat, 4 Mar 2023 at 11:08, Tom Lane wrote: > > David Rowley writes: > > On Fri, 3 Mar 2023 at 15:17, Tom Lane wrote: > > It's true that the majority of Bitmapsets are going to be just 1 word, > > but it's important to acknowledge that we do suffer i

Re: Inaccurate comment for pg_get_partkeydef

2023-03-06 Thread David Rowley
On Mon, 6 Mar 2023 at 18:54, Japin Li wrote: > PSA patch to fix a comment inaccurate. Thanks. Pushed. David

Re: using memoize in in paralel query decreases performance

2023-03-06 Thread David Rowley
On Mon, 6 Mar 2023 at 21:55, Pavel Stehule wrote: > default https://explain.depesz.com/s/fnBe It looks like the slowness is coming from the Bitmap Index scan and Bitmap heap scan rather than Memoize. -> Bitmap Heap Scan on public.item i (cost=285.69..41952.12 rows=29021 width=16) (actua

Re: Add support for unit "B" to pg_size_pretty()

2023-03-06 Thread David Rowley
On Mon, 6 Mar 2023 at 21:13, Peter Eisentraut wrote: > > On 02.03.23 20:58, David Rowley wrote: > > I think I'd prefer to see the size_bytes_unit_alias struct have an > > index into size_pretty_units[] array. i.e: > > Ok, done that way. (I had thought about that,

Re: using memoize in in paralel query decreases performance

2023-03-06 Thread David Rowley
On Mon, 6 Mar 2023 at 20:34, Pavel Stehule wrote: > In one query I can see very big overhead of memoize node - unfortunately with > hits = 0 > > The Estimate is almost very good. See details in attachment Are you able to share the version number for this? Also, it would be good to see EXPLAIN A

Re: Allow ordered partition scans in more cases

2023-03-03 Thread David Rowley
On Thu, 23 Feb 2023 at 02:10, Ronan Dunklau wrote: > I haven't looked too deeply into it, but it seems reasonable that the whole > sort would cost cheaper than individual sorts on partitions + incremental > sorts, except when the the whole sort would spill to disk much more than the > incremental

Re: Add support for unit "B" to pg_size_pretty()

2023-03-03 Thread David Rowley
On Fri, 3 Mar 2023 at 09:32, Dean Rasheed wrote: > Hmm, I think it would be easier to just have a separate table for > pg_size_bytes(), rather than reusing pg_size_pretty()'s table. I.e., > size_bytes_units[], which would only need name and multiplier columns > (not round and limit). Done that way

Re: min/max aggregation for jsonb

2023-03-03 Thread David Rowley
On Fri, 3 Mar 2023 at 23:17, Daneel Yaitskov wrote: > I wanted to use min/max aggregation functions for jsonb type and noticed > there is no functions for this type, meanwhile string/array types are > supported. It's not really clear to me how you'd want these to sort. If you just want to sort b

Re: Making empty Bitmapsets always be NULL

2023-03-02 Thread David Rowley
On Fri, 3 Mar 2023 at 15:17, Tom Lane wrote: > (Is it worth carrying both "allocated words" and "nonzero words" > fields to avoid useless memory-management effort? Dunno.) It would have been a more appealing thing to do before Bitmapset became a node type as we'd have had empty space in the stru

Re: Making empty Bitmapsets always be NULL

2023-03-02 Thread David Rowley
On Wed, 1 Mar 2023 at 10:59, Tom Lane wrote: > When I designed the Bitmapset module, I set things up so that an empty > Bitmapset could be represented either by a NULL pointer, or by an > allocated object all of whose bits are zero. I've recently come to > the conclusion that that was a bad idea

Re: Add support for unit "B" to pg_size_pretty()

2023-03-02 Thread David Rowley
On Mon, 27 Feb 2023 at 21:34, Peter Eisentraut wrote: > > On 22.02.23 03:39, David Rowley wrote: > > I think you'll need to find another way to make the aliases work. > > Maybe another array with the name and an int to reference the > > corresponding index in size_pr

Re: Add support for unit "B" to pg_size_pretty()

2023-02-21 Thread David Rowley
On Wed, 22 Feb 2023 at 12:47, Peter Eisentraut wrote: > >> diff --git a/src/backend/utils/adt/dbsize.c > >> b/src/backend/utils/adt/dbsize.c > >> index dbd404101f..9ecd5428c3 100644 > >> --- a/src/backend/utils/adt/dbsize.c > >> +++ b/src/backend/utils/adt/dbsize.c > >> @@ -49,6 +49,7 @@ struct s

Re: Reducing System Allocator Thrashing of ExecutorState to Alleviate FDW-related Performance Degradations

2023-02-21 Thread David Rowley
On Sat, 18 Feb 2023 at 06:52, Andres Freund wrote: > And did you increase ALLOCSET_DEFAULT_INITSIZE everywhere, or just passed a > larger block size in CreateExecutorState()? If the latter,the context > freelist wouldn't even come into play. I think this piece of information is critical to confi

Allow ordered partition scans in more cases

2023-02-20 Thread David Rowley
Over on [1], Benjamin highlighted that we don't do ordered partition scans in some cases where we could. Basically, what was added in 959d00e9d only works when at least one child path has pathkeys that suit the required query pathkeys. If the partitions or partitioned table does not have an index

Re: Reducing System Allocator Thrashing of ExecutorState to Alleviate FDW-related Performance Degradations

2023-02-20 Thread David Rowley
On Tue, 21 Feb 2023 at 07:30, Andres Freund wrote: > 2) We should introduce an API mcxt.c API to perform allocations that the >caller promises not to individually free. It's not just pfree. Offhand, there's also repalloc, GetMemoryChunkSpace and GetMemoryChunkContext too. I am interested in

Re: Introduce list_reverse() to make lcons() usage less inefficient

2023-02-20 Thread David Rowley
On Fri, 17 Feb 2023 at 16:35, David Rowley wrote: > > On Fri, 17 Feb 2023 at 13:23, Andres Freund wrote: > > But wouldn't an even cheaper way here be to iterate over the children in > > reverse order when match_partition_order_desc? We can do that efficiently > > no

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: Make set_ps_display faster and easier to use

2023-02-17 Thread David Rowley
Thank you for having a look at this. On Fri, 17 Feb 2023 at 14:01, Andres Freund wrote: > > +set_ps_display_suffix(const char *suffix) > > +{ > > + size_t len; > > Think this will give you an unused-variable warning in the PS_USE_NONE case. Fixed > > +#ifndef PS_USE_NONE > > + /* updat

Re: Reducing System Allocator Thrashing of ExecutorState to Alleviate FDW-related Performance Degradations

2023-02-16 Thread David Rowley
On Fri, 17 Feb 2023 at 17:40, Jonah H. Harris wrote: > Yeah. There’s definitely a smarter and more reusable approach than I was > proposing. A lot of that code is fairly mature and I figured more people > wouldn’t want to alter it in such ways - but I’m up for it if an approach > like this is t

Re: Reducing System Allocator Thrashing of ExecutorState to Alleviate FDW-related Performance Degradations

2023-02-16 Thread David Rowley
On Fri, 17 Feb 2023 at 16:40, Andres Freund wrote: > I'd like a workload that hits a perf issue with this, because I think there > likely are some general performance improvements that we could make, without > changing the initial size or the "growth rate". I didn't hear it mentioned explicitly h

Re: Introduce list_reverse() to make lcons() usage less inefficient

2023-02-16 Thread David Rowley
On Fri, 17 Feb 2023 at 13:23, Andres Freund wrote: > But wouldn't an even cheaper way here be to iterate over the children in > reverse order when match_partition_order_desc? We can do that efficiently > now. Looks like we don't have a readymade helper for it, but it'd be easy > enough to add or o

Introduce list_reverse() to make lcons() usage less inefficient

2023-02-16 Thread David Rowley
While working on [1] to make improvements in the query planner around the speed to find EquivalenceMembers in an EquivalenceClass, because that patch does have a large impact in terms of performance improvements, some performance tests with that patch started to highlight some other places that bot

Re: Todo: Teach planner to evaluate multiple windows in the optimal order

2023-02-15 Thread David Rowley
On Thu, 16 Feb 2023 at 00:45, John Naylor wrote: > Okay, here's a rerun including the sort hack, and it looks like incremental > sort is only ahead with the smallest set, otherwise same or maybe slightly > slower: > > HEAD: > > 4 ^ 8: latency average = 113.461 ms > 5 ^ 8: latency average = 786.0

Make set_ps_display faster and easier to use

2023-02-15 Thread David Rowley
While doing some benchmarking of some fast-to-execute queries, I see that set_ps_display() popping up on the profiles. Looking a little deeper, there are some inefficiencies in there that we could fix. For example, the following is pretty poor: strlcpy(ps_buffer + ps_buffer_fixed_size, activity,

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

2023-02-15 Thread David Rowley
On Wed, 15 Feb 2023 at 14:10, Andrew Dunstan wrote: > We'll email them once this is in. Most people are fairly reponsive. I pushed the rename patch earlier. How should we go about making contact with the owners? I'm thinking it may be better coming from you, especially if you think technical de

Re: Todo: Teach planner to evaluate multiple windows in the optimal order

2023-02-15 Thread David Rowley
On Wed, 15 Feb 2023 at 17:23, John Naylor wrote: > HEAD: > > 4 ^ 8: latency average = 113.976 ms > 5 ^ 8: latency average = 783.830 ms > 6 ^ 8: latency average = 3990.351 ms > 7 ^ 8: latency average = 15793.629 ms > > Skip rechecking first key: > > 4 ^ 8: latency average = 107.028 ms > 5 ^ 8: late

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

2023-02-14 Thread David Rowley
On Wed, 15 Feb 2023 at 11:27, Andrew Dunstan wrote: > It's just occurred to me that this could break the buildfarm fairly > comprehensively. I just took a count and we have 74 members using > force_parallel_mode. Maybe we need to keep force_parallel_mode as an > alternative spelling for debug_p

Re: Todo: Teach planner to evaluate multiple windows in the optimal order

2023-02-14 Thread David Rowley
On Tue, 14 Feb 2023 at 17:21, John Naylor wrote: > Not rechecking seems to eliminate the regression in 4 cases, and reduce it in > the other 2 cases. For those 2 cases (10e6 rows, random, mod 10 and 100), it > might be worthwhile to "zoom in" with more measurements, but haven't done > that yet.

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: 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 at

Re: Making aggregate deserialization (and WAL receive) functions slightly faster

2023-02-12 Thread David Rowley
On Sun, 12 Feb 2023 at 19:39, Tom Lane wrote: > I find this patch horribly dangerous. I see LogicalRepApplyLoop() does something similar with a StringInfoData. Maybe it's just scarier having an external function in stringinfo.c which does this as having it increases the chances of someone using i

Making aggregate deserialization (and WAL receive) functions slightly faster

2023-02-11 Thread David Rowley
workers: $q psql -c "alter table t set (parallel_workers = $i);" postgres echo $q > bench.sql pgbench -f bench.sql -n -T 10 postgres | grep latency done done From 75d97a066ac81f5a50b1b7618ad9e240f

Re: Speed up transaction completion faster after many relations are accessed in a transaction

2023-02-09 Thread David Rowley
#x27;ve attached another set of patches. I do need to spend longer looking at this. I'm mainly attaching these as CI seems to be highlighting a problem that I'm unable to recreate locally and I wanted to see if the attached fixes it. David From 4a546ad6d33e544dd872b23a94925a262088cd9a

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

2023-02-09 Thread David Rowley
On Thu, 9 Feb 2023 at 21:20, John Naylor wrote: > Looks good at a glance, just found a spurious word: > > + "by forcing the planner into to generate plans which contains nodes " Thanks for looking. I'll fix that. Likely the hardest part to get right here is the new name. Can anyone think of any

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

2023-02-08 Thread David Rowley
On Thu, 9 Feb 2023 at 11:26, Tom Lane wrote: > > David Rowley writes: > > I've attached a patch which does the renaming to debug_parallel_query. > > I've made it so the old name can still be used. > > There's a better way to do that last, which is to add

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

2023-02-08 Thread David Rowley
On Thu, 2 Feb 2023 at 01:24, John Naylor wrote: > > > On Wed, Feb 1, 2023 at 6:41 PM David Rowley wrote: > > > > I don't really share Laurenz's worry [2] about compatibility break > > from renaming this GUC. I think the legitimate usages of this setting &g

Re: heapgettup refactoring

2023-02-07 Thread David Rowley
On Wed, 8 Feb 2023 at 09:41, Melanie Plageman wrote: > > On Tue, Feb 07, 2023 at 05:40:13PM +1300, David Rowley wrote: > > I ended up adjusting HeapScanDescData more than what is minimally > > required to remove rs_inited. I wondered if rs_cindex should be closer > > to rs

Re: heapgettup refactoring

2023-02-06 Thread David Rowley
On Fri, 3 Feb 2023 at 15:26, David Rowley wrote: > I've pushed all but the final 2 patches now. I just pushed the final patch in the series. I held back on moving the setting of rs_inited back into the heapgettup_initial_block() helper function as I wondered if we should even keep th

Re: heapgettup refactoring

2023-02-02 Thread David Rowley
On Fri, 3 Feb 2023 at 06:23, Melanie Plageman wrote: > Your code also switches to saving the OffsetNumber -- just in a separate > variable of OffsetNumber type. I am fine with this if it your rationale > is that it is not a good idea to store a smaller number in a larger > datatype. However, the b

Re: heapgettup refactoring

2023-02-01 Thread David Rowley
move away from looking at the last tuple to get this number I've attached the rebased and updated patch. David [1] https://postgr.es/m/20221101010948.hsf33emgnwzvi...@awork3.anarazel.de From 32652b312d0c802e3105c064c24ff5b99694399c Mon Sep 17 00:00:00 2001 From: David Rowley Date: Thu, 2 Feb 20

Re: pg_dump versus hash partitioning

2023-02-01 Thread David Rowley
On Thu, 2 Feb 2023 at 11:38, Tom Lane wrote: > > Peter Geoghegan writes: > > You mentioned "minor releases" here. Who said anything about that? > > I did: I'd like to back-patch the fix if possible. I think changing > the default --load-via-partition-root choice could be back-patchable. > > If R

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

2023-02-01 Thread David Rowley
On Thu, 30 Jun 2022 at 00:31, Justin Pryzby wrote: > > On Wed, Jun 29, 2022 at 03:23:27PM +1200, David Rowley wrote: > > Over on [1] I noticed that the user had set force_parallel_mode to > > "on" in the hope that would trick the planner into making their query >

Re: heapgettup refactoring

2023-02-01 Thread David Rowley
else is too), then I can push it in the (NZ) morning. The remaining patches would need to be rebased due to my changes. David From cbd37463bdaa96afed4c7c739c8e91b770a9f8a7 Mon Sep 17 00:00:00 2001 From: David Rowley Date: Wed, 1 Feb 2023 19:35:16 +1300 Subject: [PATCH v8] Refactor heapam.c addi

Re: heapgettup refactoring

2023-01-31 Thread David Rowley
On Tue, 31 Jan 2023 at 12:18, Melanie Plageman wrote: > > On Fri, Jan 27, 2023 at 10:34 PM David Rowley wrote: > > 4. I think it might be a good idea to use unlikely() in if > > (!scan->rs_inited). The idea is to help coax the compiler into moving > > that code off to

Re: Generating "Subplan Removed" in EXPLAIN

2023-01-31 Thread David Rowley
On Wed, 1 Feb 2023 at 15:53, Yugo NAGATA wrote: > Maybe, you missed to set plan_cache_mode to force_generic_plan. > "Subplan Removed" doesn't appear when using a custom plan. I wouldn't say that's 100% true. The planner is only able to prune using values which are known during planning. Constant

Re: heapgettup() with NoMovementScanDirection unused in core?

2023-01-31 Thread David Rowley
On Wed, 1 Feb 2023 at 03:02, Melanie Plageman wrote: > > On Tue, Jan 31, 2023 at 11:46:05PM +1300, David Rowley wrote: > > Both can be easily fixed, so no need to submit another patch as far as > > I'm concerned. > > I realized I forgot a commit message in the secon

Re: heapgettup() with NoMovementScanDirection unused in core?

2023-01-31 Thread David Rowley
On Wed, 1 Feb 2023 at 03:02, Melanie Plageman wrote: > > On Tue, Jan 31, 2023 at 11:46:05PM +1300, David Rowley wrote: > > My thoughts were that we might want to put them > > table_scan_getnextslot() and table_scan_getnextslot_tidrange(). My > > rationale for that was tha

Re: heapgettup() with NoMovementScanDirection unused in core?

2023-01-31 Thread David Rowley
On Tue, 31 Jan 2023 at 09:57, Melanie Plageman wrote: > As for the asserts, I was at a bit of a loss as to where to put an > assert which will make it clear that heapgettup() and > heapgettup_pagemode() do not handle NoMovementScanDirection but was > at a higher level of the executor. My thoughts

Re: Prefetch the next tuple's memory during seqscans

2023-01-29 Thread David Rowley
On Wed, 4 Jan 2023 at 23:06, vignesh C wrote: > patching file src/backend/access/heap/heapam.c > Hunk #1 FAILED at 451. > 1 out of 6 hunks FAILED -- saving rejects to file > src/backend/access/heap/heapam.c.rej I've moved this patch to the next CF. This patch has a dependency on what's being pro

Re: heapgettup refactoring

2023-01-27 Thread David Rowley
"On Wed, 25 Jan 2023 at 10:17, Melanie Plageman wrote: > I've added a comment but I didn't include the function name in it -- I > find it repetitive when the comments above functions do that -- however, > I'm not strongly attached to that. I think the general format for header comments is: /* *

Re: heapgettup() with NoMovementScanDirection unused in core?

2023-01-27 Thread David Rowley
On Sat, 28 Jan 2023 at 12:15, Tom Lane wrote: > /* > * Determine the net effect of two direction specifications. > * This relies on having ForwardScanDirection = +1, BackwardScanDirection = > -1, > * and will probably not do what you want if applied to any other values. > */ > #define Combine

Re: Monotonic WindowFunc support for ntile(), percent_rank() and cume_dist()

2023-01-26 Thread David Rowley
On Wed, 25 Jan 2023 at 02:39, Melanie Plageman wrote: > > On Tue, Jan 24, 2023 at 02:00:33PM +1300, David Rowley wrote: > > If you feel strongly about that, then feel free to show me what you > > have in mind in more detail so I can think harder about it. > > Nah, I don&

Re: wrong Append/MergeAppend elision?

2023-01-26 Thread David Rowley
On Fri, 27 Jan 2023 at 01:30, Amit Langote wrote: > It seems that the planner currently elides an Append/MergeAppend that > has run-time pruning info (part_prune_index) set, but which I think is > a bug. This is actually how I intended it to work. Whether it was a good idea or not, I'm currently

Re: Considering additional sort specialisation functions for PG16

2023-01-26 Thread David Rowley
On Thu, 26 Jan 2023 at 23:29, John Naylor wrote: > Coming back to this, I wanted to sketch out this idea in a bit more detail. > > Have two memtuple arrays, one for first sortkey null and one for first > sortkey non-null: > - Qsort the non-null array, including whatever specialization is availabl

Re: heapgettup() with NoMovementScanDirection unused in core?

2023-01-25 Thread David Rowley
On Wed, 25 Jan 2023 at 13:55, Melanie Plageman wrote: > David Rowley and I were discussing how to test the > NoMovementScanDirection case for heapgettup() and heapgettup_pagemode() > in [1] (since there is not currently coverage). We are actually > wondering if it is dead code (in

Re: document the need to analyze partitioned tables

2023-01-25 Thread David Rowley
On Wed, 25 Jan 2023 at 19:46, Laurenz Albe wrote: > Did you see Justin's wording suggestion in > https://postgr.es/m/20230118174919.GA9837%40telsasoft.com ? > He didn't attach it as a patch, so you may have missed it. > I was pretty happy with that. I didn't pay too much attention as I tend to ap

Re: document the need to analyze partitioned tables

2023-01-24 Thread David Rowley
On Wed, 18 Jan 2023 at 22:15, Laurenz Albe wrote: > Attached is a new version of my patch that tries to improve the wording. I had a look at this and agree that we should adjust the paragraph in question if people are finding it confusing. For your wording, I found I had a small problem with cal

Re: Speed up transaction completion faster after many relations are accessed in a transaction

2023-01-23 Thread David Rowley
On Fri, 20 Jan 2023 at 00:26, vignesh C wrote: > CFBot shows some compilation errors as in [1], please post an updated > version for the same: I've attached a rebased patch. While reading over this again, I wondered if instead of allocating the memory for the LOCALLOCKOWNER in TopMemoryContext,

<    3   4   5   6   7   8   9   10   11   12   >