Re: Add bump memory context type and use it for tuplesorts

2024-02-20 Thread David Rowley
On Wed, 26 Jul 2023 at 12:11, Nathan Bossart wrote: > I think it'd be okay to steal those bits. AFAICT it'd complicate the > macros in memutils_memorychunk.h a bit, but that doesn't seem like such a > terrible price to pay to allow us to keep avoiding the glibc bit patterns. I've not adjusted an

Re: Add bump memory context type and use it for tuplesorts

2024-02-20 Thread David Rowley
Thanks for having a look at this. On Tue, 7 Nov 2023 at 07:55, Matthias van de Meent wrote: > I think it would make sense to split the "add a bump allocator" > changes from the "use the bump allocator in tuplesort" patches. I've done this and will post updated patches after replying to the other

Re: JIT compilation per plan node

2024-02-19 Thread David Rowley
On Tue, 20 Feb 2024 at 18:31, Tom Lane wrote: > FWIW, I seriously doubt that an extra walk of the plan tree is even > measurable compared to the number of cycles JIT compilation will > expend if it's called. So I don't buy your argument here. > We would be better off to do this in a way that's cl

Re: JIT compilation per plan node

2024-02-19 Thread David Rowley
On Tue, 20 Feb 2024 at 05:26, Tomas Vondra wrote: > I doubt CreatePlanContext is a great way to achieve this. For one, it > breaks the long-standing custom that PlannerInfo is the first parameter, > usually followed by RelOptInfo, etc. CreatePlanContext is added to some > functions (but not all),

Support boolcol IS [NOT] UNKNOWN in partition pruning

2024-02-19 Thread David Rowley
f1f0dced9e1165ffa8c93c7f1 Mon Sep 17 00:00:00 2001 From: David Rowley Date: Tue, 20 Feb 2024 14:56:49 +1300 Subject: [PATCH v1] Support partition pruning on boolcol IS [NOT] UNKNOWN While working on 4c2369ac5, I noticed we went out of our way not to support clauses on boolean partitioned tables i

Re: make add_paths_to_append_rel aware of startup cost

2024-02-15 Thread David Rowley
On Fri, 16 Feb 2024 at 01:09, David Rowley wrote: > > On Thu, 15 Feb 2024 at 21:42, Andy Fan wrote: > > I found the both plans have the same cost, I can't get the accurate > > cause of this after some hours research, but it is pretty similar with > > 7516056c584e3,

Re: make add_paths_to_append_rel aware of startup cost

2024-02-15 Thread David Rowley
On Thu, 15 Feb 2024 at 21:42, Andy Fan wrote: > I found the both plans have the same cost, I can't get the accurate > cause of this after some hours research, but it is pretty similar with > 7516056c584e3, so I uses a similar strategy to stable it. is it > acceptable? It's pretty hard to say. I

Re: Properly pathify the union planner

