Re: Wrong results with grouping sets

2024-05-24 Thread Richard Guo
On the basis of the parser infrastructure fixup, 0002 patch adds the nullingrel bit that references the grouping RTE to the grouping expressions. However, it seems to me that we have to manually remove this nullingrel bit from expressions in various cases where these expressions are logically

Re: Wrong results with grouping sets

2024-05-23 Thread Richard Guo
On Fri, May 17, 2024 at 5:41 PM Richard Guo wrote: > I've spent some more time on this patch, and now it passes all the > regression tests. But I had to hack explain.c and ruleutils.c to make > the varprefix stuff work as it did before, which is not great. > I've realized that I ma

Re: Wrong results with grouping sets

2024-05-17 Thread Richard Guo
On Thu, May 16, 2024 at 5:43 PM Richard Guo wrote: > I have experimented with this approach, and here is the outcome. The > patch fixes Geoff's query, but it's still somewhat messy as I'm not > experienced enough in the parser code. And the patch has not yet > implemented the nu

Re: A wrong comment about search_indexed_tlist_for_var

2024-05-16 Thread Richard Guo
On Wed, May 15, 2024 at 1:07 AM Robert Haas wrote: > On Mon, Dec 4, 2023 at 3:42 AM Richard Guo wrote: > > Then here is a trivial patch to adjust the comment, which should get > > reverted along with 867be9c07. > > Richard, since you're a committer now, maybe you'd lik

Re: Wrong results with grouping sets

2024-05-16 Thread Richard Guo
On Sun, Jan 7, 2024 at 4:59 AM Tom Lane wrote: > I don't think this is going in quite the right direction. We have > many serious problems with grouping sets (latest one today at [1]), > and I don't believe that hacking around EquivalenceClasses is going > to fix them all. > > I think that what

Re: First draft of PG 17 release notes

2024-05-09 Thread Richard Guo
to improve CTE plans by using the sort order of > columns referenced in earlier CTE clauses (Jian Guo) I think you mean a65724dfa. The author should be 'Richard Guo'. And I'm wondering if it is more accurate to state it as "Allow the optimizer to improve plans for the outer query by le

Re: Revert: Remove useless self-joins *and* -DREALLOCATE_BITMAPSETS make server crash, regress test fail.