2024-02-14 Thread David Rowley
On Wed, 7 Feb 2024 at 12:05, Andy Fan wrote: > +static int > +pathkeys_useful_for_setop(PlannerInfo *root, List *pathkeys) > +{ > + int n_common_pathkeys; > + > + if (root->setop_pathkeys == NIL) > + return 0; /* no specia

Re: Properly pathify the union planner

2024-02-14 Thread David Rowley
On Tue, 6 Feb 2024 at 22:05, Richard Guo wrote: > I'm thinking that maybe it'd be better to move the work of sorting the > subquery's paths to the outer query level, specifically within the > build_setop_child_paths() function, just before we stick SubqueryScanPath > on top of the subquery's paths

Re: An improvement on parallel DISTINCT

2024-02-07 Thread David Rowley
On Mon, 5 Feb 2024 at 14:42, Richard Guo wrote: > > > On Fri, Feb 2, 2024 at 7:36 PM David Rowley wrote: >> I think we should just make it work the same way as >> create_grouping_paths(), where grouping_target is passed as a >> parameter. >> >> I've

Re: An improvement on parallel DISTINCT

2024-02-02 Thread David Rowley
On Fri, 2 Feb 2024 at 23:39, David Rowley wrote: > I'll push the patch shortly. I've pushed the partial path sort part. Now for the other stuff you had. I didn't really like this part: + /* + * Set target for partial_distinct_rel as generate_useful_gather_paths + * requi

Re: An improvement on parallel DISTINCT

2024-02-02 Thread David Rowley
On Fri, 2 Feb 2024 at 20:47, Richard Guo wrote: > > > On Fri, Feb 2, 2024 at 11:26 AM David Rowley wrote: >> >> In light of this, do you still think it's worthwhile making this change? >> >> For me, I think all it's going to result in is extra pl

Re: An improvement on parallel DISTINCT

2024-02-01 Thread David Rowley
On Wed, 27 Dec 2023 at 00:23, Richard Guo wrote: > -- on master > EXPLAIN (costs off) > SELECT DISTINCT four FROM tenk1; > QUERY PLAN > > Unique >-> Sort > Sort Key: four > -> Gather >

Re: set_cheapest without checking pathlist

2024-02-01 Thread David Rowley
On Thu, 1 Feb 2024 at 19:36, Richard Guo wrote: > maybe we can add an Assert or a comment clarifying that the pathlist of > partially_grouped_rel cannot be empty here. There'd be no need to Assert that as set_cheapest() will raise an error when given a rel with an empty pathlist. I don't think p

Re: set_cheapest without checking pathlist

2024-01-31 Thread David Rowley
On Thu, 1 Feb 2024 at 16:29, Richard Guo wrote: > > > On Thu, Feb 1, 2024 at 10:04 AM James Coleman wrote: >> >> I don't see any inherent reason why we must always assume that >> gather_grouping_paths will always result in having at least one entry >> in pathlist. If, for example, we've disabled

Re: Incorrect cost for MergeAppend

2024-01-31 Thread David Rowley
On Thu, 1 Feb 2024 at 04:32, Tom Lane wrote: > > Alvaro Herrera writes: > > Since we have a minor coming up very soon, I think it's not a good idea > > to backpatch right now. Maybe you can push to master now, and consider > > whether to backpatch later. > > As a rule, we don't back-patch change

Re: Incorrect cost for MergeAppend

2024-01-30 Thread David Rowley
On Wed, 31 Jan 2024 at 18:58, Ashutosh Bapat wrote: > with patch > Merge Append (cost=6.94..18.90 rows=198 width=4) > without patch > Sort (cost=19.04..19.54 rows=198 width=4) > Those numbers are higher than 1% (#define STD_FUZZ_FACTOR 1.01) but > slight variation in all the GUCs that affect

Re: Apply the "LIMIT 1" optimization to partial DISTINCT

2024-01-30 Thread David Rowley
On Fri, 26 Jan 2024 at 21:14, David Rowley wrote: > However, having said that. Parallel plans are often picked when there > is some highly selective qual as parallel_tuple_cost has to be applied > to fewer tuples for such plans, so probably this is worth doing. I was messing around

Re: Incorrect cost for MergeAppend

2024-01-30 Thread David Rowley
On Wed, 31 Jan 2024 at 02:23, Alexander Kuzmenkov wrote: > > On Tue, Jan 30, 2024 at 1:20 PM David Rowley wrote: > > You should likely focus on trying to find a test that does not require > > making 2 tables with 10k rows. > > Is 1k smallint OK? It should fit in one pa

Re: Some revises in adding sorting path

2024-01-30 Thread David Rowley
On Wed, 31 Jan 2024 at 00:44, Richard Guo wrote: > This patchset does not aim to introduce anything new; it simply > refactors the existing code. The newly added tests are used to show > that the code that is touched here is not redundant, but rather > essential for generating certain paths. I r

Re: Incorrect cost for MergeAppend

2024-01-30 Thread David Rowley
On Wed, 31 Jan 2024 at 00:06, Alexander Kuzmenkov wrote: > Here is a small patch that reproduces the problem on two tables with > inheritance, and fixes it. I'll add it to the Commitfest. Good catch. It seems to have been broken since MergeAppends were added in 11cad29c9. The code fix looks goo

Re: Some revises in adding sorting path

2024-01-30 Thread David Rowley
On Mon, 29 Jan 2024 at 22:39, Richard Guo wrote: > So in the v3 patch I've brought back the logic that considers > incremental sort on partial paths in gather_grouping_paths(). However, > I failed to compose a test case for this scenario without having to > generate a huge table. So in the v3 pa

Re: Delay Memoize hashtable build until executor run

2024-01-29 Thread David Rowley
On Tue, 30 Jan 2024 at 12:25, David Rowley wrote: > I'll push this change to master only as there don't seem to have been > any complaints. We can reconsider that if someone complains. Pushed. David

Re: Delay Memoize hashtable build until executor run

2024-01-29 Thread David Rowley
On Fri, 26 Jan 2024 at 19:54, David Rowley wrote: > Currently, nodeMemoize.c builds the hashtable for the cache during > executor startup. This is not what is done in hash joins. I think we > should make the two behave the same way. I ran a few benchmarks on this, mostly for archive

Re: Optmize bitmapword macros calc (src/backend/nodes/bitmapset.c)

2024-01-29 Thread David Rowley
On Tue, 30 Jan 2024 at 08:32, Nathan Bossart wrote: > I'm currently +0.1 for this change. I don't see any huge problem with > trimming a few instructions, but I'm dubious there's any measurable impact. > However, a cycle saved is a cycle earned... FWIW, In [1] and subsequent replies, there are s

Re: Streaming read-ready sequential scan code

2024-01-29 Thread David Rowley
On Tue, 30 Jan 2024 at 10:17, Melanie Plageman wrote: > Though logically the performance with 0001 and 0002 should be the same > as master (no new non-inline function calls, no additional looping), > I've done a bit of profiling anyway. I created a large multi-GB table, > read it all into shared b

Re: A performance issue with Memoize

2024-01-26 Thread David Rowley
On Sat, 27 Jan 2024 at 09:41, Tom Lane wrote: > > David Rowley writes: > > I've adjusted the comments to what you mentioned and also leaned out > > the pretty expensive test case to something that'll run much faster > > and pushed the result. > > drongo an

Re: Apply the "LIMIT 1" optimization to partial DISTINCT

2024-01-26 Thread David Rowley
On Fri, 26 Jan 2024 at 20:42, Richard Guo wrote: > > In 5543677ec9 we introduced an optimization that uses Limit instead of > Unique to implement DISTINCT when all the DISTINCT pathkeys have been > marked as redundant. I happened to notice that this optimization was > not applied to partial DISTI

Re: A performance issue with Memoize

2024-01-25 Thread David Rowley
On Fri, 26 Jan 2024 at 19:03, Richard Guo wrote: > At first I wondered if we should assume that the same param expr must > have the same equality operator. If not, we should also check the > operator to tell if the cache key is a duplicate, like > > - if (!list_member(*param_exprs, expr)

Delay Memoize hashtable build until executor run

2024-01-25 Thread David Rowley
Currently, nodeMemoize.c builds the hashtable for the cache during executor startup. This is not what is done in hash joins. I think we should make the two behave the same way. Per [1] and the corresponding discussion leading to that, making a possibly large allocation at executor startup can lea

Re: A performance issue with Memoize

2024-01-25 Thread David Rowley
On Fri, 26 Jan 2024 at 16:51, Tom Lane wrote: > >> However ... it seems like we're not out of the woods yet. Why > >> is Richard's proposed test case still showing > >> + -> Memoize (actual rows=5000 loops=N) > >> + Cache Key: t1.two, t1.two > >> Seems like there is missing

Re: A performance issue with Memoize

2024-01-25 Thread David Rowley
On Fri, 26 Jan 2024 at 07:32, Tom Lane wrote: > > David Rowley writes: > > I'd feel better about doing it your way if Tom could comment on if > > there was a reason he put the function calls that way around in > > 5ebaaa494. > > I'm fairly sure I thought

Re: A performance issue with Memoize

2024-01-24 Thread David Rowley
On Fri, 20 Oct 2023 at 23:40, Richard Guo wrote: > The Memoize runtime stats 'Hits: 0 Misses: 1 Evictions: ' > seems suspicious to me, so I've looked into it a little bit, and found > that the MemoizeState's keyparamids and its outerPlan's chgParam are > always different, and that makes

Re: pgsql: Add better handling of redundant IS [NOT] NULL quals

2024-01-23 Thread David Rowley
On Wed, 24 Jan 2024 at 08:15, Alvaro Herrera wrote: > (Similarly, allowing GROUP BY to ignore columns not in the GROUP BY, > when a UNIQUE constraint exists and all columns are NOT NULL; currently > we allow that for PRIMARY KEY, but if you have the NOT NULL constraint > OIDs to cue the plan inval

Re: Reordering DISTINCT keys to match input path's pathkeys

2024-01-23 Thread David Rowley
On Tue, 23 Jan 2024 at 18:56, Richard Guo wrote: > > Similar to what we did to GROUP BY keys in 0452b461bc, I think we can do > the same to DISTINCT keys, i.e. reordering DISTINCT keys to match input > path's pathkeys, which can help reduce cost by avoiding unnecessary > re-sort, or allowing us to

Re: Removing const-false IS NULL quals and redundant IS NOT NULL quals

2024-01-22 Thread David Rowley
On Tue, 23 Jan 2024 at 00:11, David Rowley wrote: > I've attached v11 which updates the expected results in some newly > added regression tests. I went over this again. I did a little more work adjusting comments and pushed it. Thanks for all your assistance with this, Richard. David

Re: Removing const-false IS NULL quals and redundant IS NOT NULL quals

2024-01-22 Thread David Rowley
other changes. David From 095744f5583ab5446c1cdb75bfd3b40c7ab493d8 Mon Sep 17 00:00:00 2001 From: David Rowley Date: Thu, 7 Dec 2023 22:52:34 +1300 Subject: [PATCH v11] Reduce NullTest quals to constant TRUE or FALSE --- .../postgres_fdw/expected/postgres_fdw.out| 16 +- contrib/postgres_fdw

Re: Removing const-false IS NULL quals and redundant IS NOT NULL quals

2024-01-22 Thread David Rowley
On Thu, 28 Dec 2023 at 00:38, Andy Fan wrote: > I also want to add notnullattnums for the UniqueKey stuff as well, by > comparing your implementation with mine, I found you didn't consider > the NOT NULL generated by filter. After apply your patch: > > create table a(a int); > explain (costs off)

Re: Prefetch the next tuple's memory during seqscans

2024-01-22 Thread David Rowley
On Sat, 20 Jan 2024 at 16:35, vignesh C wrote: > I'm seeing that there has been no activity in this thread for more > than 6 months, I'm planning to close this in the current commitfest > unless someone is planning to take it forward. Thanks for the reminder about this. Since the heapgettup/hea

Re: Strange Bitmapset manipulation in DiscreteKnapsack()

2024-01-18 Thread David Rowley
On Thu, 18 Jan 2024 at 16:24, David Rowley wrote: > On Thu, 18 Jan 2024 at 15:22, Richard Guo wrote: > > Do you think we can use 'memcpy(a, b, BITMAPSET_SIZE(b->nwords))' > > directly in the new bms_replace_members() instead of copying the > > bitmapwords o

Re: Strange Bitmapset manipulation in DiscreteKnapsack()

2024-01-18 Thread David Rowley
On Fri, 19 Jan 2024 at 01:07, Andy Fan wrote: > I find the following code in DiscreteKnapsack is weird. > > > for (i = 0; i <= max_weight; ++i) > { > values[i] = 0; > > ** memory allocation here, and the num_items bit is removed later ** > > sets[i]

Re: Strange Bitmapset manipulation in DiscreteKnapsack()

2024-01-17 Thread David Rowley
On Thu, 18 Jan 2024 at 14:58, Andy Fan wrote: > I want to know if "user just want to zero out the flags in bitmapset > but keeping the memory allocation" is a valid requirement. Commit > 00b41463c makes it is hard IIUC. Looking at your patch, I see: +/* + * does this break commit 00b41463c21615f

Re: Strange Bitmapset manipulation in DiscreteKnapsack()

2024-01-17 Thread David Rowley
Thanks for having a look at this again. On Thu, 18 Jan 2024 at 15:22, Richard Guo wrote: > Do you think we can use 'memcpy(a, b, BITMAPSET_SIZE(b->nwords))' > directly in the new bms_replace_members() instead of copying the > bitmapwords one by one, like: > > - i = 0; > - do > - { > -

Re: Strange Bitmapset manipulation in DiscreteKnapsack()

2024-01-17 Thread David Rowley
On Tue, 16 Jan 2024 at 16:32, David Rowley wrote: > > While working on [1], I noticed some strange code in > DiscreteKnapsack() which seems to be aiming to copy the Bitmapset. > > It's not that obvious at a casual glance, but: > > sets[j] = bms_del_members(sets[j], sets[

Re: New Window Function: ROW_NUMBER_DESC() OVER() ?

2024-01-16 Thread David Rowley
On Wed, 17 Jan 2024 at 15:28, Maiquel Grassi wrote: > On Wed, 17 Jan 2024 at 14:36, David Rowley wrote: > > If you were looking for something to optimize in this rough area, then > > perhaps adding some kind of "Backward WindowAgg" node (by overloading > > the

Re: New Window Function: ROW_NUMBER_DESC() OVER() ?

2024-01-16 Thread David Rowley
On Wed, 17 Jan 2024 at 08:51, Michał Kłeczek wrote: > I think that’s the main issue: what (semantically) does row_number() mean in > that case? You could equally well generate random numbers? Well, not quite random as at least row_number() would ensure the number is unique in the result set. The

Re: Revise the Asserts added to bimapset manipulation functions

2024-01-16 Thread David Rowley
On Tue, 16 Jan 2024 at 21:00, Richard Guo wrote: > Thank you so much for all the work you have put into making this patch > perfect. I reviewed through the v3 patch and I do not have further > comments. I think it's in good shape now. Thanks for looking again. I pushed the patch after removing

Strange Bitmapset manipulation in DiscreteKnapsack()

2024-01-15 Thread David Rowley
While working on [1], I noticed some strange code in DiscreteKnapsack() which seems to be aiming to copy the Bitmapset. It's not that obvious at a casual glance, but: sets[j] = bms_del_members(sets[j], sets[j]); this is aiming to zero all the words in the set by passing the same set in both para

Re: Revise the Asserts added to bimapset manipulation functions

2024-01-15 Thread David Rowley
If we use > bms_free(), the Assert would fail. > > It seems to me that this scenario can only occur within the bitmapset > manipulation functions. Outside of bitmapset.c, I think it should be > quite safe to call bms_free() with this Assert. So I think it should > not have problem to

Re: Reducing output size of nodeToString

2024-01-02 Thread David Rowley
On Thu, 14 Dec 2023 at 19:21, Matthias van de Meent wrote: > > On Thu, 7 Dec 2023 at 13:09, David Rowley wrote: > > We could also easily serialize plans to binary format for copying to > > parallel workers rather than converting them to a text-based > > serialized format

Re: Revise the Asserts added to bimapset manipulation functions

2023-12-30 Thread David Rowley
On Fri, 29 Dec 2023 at 23:38, Richard Guo wrote: > After applying this process to the first RestrictInfo, the bitmapset now > becomes {t2, t3}. Consequently, the second RestrictInfo also perceives > {t2, t3} as its required_relids. As a result, when attempting to remove > it from the joininfo li

Re: Revise the Asserts added to bimapset manipulation functions

2023-12-29 Thread David Rowley
On Fri, 29 Dec 2023 at 21:07, Richard Guo wrote: > It seems to me that the former scenario also makes sense in some cases. > For instance, let's say there are two pointers in two structs, s1->p and > s2->p, both referencing the same bitmapset. If we modify the bitmapset > via s1->p (even if no re

Re: Revise the Asserts added to bimapset manipulation functions

2023-12-28 Thread David Rowley
On Thu, 28 Dec 2023 at 23:21, David Rowley wrote: > then instead of having to do: > > #ifdef REALLOCATE_BITMAPSETS > a = (Bitmapset *) palloc(BITMAPSET_SIZE(tmp->nwords)); > memcpy(a, tmp, BITMAPSET_SIZE(tmp->nwords)); > pfree(tmp); > #endif > > all over the p

Re: Revise the Asserts added to bimapset manipulation functions

2023-12-28 Thread David Rowley
On Wed, 27 Dec 2023 at 22:30, Richard Guo wrote: > The Asserts added to bitmapset.c by commits 71a3e8c43b and 7d58f2342b > contain some duplicates, such as in bms_difference, bms_is_subset, > bms_subset_compare, bms_int_members and bms_join. For instance, I'm just learning of these changes now.

Re: Postgres db Update to Version 15

2023-12-10 Thread David Rowley
On Sun, 10 Dec 2023 at 04:10, Ritthaler, Axel wrote: > The Root Cause of this behavior, as aligned with AWS RDS Support, has been a > new feature-set coding (parallel_feature_query) with Postgres Version 15, > that shows a different behavior due to related parameter > (max_parallel_workers_per_

Re: Memory consumed by paths during partitionwise join planning

2023-12-07 Thread David Rowley
On Fri, 8 Dec 2023 at 18:02, Ashutosh Bapat wrote: > given path. E.g. we have three path chains as follows > 1. joinpath_1->joinpath_2->seqpath_1, > 2. joinpath_3->joinpath_4->seqpath_1, > 3. joinpath_5->joinpath_2->seqpath_1 > > Please note that this is not full path tree/forest. It is difficult

Re: micro-optimizing json.c

2023-12-07 Thread David Rowley
On Fri, 8 Dec 2023 at 12:13, Nathan Bossart wrote: > Here's a patch that removes a couple of strlen() calls that showed up > prominently in perf for a COPY TO (FORMAT json) on 110M integers. On my > laptop, I see a 20% speedup from ~23.6s to ~18.9s for this test. + seplen = use_line_feeds ? size

Re: Memory consumed by paths during partitionwise join planning

2023-12-07 Thread David Rowley
On Fri, 1 Dec 2023 at 19:59, Ashutosh Bapat wrote: > I am fine to work on this further if the community thinks it is > acceptable. It seems from your point of view the worth of this work is > as follows > a. memory savings - not worth it > b. getting rid of !IsA(old_path, IndexPath) - worth it > c

Re: Reducing output size of nodeToString

2023-12-07 Thread David Rowley
On Thu, 7 Dec 2023 at 10:09, Matthias van de Meent wrote: > PFA a patch that reduces the output size of nodeToString by 50%+ in > most cases (measured on pg_rewrite), which on my system reduces the > total size of pg_rewrite by 33% to 472KiB. This does keep the textual > pg_node_tree format alive,

Removing const-false IS NULL quals and redundant IS NOT NULL quals

2023-12-07 Thread David Rowley
ov 29, 2023 at 8:48 AM David Rowley wrote: >> On looking deeper, I see you're overwriting the rinfo_serial of the >> const-false RestrictInfo with the one from the original RestrictInfo. >> If that's the correct thing to do then the following comment would >> nee

Re: Something seems weird inside tts_virtual_copyslot()

2023-12-07 Thread David Rowley
On Fri, 1 Dec 2023 at 14:30, David Rowley wrote: > > On Fri, 1 Dec 2023 at 13:14, Andres Freund wrote: > > So I think adding an assert to ExecCopySlot(), perhaps with a comment saying > > that the restriction could be lifted with a bit of work, would be fine. > > How abou

Re: catalog access with reset GUCs during parallel worker startup

2023-12-06 Thread David Rowley
On Thu, 10 Feb 2022 at 13:33, Andres Freund wrote: > Postmaster's GUC state will already be loaded via read_nondefault_variables(), > much earlier in startup. Well before bgworkers get control and before > transaction environment is up. > > Setting a watchpoint on enableFsync, in a parallel worker

Re: catalog access with reset GUCs during parallel worker startup

2023-12-06 Thread David Rowley
On Wed, 23 Feb 2022 at 15:51, Andres Freund wrote: > The tests fail: > +ERROR: invalid value for parameter "session_authorization": "andres" > +CONTEXT: while setting parameter "session_authorization" to "andres" > +parallel worker > +while scanning relation "public.pvactst" > > but that's easil

Re: Dumped SQL failed to execute with ERROR "GROUP BY position -1 is not in select list"

2023-12-03 Thread David Rowley
On Mon, 4 Dec 2023 at 02:38, Haotian Chen wrote: > Yes, I updated my patch and just used oid numbers 558 and 1751 stand for > int4um and numeric_uminus. Maybe we could define a macro for them, > but seems unnecessary. The thing to do here is modify pg_operator.dat and give both of these operators

Re: Something seems weird inside tts_virtual_copyslot()

2023-11-30 Thread David Rowley
On Fri, 1 Dec 2023 at 13:14, Andres Freund wrote: > So I think adding an assert to ExecCopySlot(), perhaps with a comment saying > that the restriction could be lifted with a bit of work, would be fine. Thanks for looking at this again. How about the attached? I wrote the comment you mentioned

Re: Memory consumed by paths during partitionwise join planning

2023-11-28 Thread David Rowley
On Fri, 28 Jul 2023 at 02:06, Ashutosh Bapat wrote: > Table 1: Join between unpartitioned tables > Number of tables | without patch | with patch | % reduction | > being joined ||| | > -- >

Re: Properly pathify the union planner

2023-11-28 Thread David Rowley
On Fri, 24 Nov 2023 at 18:43, Ashutosh Bapat wrote: > > On Fri, Nov 24, 2023 at 3:59 AM David Rowley wrote: > > For now, as a temporary fix, I've just #ifdef'd out the code in > > add_path() that's pfreeing the old path. I have drafted a patch that > > re

Re: Don't use bms_membership in places where it's not needed

2023-11-27 Thread David Rowley
On Tue, 28 Nov 2023 at 11:21, Andres Freund wrote: > Hm, does this ever matter from a performance POV? The current code does look > simpler to read to me. If the overhead is relevant, I'd instead just move the > code into a static inline? I didn't particularly find the code in examine_variable()

Re: Don't use bms_membership in places where it's not needed

2023-11-27 Thread David Rowley
On Fri, 24 Nov 2023 at 19:54, Richard Guo wrote: > +1 to the idea. > > I think you have a typo in distribute_restrictinfo_to_rels. We should > remove the call of bms_singleton_member and use relid instead. Thanks for reviewing. I've now pushed this. David

Don't use bms_membership in places where it's not needed

2023-11-23 Thread David Rowley
While working on the patch in [1], I noticed that ever since 00b41463c, it's now suboptimal to do the following: switch (bms_membership(relids)) { case BMS_EMPTY_SET: /* handle empty set */ break; case BMS_SINGLETON: /* call bms_singleton_member() and handle singleton

Re: Properly pathify the union planner

2023-11-23 Thread David Rowley
On Thu, 2 Nov 2023 at 12:42, David Rowley wrote: > One part that still needs work is the EquivalanceClass building. > Because we only build the final targetlist for the Append after > planning all the append child queries, I ended up having to populate > the EquivalanceClasses ba

Re: ensure, not insure

2023-11-09 Thread David Rowley
On Thu, 9 Nov 2023 at 14:22, Michael Paquier wrote: > > On Wed, Nov 08, 2023 at 08:31:28PM +1300, David Rowley wrote: > > Those all look fine to me too. > > +1. I've pushed this. I backpatched due to the typo in the fsync GUC description. I'd have only pushed t

Re: ensure, not insure

2023-11-07 Thread David Rowley
On Wed, 8 Nov 2023 at 19:56, Peter Smith wrote: > gettext_noop("The server will use the fsync() system call in several > places to make " > "sure that updates are physically written to disk. This insures " > "that a database cluster will recover to a consistent state after " > "an operating

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

2023-11-06 Thread David Rowley
On Thu, 2 Nov 2023 at 22:42, Amit Kapila wrote: > The other two look good to me. Thanks for looking. I spent some time trying to see if the performance changes much with either of these cases. For the XLogWalRcvProcessMsg() I was unable to measure any difference even when replaying inserts into

Re: Compiling warnings on old GCC

2023-11-05 Thread David Rowley
On Mon, 6 Nov 2023 at 19:14, Richard Guo wrote: > I came across the following compiling warnings on GCC (Red Hat 4.8.5-44) > 4.8.5 with 'CFLAGS=-Og' > I wonder if this is worth fixing, maybe by a trivial patch like > attached. There's some relevant discussion in https://postgr.es/m/flat/20220602

Re: Something seems weird inside tts_virtual_copyslot()

2023-11-05 Thread David Rowley
On Sat, 4 Nov 2023 at 15:15, Andres Freund wrote: > > On 2023-11-01 11:35:50 +1300, David Rowley wrote: > > I changed the Assert in tts_virtual_copyslot() to check the natts > > match in each of the slots and all of the regression tests still pass, > > so it seems we hav

Re: Pre-proposal: unicode normalized text

2023-11-03 Thread David Rowley
On Sat, 4 Nov 2023 at 10:57, Thomas Munro wrote: > > On Fri, Nov 3, 2023 at 9:01 PM David Rowley wrote: > > On Fri, 3 Nov 2023 at 20:49, Jeff Davis wrote: > > > I think I just need to add unicode_category.c to @pgcommonallfiles in > > > Mkvcbuild.pm. I'll do

Re: Pre-proposal: unicode normalized text

2023-11-03 Thread David Rowley
On Fri, 3 Nov 2023 at 20:49, Jeff Davis wrote: > > On Fri, 2023-11-03 at 10:51 +1300, Thomas Munro wrote: > > bowerbird and hammerkop didn't like commit a02b37fc. They're still > > using the old 3rd build system that is not tested by CI. It's due > > for > > removal in the 17 cycle IIUC but in t

Re: Possible typo in nodeAgg.c

2023-11-02 Thread David Rowley
On Fri, 3 Nov 2023 at 13:49, Bruce Momjian wrote: > > On Fri, Oct 16, 2020 at 09:03:52AM +, Hou, Zhijie wrote: > > /* > >* Don't set the limit below 3/4 of hash_mem. In that case, we are at > > the > >* minimum number of partitions, so we aren't going to dramatically >

Re: Why is DEFAULT_FDW_TUPLE_COST so insanely low?

2023-11-02 Thread David Rowley
On Fri, 3 Nov 2023 at 01:02, Richard Guo wrote: > It seems that the test is still not stable on 32-bit machines even after > 4b14e18714. I see the following plan diff on cfbot [1]. I recreated that locally this time. Seems there's still flexibility to push or not push down the sort and the cost

Re: Why is DEFAULT_FDW_TUPLE_COST so insanely low?

2023-11-02 Thread David Rowley
On Thu, 2 Nov 2023 at 18:39, Michael Paquier wrote: > The CI has been telling me that the plans of the tests introduced by > this patch are not that stable when building with 32b. See: > diff -U3 /tmp/cirrus-ci-build/contrib/postgres_fdw/expected/postgres_fdw.out > /tmp/cirrus-ci-build/build-32/

Re: Why is DEFAULT_FDW_TUPLE_COST so insanely low?

2023-11-01 Thread David Rowley
On Tue, 31 Oct 2023 at 11:16, David Rowley wrote: > I'd be happy if anyone else would like to try the same experiment to > see if there's some other value of DEFAULT_FDW_TUPLE_COST that might > suit better. No takers on the additional testing so I've pushed the patch

Properly pathify the union planner

2023-11-01 Thread David Rowley
The upper planner was pathified many years ago now. That was a large chunk of work and because of that, the union planner was not properly pathified in that effort. A small note was left in recurse_set_operations() to mention about future work. You can see this lack of pathification in make_unio

Something seems weird inside tts_virtual_copyslot()

2023-10-31 Thread David Rowley
Looking at the Assert inside tts_virtual_copyslot(), it does: Assert(srcdesc->natts <= dstslot->tts_tupleDescriptor->natts); So, that seems to indicate that it's ok for the src slot to have fewer attributes than the destination. The code then calls tts_virtual_clear(dstslot), then slot_getallatt

Re: small erreport bug over partitioned table pgrowlocks module

2023-10-30 Thread David Rowley
On Tue, 31 Oct 2023 at 13:18, David Rowley wrote: > Here's a patch that puts the relam check last. I've pushed that patch. David

Re: small erreport bug over partitioned table pgrowlocks module

2023-10-30 Thread David Rowley
On Tue, 31 Oct 2023 at 13:00, jian he wrote: > BEGIN; > CREATE TABLE fk_parted_pk (a int PRIMARY KEY) PARTITION BY LIST (a); > SELECT * FROM pgrowlocks('fk_parted_pk'); > ERROR: only heap AM is supported > > error should be the following part: > if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TAB

Re: Why is DEFAULT_FDW_TUPLE_COST so insanely low?

2023-10-30 Thread David Rowley
On Tue, 31 Oct 2023 at 03:01, Bruce Momjian wrote: > I think you just go and change it. Your number is better than what we > have, and if someone wants to suggest a better number, we can change it > later. I did some more experimentation on the actual costs of getting a tuple from a foreign serv

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

2023-10-30 Thread David Rowley
On Mon, 30 Oct 2023 at 23:48, Amit Kapila wrote: > > On Fri, Oct 27, 2023 at 3:23 AM David Rowley wrote: > > * parallel.c in HandleParallelMessages(): > > * applyparallelworker.c in HandleParallelApplyMessages(): > > Both the above calls are used to handle ERROR/NOTICE

Re: Why is DEFAULT_FDW_TUPLE_COST so insanely low?

2023-10-29 Thread David Rowley
On Sun, 29 Oct 2023 at 12:45, Bruce Momjian wrote: > Has anything been done about this issue? Nothing has been done. I was hoping to get the attention of a few people who have dealt more with postgres_fdw in the past. I've attached a patch with adjusts DEFAULT_FDW_TUPLE_COST and sets it to 0.2.

Re: Use virtual tuple slot for Unique node

2023-10-29 Thread David Rowley
On Fri, 27 Oct 2023 at 22:05, Ashutosh Bapat wrote: > > On Fri, Oct 27, 2023 at 8:48 AM David Rowley wrote: > > I was uncertain if the old behaviour of when srcslot contains fewer > > attributes than dstslot was intended or not. What happens there is > > that we&#x

Re: Use virtual tuple slot for Unique node

2023-10-26 Thread David Rowley
On Wed, 25 Oct 2023 at 22:48, Ashutosh Bapat wrote: > We may save the size of data in VirtualTupleTableSlot, thus avoiding > the first loop. I assume that when allocating > VirtualTupleTableSlot->data, we always know what size we are > allocating so it should be just a matter of saving it in > Vir

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

2023-10-26 Thread David Rowley
On Thu, 26 Oct 2023 at 17:00, David Rowley wrote: > Thanks for looking at this again. I fixed up each of those and pushed > the result, mentioning the incompatibility in the commit message. > > Now that that's done, I've attached a patch which makes use of the new &g

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

2023-10-25 Thread David Rowley
On Thu, 26 Oct 2023 at 08:43, Tom Lane wrote: > I think that we can make that assumption starting with v17. > Back-patching it would be hazardous perhaps; but if there's some > function out there that depends on NUL termination, testing should > expose it before too long. Wouldn't hurt to mention

Re: Add null termination to string received in parallel apply worker

2023-10-25 Thread David Rowley
On Wed, 11 Oct 2023 at 19:54, Zhijie Hou (Fujitsu) wrote: > The parallel apply worker didn't add null termination to the string received > from the leader apply worker via the shared memory queue. This action doesn't > bring bugs as it's binary data but violates the rule established in > StringIn

Re: Document aggregate functions better w.r.t. ORDER BY

2023-10-25 Thread David Rowley
On Thu, 26 Oct 2023 at 13:10, David G. Johnston wrote: > Question: Do you know whether we for certain always sort ascending here to > compute the unique values or whether if, say, there is an index on the column > in descending order (or ascending and traversed backwards) that the data > within

Re: Add new for_each macros for iterating over a List that do not require ListCell pointer

2023-10-24 Thread David Rowley
On Wed, 25 Oct 2023 at 06:00, Jelte Fennema wrote: > Attached is a fixed version. With foreach(), we commonly do "if (lc == NULL)" at the end of loops as a way of checking if we did "break" to terminate the loop early. Doing the equivalent with the new macros won't be safe as the list element's v

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

2023-10-24 Thread David Rowley
On Tue, 17 Oct 2023 at 20:39, David Rowley wrote: > > On Mon, 16 Oct 2023 at 05:56, Tom Lane wrote: > > I guess the next question is whether we want to stop here or > > try to relax the requirement about NUL-termination. I'd be inclined > > to call that a separa

Re: Use virtual tuple slot for Unique node

2023-10-23 Thread David Rowley
AIN ANALYZE has additional timing going on and we may end up not de-toasting toasted Datums. > On Thu, Oct 19, 2023 at 4:26 PM David Rowley wrote: > > We can see that Q2 and Q3 become a bit slower. This makes sense as > > tts_virtual_materialize() is quite a bit more complex than > &g

Re: Use virtual tuple slot for Unique node

2023-10-19 Thread David Rowley
On Thu, 19 Oct 2023 at 22:29, David Rowley wrote: > It's hard to imagine why there would be a slowdown as this query uses > a TTSOpsMinimalTuple slot type in the patch and the unpatched version. I shrunk down your table sizes to 10k rows instead of 1 million rows to reduce the CPU cac

Re: Use virtual tuple slot for Unique node

2023-10-19 Thread David Rowley
On Thu, 12 Oct 2023 at 23:06, Ashutosh Bapat wrote: > Q7 select distinct a,b from (select string_agg(left(a, 100), ', ') > over (order by a rows 2 preceding) a, b from t_text) q > HEAD: 16070.62 ms > patched: 16182.16 ms Did you time the SELECT or EXPLAIN ANALYZE? With SELECT, I'm unable to recr

<    1   2   3   4   5   6   7   8   9   10   >