2024-05-08 Thread Richard Guo
On Thu, May 9, 2024 at 6:40 AM Tom Lane wrote: > David Rowley writes: > > I'm fine with this one as it's the same as what I already mentioned > > earlier. I had imagined doing bms_del_member(bms_copy ... but maybe > > the compiler is able to optimise away the additional store. Likely, it > >

Re: A problem about partitionwise join

2024-05-08 Thread Richard Guo
On Fri, May 3, 2024 at 9:31 PM Robert Haas wrote: > On Fri, May 3, 2024 at 7:47 AM Richard Guo wrote: > > I think one concern regarding performance cost is that the function > > exprs_known_equal() would be called O(N^2) times, where N is the number > > of partition ke

Re: Revert: Remove useless self-joins *and* -DREALLOCATE_BITMAPSETS make server crash, regress test fail.

2024-05-07 Thread Richard Guo
On Tue, May 7, 2024 at 1:46 PM David Rowley wrote: > On Tue, 7 May 2024 at 17:28, Tom Lane wrote: > > What I'm trying to figure out here is whether we have a live bug > > in this area in released branches; and if so, why we've not seen > > reports of that. > > We could check what portions of

Re: Revert: Remove useless self-joins *and* -DREALLOCATE_BITMAPSETS make server crash, regress test fail.

2024-05-07 Thread Richard Guo
On Tue, May 7, 2024 at 1:22 PM David Rowley wrote: > It would be good to get some build farm coverage of this so we don't > have to rely on manual testing. I wonder if it's a good idea to just > define REALLOCATE_BITMAPSETS when #ifdef CLOBBER_FREED_MEMORY... or if > we should ask on the

Re: Revert: Remove useless self-joins *and* -DREALLOCATE_BITMAPSETS make server crash, regress test fail.

2024-05-06 Thread Richard Guo
On Tue, May 7, 2024 at 11:35 AM jian he wrote: > hi, > > SELECT table_name, column_name, is_updatable > FROM information_schema.columns > WHERE table_name LIKE E'r_\\_view%' > ORDER BY table_name, ordinal_position; > > at d1d286d83c0eed695910cb20d970ea9bea2e5001, > this query in

Re: A problem about partitionwise join

2024-05-03 Thread Richard Guo
On Wed, May 1, 2024 at 1:31 AM Robert Haas wrote: > I think it's slightly questionable whether this patch is worthwhile. > The case memorialized in the regression tests, t1.a = t2.a AND t1.a = > t2.b, is a very weird thing to do. The case mentioned in the original > email, foo.k1 = bar.k1 and

Re: Removing unneeded self joins

2024-05-02 Thread Richard Guo
On Thu, May 2, 2024 at 6:08 PM Alexander Korotkov wrote: > On Thu, May 2, 2024 at 12:45 PM Andrei Lepikhov > wrote: > > One question for me is: Do we anticipate other lateral self-references > > except the TABLESAMPLE case? Looking into the extract_lateral_references > > implementation, I see

Re: [PATCH] Fix bug when calling strncmp in check_authmethod_valid

2024-04-29 Thread Richard Guo
On Tue, Apr 30, 2024 at 10:41 AM Jingxian Li wrote: > Attached is a patch that fixes bug when calling strncmp function, in > which case the third argument (authmethod - strchr(authmethod, ' ')) > may be negative, which is not as expected.. Nice catch. I think you're right from a quick glance.

Re: A failure in prepared_xacts test

2024-04-29 Thread Richard Guo
On Tue, Apr 30, 2024 at 6:43 AM Michael Paquier wrote: > I'd be curious about any discussion involving the structure of the > meson tests. +1. I'm kind of worried that the expansion of parallelization could lead to more instances of instability. Alexander mentioned one such case at [1]. I

Re: A failure in prepared_xacts test

2024-04-29 Thread Richard Guo
On Mon, Apr 29, 2024 at 9:45 PM Tom Lane wrote: > Richard Guo writes: > > I noticed that some TAP tests from recovery and subscription would > > select the count from pg_prepared_xacts. I wonder if these tests would > > be affected if there are any prepared transactions on

Re: A failure in prepared_xacts test

2024-04-29 Thread Richard Guo
On Mon, Apr 29, 2024 at 2:58 PM Michael Paquier wrote: > On Mon, Apr 29, 2024 at 01:32:40AM -0400, Tom Lane wrote: > > (BTW, on the same logic, should ecpg's twophase.pgc be using a > > prepared-transaction name that's less generic than "gxid"?) > > I've hesitated a few seconds about that before

Re: A failure in prepared_xacts test

2024-04-29 Thread Richard Guo
On Mon, Apr 29, 2024 at 1:11 PM Tom Lane wrote: > Up to now, we've only worried about whether tests running in parallel > within a single test suite can interact. It's quite scary to think > that the meson setup has expanded the possibility of interactions > to our entire source tree. Maybe

A failure in prepared_xacts test

2024-04-28 Thread Richard Guo
Yesterday I noticed a failure on cirrus-ci for the 'Right Semi Join' patch. The failure can be found at [1], and it looks like: --- /tmp/cirrus-ci-build/src/test/regress/expected/prepared_xacts.out 2024-04-27 00:41:25.831297000 + +++

Re: Short-circuit sort_inner_and_outer if there are no mergejoin clauses

2024-04-26 Thread Richard Guo
On Thu, Apr 25, 2024 at 7:25 PM Ashutosh Bapat wrote: > Quickly looking at the function, the patch would make it more apparent > that the function is a noop when mergeclause_list is empty. > Thanks for looking at this patch. Yes, that's what it does. > I haven't looked closely to see if

Re: Short-circuit sort_inner_and_outer if there are no mergejoin clauses

2024-04-25 Thread Richard Guo
On Wed, Apr 24, 2024 at 5:13 PM Richard Guo wrote: > In sort_inner_and_outer, we create mergejoin join paths by explicitly > sorting both relations on each possible ordering of the available > mergejoin clauses. However, if there are no available mergejoin > clauses, we can skip

Re: Support "Right Semi Join" plan shapes

2024-04-24 Thread Richard Guo
Here is another rebase with a commit message to help review. I also tweaked some comments. Thanks Richard v5-0001-Support-Right-Semi-Join-plan-shapes.patch Description: Binary data

Re: docs: minor typo fix for "lower(anymultirange)"

2024-04-24 Thread Richard Guo
On Thu, Apr 25, 2024 at 8:40 AM Ian Lawrence Barwick wrote: > Hi > > Here: > > > https://www.postgresql.org/docs/current/functions-range.html#MULTIRANGE-FUNCTIONS-TABLE > > the description for "lower(anymultirange)": > > > (NULL if the multirange is empty has no lower bound). > > is missing "or"

Short-circuit sort_inner_and_outer if there are no mergejoin clauses

2024-04-24 Thread Richard Guo
In sort_inner_and_outer, we create mergejoin join paths by explicitly sorting both relations on each possible ordering of the available mergejoin clauses. However, if there are no available mergejoin clauses, we can skip this process entirely. It seems that this is a relatively common scenario.

Re: Assert failure in _bt_preprocess_array_keys

2024-04-22 Thread Richard Guo
On Mon, Apr 22, 2024 at 10:52 AM Peter Geoghegan wrote: > On Sun, Apr 21, 2024 at 10:36 PM Richard Guo > wrote: > > I didn't spend much time digging into it, but I wonder if this Assert is > > sensible. I noticed that before commit 5bf748b86b, the two datatypes > > were

Assert failure in _bt_preprocess_array_keys

2024-04-21 Thread Richard Guo
I came across an assert failure in _bt_preprocess_array_keys regarding the sanity check on the datatype of the array elements. It can be reproduced with the query below. create table t (c int4range); create unique index on t (c); select * from t where c in ('(1, 100]'::int4range, '(50,

Re: Retiring is_pushed_down

2024-04-18 Thread Richard Guo
Here is another rebase over 3af7040985. Nothing else has changed. Thanks Richard v4-0001-Retiring-is_pushed_down.patch Description: Binary data

Re: [PATCH] Replace magic constant 3 with NUM_MERGE_MATCH_KINDS

2024-04-16 Thread Richard Guo
On Tue, Apr 16, 2024 at 5:48 PM Aleksander Alekseev < aleksan...@timescale.com> wrote: > Oversight of 0294df2f1f84 [1]. > > [1]: > https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=0294df2f1f84 +1. I think this change improves the code quality. I searched for other arrays indexed

Re: Typos in the code and README

2024-04-16 Thread Richard Guo
On Mon, Apr 15, 2024 at 8:26 PM Daniel Gustafsson wrote: > Thanks. Collecting all the ones submitted here, as well as a few submitted > off-list by Alexander, the patch is now a 3-part patchset of cleanups: > > 0001 contains the typos and duplicate words fixups, 0002 fixes a parameter > with >

Re: Incorrect handling of IS [NOT] NULL quals on inheritance parents

2024-04-12 Thread Richard Guo
On Fri, Apr 12, 2024 at 4:19 PM David Rowley wrote: > After further work on the comments, I pushed the result. > > Thanks for working on this. Thanks for pushing! BTW, I noticed a typo in the comment of add_base_clause_to_rel. --- a/src/backend/optimizer/plan/initsplan.c +++

Re: BitmapHeapScan streaming read user and prelim refactoring

2024-04-11 Thread Richard Guo
On Sun, Apr 7, 2024 at 10:42 PM Tomas Vondra wrote: > create table t (a int, b int) with (fillfactor=10); > insert into t select mod((i/22),2), (i/22) from generate_series(0,1000) > S(i); > create index on t(a); > vacuum analyze t; > > set enable_indexonlyscan = off; > set enable_seqscan = off;

Re: Add SPLIT PARTITION/MERGE PARTITIONS commands

2024-04-11 Thread Richard Guo
On Thu, Apr 11, 2024 at 1:22 AM Dmitry Koval wrote: > 2) v1-0002-Fixes-for-english-text.patch - fixes for English text > (comments, error messages etc.). FWIW, I also proposed a patch earlier that fixes error messages and comments in the split partition code at

Re: Incorrect handling of IS [NOT] NULL quals on inheritance parents

2024-04-11 Thread Richard Guo
On Thu, Apr 11, 2024 at 10:23 AM David Rowley wrote: > On Wed, 10 Apr 2024 at 19:12, Richard Guo wrote: > > And I think recording NOT NULL columns for traditional inheritance > > parents can be error-prone for some future optimization where we look > > at an inheritance pa

Revise some error messages in split partition code

2024-04-10 Thread Richard Guo
I noticed some error messages in the split partition code that are not up to par. Such as: "new partitions not have value %s but split partition has" how about we revise it to: "new partitions do not have value %s but split partition does" Another one is: "any partition in the list should be

Re: Incorrect handling of IS [NOT] NULL quals on inheritance parents

2024-04-10 Thread Richard Guo
On Wed, Apr 10, 2024 at 1:13 PM David Rowley wrote: > I looked at the patch and I don't think it's a good idea to skip > recording NOT NULL constraints to fix based on the fact that it > happens to result in this particular optimisation working correctly. > It seems that just makes this work in

Incorrect handling of IS [NOT] NULL quals on inheritance parents

2024-04-09 Thread Richard Guo
In b262ad440e we introduced an optimization that drops IS NOT NULL quals on a NOT NULL column, and reduces IS NULL quals on a NOT NULL column to constant-FALSE. I happened to notice that this is not working correctly for traditional inheritance parents. Traditional inheritance parents might have

Remove excessive trailing semicolons

2024-03-29 Thread Richard Guo
Noticed some newly introduced excessive trailing semicolons: $ git grep -E ";;$" -- *.c *.h src/include/lib/radixtree.h:int deletepos = slot - n4->children;; src/test/modules/test_tidstore/test_tidstore.c: BlockNumber prevblkno = 0;; Here is a trivial patch to

Re: To what extent should tests rely on VACUUM ANALYZE?

2024-03-29 Thread Richard Guo
On Thu, Mar 28, 2024 at 11:00 PM Alexander Lakhin wrote: > When running multiple 027_stream_regress.pl test instances in parallel > (and with aggressive autovacuum) on a rather slow machine, I encountered > test failures due to the subselect test instability just as the following > failures on

Re: To what extent should tests rely on VACUUM ANALYZE?

2024-03-29 Thread Richard Guo
On Fri, Mar 29, 2024 at 1:33 AM Tom Lane wrote: > Tomas Vondra writes: > > Yeah. I think it's good to design the data/queries in such a way that > > the behavior does not flip due to minor noise like in this case. > > +1 Agreed. The query in problem is: -- we can pull up the sublink into

Re: Properly pathify the union planner

2024-03-27 Thread Richard Guo
On Wed, Mar 27, 2024 at 6:34 PM David Rowley wrote: > On Wed, 27 Mar 2024 at 22:47, David Rowley wrote: > > I did wonder when first working on this patch if subquery_planner() > > should grow an extra parameter, or maybe consolidate some existing > > ones by passing some struct that provides

Re: Remove some redundant set_cheapest() calls

2024-03-27 Thread Richard Guo
On Wed, Mar 27, 2024 at 10:59 PM Tom Lane wrote: > Richard Guo writes: > > On Wed, Mar 27, 2024 at 4:06 AM Tom Lane wrote: > >> I'm less convinced about changing this. I'd rather keep it consistent > >> with mark_dummy_rel. > > > Hm, I wonder if we should

Re: Remove some redundant set_cheapest() calls

2024-03-27 Thread Richard Guo
On Wed, Mar 27, 2024 at 4:06 AM Tom Lane wrote: > Richard Guo writes: > > I happened to notice that the set_cheapest() calls in functions > > set_namedtuplestore_pathlist() and set_result_pathlist() are redundant, > > as we've centralized the set_cheapest() call

Re: Propagate pathkeys from CTEs up to the outer query

2024-03-27 Thread Richard Guo
On Wed, Mar 27, 2024 at 1:20 AM Tom Lane wrote: > Richard Guo writes: > > I agree with your points. Previously I was thinking that CTEs were the > > only scenario where we needed to remember the best path and only > > required the best path's pathkeys. However,

Re: Properly pathify the union planner

2024-03-26 Thread Richard Guo
On Wed, Mar 27, 2024 at 6:23 AM David Rowley wrote: > Because this field is set, it plans the CTE thinking it's a UNION > child and breaks when it can't find a SortGroupClause for the CTE's > target list item. Right. The problem here is that we mistakenly think that the CTE query is a

Remove some redundant set_cheapest() calls

2024-03-26 Thread Richard Guo
I happened to notice that the set_cheapest() calls in functions set_namedtuplestore_pathlist() and set_result_pathlist() are redundant, as we've centralized the set_cheapest() calls in set_rel_pathlist(). Attached is a trivial patch to remove these calls. BTW, I suspect that the set_cheapest()

Re: Propagate pathkeys from CTEs up to the outer query

2024-03-26 Thread Richard Guo
On Tue, Mar 26, 2024 at 1:39 AM Tom Lane wrote: > I got around to looking at this finally. I was a bit surprised by > your choice of data structure. You made a per-CTE-item cte_paths > list paralleling cte_plan_ids, but what I had had in mind was a > per-subplan list of paths paralleling

Re: Memory consumed by child SpecialJoinInfo in partitionwise join planning

2024-03-25 Thread Richard Guo
On Mon, Mar 25, 2024 at 5:17 PM Amit Langote wrote: > I've pushed this now. > > When updating the commit message, I realized that you had made the > right call to divide the the changes around not translating the dummy > SpecialJoinInfos into a separate patch. So, I pushed it like that. Sorry

Re: Properly pathify the union planner

2024-03-25 Thread Richard Guo
On Mon, Mar 25, 2024 at 9:44 AM David Rowley wrote: > It seems ok that > the ec_indexes are not set for the top-level set RelOptInfo as > get_eclass_for_sort_expr() does not make use of ec_indexes while > searching for an existing EquivalenceClass. Maybe we should fix this > varno == 0 hack and

Re: A problem about partitionwise join

2024-03-24 Thread Richard Guo
On Tue, Mar 19, 2024 at 3:40 PM Ashutosh Bapat wrote: > On Tue, Mar 19, 2024 at 8:18 AM Richard Guo > wrote: > >> On Thu, Mar 7, 2024 at 7:13 PM Ashutosh Bapat < >> ashutosh.bapat@gmail.com> wrote: >> >>> Approach >>> >>>

Re: Add Index-level REINDEX with multiple jobs

2024-03-24 Thread Richard Guo
On Mon, Mar 25, 2024 at 10:07 AM Tom Lane wrote: > Alexander Korotkov writes: > > Here goes the revised patch. I'm going to push this if there are no > objections. > > Quite a lot of the buildfarm is complaining about this: > > reindexdb.c: In function 'reindex_one_database': >

Re: Oversight in reparameterize_path_by_child leading to executor crash

2024-03-21 Thread Richard Guo
On Wed, Mar 20, 2024 at 2:57 AM Tom Lane wrote: > Richard Guo writes: > > Here is the patch for HEAD. I simply re-posted v10. Nothing has > > changed. > > I got back to this finally, and pushed it with some minor cosmetic > adjustments. Thanks for pushing! Thanks Richard

Re: Support run-time partition pruning for hash join

2024-03-19 Thread Richard Guo
On Tue, Jan 30, 2024 at 10:33 AM Richard Guo wrote: > Attached is an updated patch. Nothing else has changed. > Here is another rebase over master so it applies again. Nothing else has changed. Thanks Richard v7-0001-Support-run-time-partition-pruning-for-hash-join.patch Descr

Re: A problem about partitionwise join

2024-03-18 Thread Richard Guo
(Sorry it takes me some time to get back to this thread.) On Thu, Mar 7, 2024 at 7:13 PM Ashutosh Bapat wrote: > I did a deeper review of the patch. Here are some comments > Thank you for the review! > Approach > > The equijoin condition between partition keys doesn't appear in the

Re: Check lateral references within PHVs for memoize cache keys

2024-03-18 Thread Richard Guo
On Fri, Feb 2, 2024 at 5:18 PM Richard Guo wrote: > The v4 patch does not apply any more. I've rebased it on master. > Nothing else has changed. > Here is another rebase over master so it applies again. I also added a commit message to help review. Nothing else has changed. Thank

Re: Regarding the order of the header file includes

2024-03-17 Thread Richard Guo
On Wed, Mar 13, 2024 at 10:07 PM Peter Eisentraut wrote: > On 12.03.24 12:47, Richard Guo wrote: > > > > On Wed, Mar 6, 2024 at 5:32 PM Richard Guo > <mailto:guofengli...@gmail.com>> wrote: > > > > Please note that this patch only addresses th

Re: Regarding the order of the header file includes

2024-03-12 Thread Richard Guo
On Wed, Mar 6, 2024 at 5:32 PM Richard Guo wrote: > Please note that this patch only addresses the order of header file > includes in backend modules (and might not be thorough). It is possible > that other modules may have a similar issue, but I have not evaluated > them yet. >

Re: Regarding the order of the header file includes

2024-03-12 Thread Richard Guo
On Fri, Mar 8, 2024 at 6:58 PM Bharath Rupireddy < bharath.rupireddyforpostg...@gmail.com> wrote: > On Thu, Mar 7, 2024 at 12:39 PM Richard Guo > wrote: > > > > While rebasing one of my patches I noticed that the header file includes > > in relnode.c are not sorted

Re: Properly pathify the union planner

2024-03-11 Thread Richard Guo
On Fri, Mar 8, 2024 at 11:31 AM David Rowley wrote: > On Fri, 8 Mar 2024 at 00:39, Richard Guo wrote: > > I would like to have another look, but it might take several days. > > Would that be too late? > > Please look. Several days is fine. I'd like to start looking on Mon

Re: Invalid query generated by postgres_fdw with UNION ALL and ORDER BY

2024-03-08 Thread Richard Guo
On Fri, Mar 8, 2024 at 10:13 AM David Rowley wrote: > The fix could also be to use deparseConst() in appendOrderByClause() > and have that handle Const EquivalenceMember instead. I'd rather just > skip them. To me, that seems less risky than ensuring deparseConst() > handles all Const types

Re: Properly pathify the union planner

2024-03-07 Thread Richard Guo
On Thu, Mar 7, 2024 at 7:16 PM David Rowley wrote: > On Thu, 15 Feb 2024 at 17:30, David Rowley wrote: > > > > 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

Re: Get rid of the excess semicolon in planner.c

2024-03-06 Thread Richard Guo
On Wed, Mar 6, 2024 at 6:00 AM David Rowley wrote: > On Wed, 6 Mar 2024 at 00:43, Richard Guo wrote: > > > > I think this is a typo introduced in 0452b461b. > > > > +root->processed_groupClause = list_copy(parse->groupClause);; > > "git grep -E

Re: Regarding the order of the header file includes

2024-03-06 Thread Richard Guo
On Wed, Mar 6, 2024 at 6:25 PM Bharath Rupireddy < bharath.rupireddyforpostg...@gmail.com> wrote: > On Wed, Mar 6, 2024 at 3:02 PM Richard Guo wrote: > > > > I think we generally follow the rule that we include 'postgres.h' or > > 'postgres_fe.h' first, foll

Regarding the order of the header file includes

2024-03-06 Thread Richard Guo
I think we generally follow the rule that we include 'postgres.h' or 'postgres_fe.h' first, followed by system header files, and then postgres header files ordered in ASCII value. I noticed that in some C files we fail to follow this rule strictly. Attached is a patch to fix this. Back in 2019,

Get rid of the excess semicolon in planner.c

2024-03-05 Thread Richard Guo
I think this is a typo introduced in 0452b461b. +root->processed_groupClause = list_copy(parse->groupClause);; The extra empty statement is harmless in most times, but I still think it would be better to get rid of it. Attached is a trivial patch to do that. Thanks Richard

Re: Refactoring backend fork+exec code

2024-03-05 Thread Richard Guo
On Mon, Mar 4, 2024 at 1:40 AM Heikki Linnakangas wrote: > On 22/02/2024 02:37, Heikki Linnakangas wrote: > > Here's another patch version that does that. Yeah, I agree it's nicer in > > the end. > > > > I'm pretty happy with this now. I'll read through these patches myself > > again after

Re: Support "Right Semi Join" plan shapes

2024-03-04 Thread Richard Guo
On Tue, Jan 30, 2024 at 2:51 PM Alena Rybakina wrote: > I have reviewed your patch and I think it is better to add an Assert for > JOIN_RIGHT_SEMI to the ExecMergeJoin and ExecNestLoop functions to > prevent the use of RIGHT_SEMI for these types of connections (NestedLoop > and MergeJoin).

Re: Support "Right Semi Join" plan shapes

2024-03-04 Thread Richard Guo
On Mon, Mar 4, 2024 at 10:33 AM wenhui qiu wrote: > HI Richard > Now it is starting the last commitfest for v17, can you respond to > Alena Rybakina points? > Thanks for reminding. Will do that soon. Thanks Richard

Re: "type with xxxx does not exist" when doing ExecMemoize()

2024-02-26 Thread Richard Guo
On Mon, Feb 26, 2024 at 3:54 PM Andrei Lepikhov wrote: > On 26/2/2024 12:44, Tender Wang wrote: > > Make sense. I found MemoizeState already has a MemoryContext, so I used > it. > > I update the patch. > This approach is better for me. In the next version of this patch, I > included a test case.

Re: POC: GROUP BY optimization

2024-02-21 Thread Richard Guo
On Thu, Feb 22, 2024 at 12:18 PM Andrei Lepikhov wrote: > On 22/2/2024 09:09, Richard Guo wrote: > > I looked through the v2 patch and have two comments. > > > > * The test case under "Check we don't pick aggregate path key instead of > > grouping path key" d

Re: POC: GROUP BY optimization

2024-02-21 Thread Richard Guo
On Wed, Feb 21, 2024 at 6:20 PM Alexander Korotkov wrote: > Hi, Richard! > > > What do you think about the revisions for the test cases? > > I've rebased your patch upthread. Did some minor beautifications. > > > * The table 'btg' is inserted with 1 tuples, which seems a bit > > expensive

Re: A problem about partitionwise join

2024-02-21 Thread Richard Guo
On Tue, Aug 2, 2022 at 4:24 AM Jacob Champion wrote: > Once you think you've built up some community support and the patchset > is ready for review, you (or any interested party) can resurrect the > patch entry by visiting > > https://commitfest.postgresql.org/38/2266/ > > and changing the

Re: POC: GROUP BY optimization

2024-02-21 Thread Richard Guo
On Fri, Feb 2, 2024 at 12:40 PM Andrei Lepikhov wrote: > On 2/2/2024 11:06, Richard Guo wrote: > > > > On Fri, Feb 2, 2024 at 11:32 AM Richard Guo > <mailto:guofengli...@gmail.com>> wrote: > > > > On Fri, Feb 2, 2024 at 10:02 AM Tom Lane &

Re: Removing unneeded self joins

2024-02-20 Thread Richard Guo
On Mon, Feb 19, 2024 at 1:24 PM Andrei Lepikhov wrote: > On 18/2/2024 23:18, Alexander Korotkov wrote: > > On Sun, Feb 18, 2024 at 5:04 PM Alexander Korotkov > wrote: > >> On Sun, Feb 18, 2024 at 3:00 PM Alexander Lakhin > wrote: > >>> Please look at the following query which fails with an

Re: Properly pathify the union planner

2024-02-06 Thread Richard Guo
On Fri, Nov 24, 2023 at 6:29 AM David Rowley wrote: > I've attached the updated patch. This one is probably ready for > someone to test out. There will be more work to do, however. I just started reviewing this patch and haven't looked through all the details yet. Here are some feedbacks

cfbot does not list patches in 'Current commitfest'

2024-02-04 Thread Richard Guo
... and the patches in CF #47 (currently open) are listed in 'Next commitfest'. I guess we need to manually create a "next" commitfest? Thanks Richard

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

2024-02-04 Thread Richard Guo
cfbot reminds that this patch does not apply any more. So I've rebased it on master, and also adjusted the test cases a bit. Thanks Richard v2-0001-Reordering-DISTINCT-keys-to-match-input-path-s-pathkeys.patch Description: Binary data

Re: An improvement on parallel DISTINCT

2024-02-04 Thread Richard Guo
On Fri, Feb 2, 2024 at 7:36 PM David Rowley wrote: > 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 > + * requires that the input rel has a valid reltarget. > + */ > +

Re: An improvement on parallel DISTINCT

2024-02-04 Thread Richard Guo
On Fri, Feb 2, 2024 at 6:39 PM David Rowley wrote: > So the gains increase with more parallel workers due to pushing more > work to the worker. Amdahl's law approves of this. > > I'll push the patch shortly. Thanks for the detailed testing and pushing the patch! Thanks Richard

Re: Check lateral references within PHVs for memoize cache keys

2024-02-02 Thread Richard Guo
On Mon, Dec 25, 2023 at 3:01 PM Richard Guo wrote: > On Thu, Jul 13, 2023 at 3:12 PM Richard Guo > wrote: > >> So I'm wondering if it'd be better that we move all this logic of >> computing additional lateral references within PHVs to get_memoize_path, >> whe

Re: Commitfest 2024-01 is now closed

2024-02-01 Thread Richard Guo
On Fri, Feb 2, 2024 at 2:41 PM Michael Paquier wrote: > On Fri, Feb 02, 2024 at 11:56:36AM +0530, Amit Kapila wrote: > > On Fri, Feb 2, 2024 at 10:58 AM Tom Lane wrote: > >> Thanks for all the work you did running it! CFM is typically a > >> thankless exercise in being a nag, but I thought you

Re: An improvement on parallel DISTINCT

2024-02-01 Thread Richard Guo
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 planner work > without any performance gains. Hmm, with the query below, I can see that the new plan is

Re: POC: GROUP BY optimization

2024-02-01 Thread Richard Guo
On Fri, Feb 2, 2024 at 11:32 AM Richard Guo wrote: > On Fri, Feb 2, 2024 at 10:02 AM Tom Lane wrote: > >> One of the test cases added by this commit has not been very >> stable in the buildfarm. Latest example is here: >> >> >> https://buildfarm.postgresql.o

Re: POC: GROUP BY optimization

2024-02-01 Thread Richard Guo
On Fri, Feb 2, 2024 at 10:02 AM Tom Lane wrote: > Alexander Korotkov writes: > > I'm going to push this if there are no objections. > > One of the test cases added by this commit has not been very > stable in the buildfarm. Latest example is here: > > >

Re: Oversight in reparameterize_path_by_child leading to executor crash

2024-02-01 Thread Richard Guo
On Fri, Feb 2, 2024 at 1:45 AM Tom Lane wrote: > I pushed v12 (with some cosmetic adjustments) into the back branches, > since we're getting close to the February release freeze. We still > need to deal with the larger fix for HEAD. Please re-post that > one so that the cfbot knows which is

Re: set_cheapest without checking pathlist

2024-02-01 Thread Richard Guo
On Thu, Feb 1, 2024 at 5:03 PM David Rowley wrote: > 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_

Re: A performance issue with Memoize

2024-02-01 Thread Richard Guo
On Thu, Feb 1, 2024 at 3:43 PM Anthonin Bonnefoy < anthonin.bonne...@datadoghq.com> wrote: > Hi, > > I've seen a similar issue with the following query (tested on the current > head): > > EXPLAIN ANALYZE SELECT * FROM tenk1 t1 > LEFT JOIN LATERAL (SELECT t1.two, tenk2.hundred, tenk2.two FROM

Re: set_cheapest without checking pathlist

2024-01-31 Thread Richard Guo
On Thu, Feb 1, 2024 at 11:37 AM David Rowley wrote: > 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 alway

Re: set_cheapest without checking pathlist

2024-01-31 Thread Richard Guo
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 incremental sort and the > cheapest partial path happens to

Re: Oversight in reparameterize_path_by_child leading to executor crash

2024-01-31 Thread Richard Guo
On Wed, Jan 31, 2024 at 11:21 PM Tom Lane wrote: > Richard Guo writes: > > On Wed, Jan 31, 2024 at 5:12 AM Tom Lane wrote: > >> * Why is it okay to just use pull_varnos on a tablesample expression, > >> when contain_references_to() does something different?

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

2024-01-30 Thread Richard Guo
On Wed, Jan 31, 2024 at 12:26 PM David Rowley wrote: > 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

Re: Some revises in adding sorting path

2024-01-30 Thread Richard Guo
On Wed, Jan 31, 2024 at 5:13 AM David Rowley wrote: > 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

Re: Oversight in reparameterize_path_by_child leading to executor crash

2024-01-30 Thread Richard Guo
On Wed, Jan 31, 2024 at 5:12 AM Tom Lane wrote: > Richard Guo writes: > > On Wed, Jan 17, 2024 at 5:01 PM Richard Guo > wrote: > >> Sure, here it is: > >> v10-0001-Avoid-reparameterizing-Paths-when-it-s-not-suitable.patch > > > I forgot to mention th

Re: Some revises in adding sorting path

2024-01-30 Thread Richard Guo
On Tue, Jan 30, 2024 at 7:00 PM David Rowley wrote: > 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 t

Re: Support run-time partition pruning for hash join

2024-01-29 Thread Richard Guo
On Sat, Jan 27, 2024 at 11:29 AM vignesh C wrote: > CFBot shows that the patch does not apply anymore as in [1]: > > Please post an updated version for the same. Attached is an updated patch. Nothing else has changed. Thanks Richard

Re: Retiring is_pushed_down

2024-01-29 Thread Richard Guo
On Sun, Jan 21, 2024 at 8:37 PM vignesh C wrote: > I'm seeing that there has been no activity in this thread for nearly 6 > months, I'm planning to close this in the current commitfest unless > someone is planning to take it forward. It can be opened again when > there is more interest. I'm

Re: Some revises in adding sorting path

2024-01-29 Thread Richard Guo
On Mon, Jul 17, 2023 at 4:55 PM Richard Guo wrote: > But I did not find a query that makes an incremental sort in this case. > After trying for a while it seems to me that we do not need to consider > incremental sort in this case, because for a partial path of a grouped > or parti

Re: Wrong rows estimations with joins of CTEs slows queries by more than factor 500

2024-01-28 Thread Richard Guo
On Mon, Jan 29, 2024 at 11:20 AM vignesh C wrote: > On Mon, 29 Jan 2024 at 08:01, Richard Guo wrote: > > On Sat, Jan 27, 2024 at 10:08 AM vignesh C wrote: > >> I have changed the status of the commitfest entry to "Committed" as I > >> noticed the patch has

Propagate pathkeys from CTEs up to the outer query

2024-01-28 Thread Richard Guo
In [1] we've reached a conclusion that for a MATERIALIZED CTE it's okay to 'allow our statistics or guesses for the sub-query to subsequently influence what the upper planner does'. Commit f7816aec23 exposes column statistics to the upper planner. In the light of that, here is a patch that

Re: Wrong rows estimations with joins of CTEs slows queries by more than factor 500

2024-01-28 Thread Richard Guo
On Sat, Jan 27, 2024 at 10:08 AM vignesh C wrote: > On Mon, 8 Jan 2024 at 22:21, Tom Lane wrote: > > > > Richard Guo writes: > > > On Sun, Jan 7, 2024 at 6:41 AM Tom Lane wrote: > > >> Thanks for the report! I guess we need something like the attached.

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

2024-01-26 Thread Richard Guo
On Tue, Jan 23, 2024 at 5:03 PM David Rowley wrote: > I've not caught up on the specifics of 0452b461b, but I just wanted to > highlight that there was some work done in [1] in this area. It seems > Ankit didn't ever add that to a CF, so that might explain why it's > been lost. > > Anyway, just

  1   2   3   4   5   6   7   >