In get_memoize_path() we have a theory about avoiding memoize node if
there are volatile functions in the inner rel's target/restrict list.
/*
* We can't use a memoize node if there are volatile functions in the
* inner rel's target list or restrict list. A cache hit could reduce the
* num
On Mon, Aug 7, 2023 at 6:19 PM David Rowley wrote:
> On Fri, 4 Aug 2023 at 22:26, Richard Guo wrote:
> > explain (costs off)
> > select * from t t1 left join lateral
> > (select t1.a as t1a, t2.a as t2a from t t2) s
> > on t1.a = s.t2a + random();
>
On Mon, Aug 7, 2023 at 9:29 PM Ashutosh Bapat
wrote:
> Thanks. The patch looks good overall. I like it because of its potential
> to reduce memory consumption in reparameterization. That's cake on top of
> cream :)
>
Thanks for reviewing this patch!
> Some comments here.
>
> + {
> + FLAT_COPY_
On Tue, Aug 8, 2023 at 4:40 PM Etsuro Fujita
wrote:
> I modified the code a bit further to use an if-test to avoid a useless
> function call, and added/tweaked comments and docs further. Attached
> is a new version of the patch. I am planning to commit this, if there
> are no objections.
+1 t
On Thu, Jul 27, 2023 at 10:06 PM Ashutosh Bapat <
ashutosh.bapat@gmail.com> wrote:
> 0002 - WIP patch to avoid repeated translations of RestrictInfo.
> The WIP patch avoids repeated translations by tracking the child for
> which a RestrictInfo is translated and reusing the same translation
> e
On Tue, Aug 8, 2023 at 7:34 PM Ashutosh Bapat
wrote:
> On Tue, Aug 8, 2023 at 12:44 PM Richard Guo
> wrote:
> > This is not an existing bug. Our current code (without this patch)
> > would always call create_nestloop_path() after the reparameterization
> > work has been
On Tue, Apr 18, 2023 at 5:07 PM Richard Guo wrote:
> In thread [1] which discussed 'Right Anti Join', Tom once mentioned
> 'Right Semi Join'. After a preliminary investigation I think it is
> beneficial and can be implemented with very short change. With 'Righ
On Thu, Aug 10, 2023 at 8:57 AM Julien Rouhaud wrote:
> On Wed, Aug 09, 2023 at 10:29:56AM -0500, Tristan Partin wrote:
> > On Wed Aug 9, 2023 at 10:02 AM CDT, Peter Eisentraut wrote:
> > >
> > > This patch has apparently upset one buildfarm member with a very old
> > > compiler:
> > >
> https://
As explained in the comments for generate_orderedappend_paths, we don't
currently support parameterized MergeAppend paths, and it doesn't seem
like going to change anytime soon. Based on that, we could simplify
create_merge_append_path a bit, such as set param_info to NULL directly
rather than ca
On Tue, Aug 15, 2023 at 7:50 PM Etsuro Fujita
wrote:
> So we should have modified the second one as well? Attached is a
> small patch for that.
Agreed, nice catch! +1 to the patch.
Thanks
Richard
If we have a hash join with an Append node on the outer side, something
like
Hash Join
Hash Cond: (pt.a = t.a)
-> Append
-> Seq Scan on pt_p1 pt_1
-> Seq Scan on pt_p2 pt_2
-> Seq Scan on pt_p3 pt_3
-> Hash
-> Seq Scan on t
We can actually prune
On Sun, Aug 20, 2023 at 11:48 PM Tom Lane wrote:
> I looked over the v3 patch. I think it's going in generally
> the right direction, but there is a huge oversight:
> path_is_reparameterizable_by_child does not come close to modeling
> the success/failure conditions of reparameterize_path_by_chi
On Tue, Aug 22, 2023 at 4:48 AM Tom Lane wrote:
> I spent some time reviewing the v4 patch. I noted that
> path_is_reparameterizable_by_child still wasn't modeling the pass/fail
> behavior of reparameterize_path_by_child very well, because that
> function checks this at every recursion level:
>
On Mon, Aug 21, 2023 at 8:34 PM Andy Fan wrote:
> This feature looks good, but is it possible to know if we can prune
> any subnodes before we pay the extra effort (building the Hash
> table, for each row... stuff)?
>
It might be possible if we take the partition prunning into
consideration when
On Tue, Aug 22, 2023 at 2:38 PM David Rowley wrote:
> With Hash Join, it seems to me that the pruning must take place for
> every row that makes it into the hash table. There will be maybe
> cases where the unioned set of partitions simply yields every
> partition and all the work results in no
On Tue, Aug 22, 2023 at 10:39 PM Tom Lane wrote:
> Richard Guo writes:
> > I'm wondering if we can instead adjust the 'inner_req_outer' in
> > create_nestloop_path before we perform the check to work around this
> > issue for the back branches, something l
On Thu, Aug 24, 2023 at 1:44 AM Tom Lane wrote:
> Richard Guo writes:
> > If we go with the "tablesample scans can't be reparameterized" approach
> > in the back branches, I'm a little concerned that what if we find more
> > cases in the futrue where w
On Tue, Aug 22, 2023 at 2:38 PM David Rowley wrote:
> It would be good to see some performance comparisons of the worst case
> to see how much overhead the pruning code adds to Hash Join. It may
> well be that we need to consider two Hash Join paths, one with and one
> without run-time pruning.
On Fri, Aug 25, 2023 at 11:03 AM David Rowley wrote:
> On Thu, 24 Aug 2023 at 21:27, Richard Guo wrote:
> > I think we need to solve this problem first before we can
> > make this new partition pruning mechanism some useful in practice, but
> > how? Some thoughts current
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
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
https://www.postgresql.org/messag
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;
>
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
+++ b/src/backend/opt
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
> t
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
Here is another rebase over 3af7040985. Nothing else has changed.
Thanks
Richard
v4-0001-Retiring-is_pushed_down.patch
Description: Binary data
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, 300]'::in
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
> >
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.
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"
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
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
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 creat
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 +
+++
/tmp/cirrus-ci-build/build/testrun/regress-running/regress/
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 tha
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
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
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 ha
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.
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 th
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 foo
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 src/test/regr
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 buildfa
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 REA
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
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
> > do
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 q
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
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&
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
> imp
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
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 belo
On Fri, May 24, 2024 at 9:08 PM Richard Guo wrote:
> On the basis of the parser infrastructure fixup, 0002 patch adds the
> nullingrel bit that references the grouping RTE to the grouping
> expressions.
I found a bug in the v6 patch. The following query would trigger the
On Tue, Aug 2, 2022 at 3:51 AM Tom Lane wrote:
> Here's a rebase up to HEAD, mostly to placate the cfbot.
> I accounted for d8e34fa7a (s/all_baserels/all_query_rels/
> in those places) and made one tiny bug-fix change.
> Nothing substantive as yet.
When we add required PlaceHolderVars to a join
On Wed, Aug 17, 2022 at 4:57 AM Tom Lane wrote:
> On further thought, it seems better to maintain the index array
> from the start, allowing complete replacement of the linear list
> searches. We can add a separate bool flag to denote frozen-ness.
+1 for 0001 patch. Now we process plain Vars a
On Sun, Aug 21, 2022 at 6:52 AM Tom Lane wrote:
> What I'm thinking we should do about this, once we detect that
> this identity is applicable, is to generate *both* forms of Pbc,
> either adding or removing the varnullingrels bits depending on
> which form we got from the parser. Then, when we
On Wed, Aug 24, 2022 at 6:49 PM Alvaro Herrera
wrote:
> On 2022-Aug-24, mahendrakar s wrote:
>
> > Hi,
> > Can we have a parameter to control the recursion depth in these cases to
> > avoid crashes?
>
> We already have one (max_stack_depth). The problem is lack of calling
> the control function
On Wed, Aug 24, 2022 at 7:12 PM Richard Guo wrote:
>
> On Wed, Aug 24, 2022 at 6:49 PM Alvaro Herrera
> wrote:
>
>> On 2022-Aug-24, mahendrakar s wrote:
>>
>> > Hi,
>> > Can we have a parameter to control the recursion depth in these cases to
>
On Thu, Aug 25, 2022 at 5:18 AM Tom Lane wrote:
> Richard Guo writes:
> > Do we need to also
> > generate two SpecialJoinInfos for the B/C join in the first order, with
> > and without the A/B join in its min_lefthand?
>
> No, the SpecialJoinInfos would stay as they
On Fri, Aug 19, 2022 at 2:45 AM Tom Lane wrote:
> Here's a rebase up to HEAD, mainly to get the cfbot back in sync
> as to what's the live patch.
Noticed another different behavior from previous. When we try to reduce
JOIN_LEFT to JOIN_ANTI, we want to know if the join's own quals are
strict fo
On Mon, Aug 29, 2022 at 2:30 PM Richard Guo wrote:
>
> On Fri, Aug 19, 2022 at 2:45 AM Tom Lane wrote:
>
>> Here's a rebase up to HEAD, mainly to get the cfbot back in sync
>> as to what's the live patch.
>
>
> Noticed another different behavior from pre
On Wed, Aug 31, 2022 at 6:57 AM Tom Lane wrote:
> I wrote:
> > The upstream recommendation, which seems pretty sane to me, is to
> > simply reject any string exceeding some threshold length as not
> > possibly being a word. Apparently it's common to use thresholds
> > as small as 64 bytes, but i
On Thu, Sep 1, 2022 at 2:18 PM Etsuro Fujita
wrote:
> While working on something else, I noticed that commit 487e9861d added
> a new field to struct Trigger, but failed to update $SUBJECT to match.
> Attached is a small patch for that.
+1. Good catch.
Thanks
Richard
On Wed, Aug 31, 2022 at 2:35 PM Andrey Lepikhov
wrote:
> Before flattening procedure we just look through the quals of subquery,
> pull to the upper level OpExpr's containing variables from the upper
> relation and replace their positions in the quals with true expression.
> Further, the flatteni
On Fri, Sep 2, 2022 at 2:52 AM Nathan Bossart
wrote:
> I'm hoping to spend a bit more time looking for additional applications of
> the pg_lfind*() suite of functions (and anywhere else where SIMD might be
> useful, really). If you have any ideas in mind, I'm all ears.
+1 for the proposal. I d
On Fri, Sep 2, 2022 at 7:09 PM Andrey Lepikhov
wrote:
> On 9/1/22 17:24, Richard Guo wrote:
> > On Wed, Aug 31, 2022 at 2:35 PM Andrey Lepikhov
> > mailto:a.lepik...@postgrespro.ru>> wrote:
> > Before flattening procedure we just look through the quals of
>
On Tue, Sep 6, 2022 at 1:18 AM Tom Lane wrote:
> Zhang Mingli writes:
> > Macro exec_subplan_get_plan is not used anymore.
> > Attach a patch to remove it.
>
> Hm, I wonder why it's not used anymore. Maybe we no longer need
> that list at all? If we do, should use of the macro be
> re-introduc
Hi hackers,
While wandering around the codes of reducing outer joins, I noticed that
when determining which base rels/Vars are forced nonnullable by given
clause, we don't take SubPlan into consideration. Does anyone happen to
know what is the concern behind that?
IMO, for SubPlans of type ALL/AN
AFAICS, the Vars forced nonnullable by given clause are only used to
check if we can reduce JOIN_LEFT to JOIN_ANTI, and it is checking the
join's own quals there. It seems to me we do not need to pass down
nonnullable_vars by upper quals to the children of a join.
Attached is a patch to remove the
On Tue, Sep 13, 2022 at 11:35 AM Zhang Mingli wrote:
> Param isSlice was once used to identity targetTypeId for
> transformAssignmentIndirection.
>
> In commit c7aba7c14e, the evaluation was pushed down to
> transformContainerSubscripts.
>
> No need to keep isSlice around transformAssignmentSubsc
On Wed, Sep 14, 2022 at 10:46 AM Kyotaro Horiguchi
wrote:
> I found a small typo in a comment in pgbench.c of 15/master.
>
> - * Return the number fo failed transactions.
> + * Return the number of failed transactions.
>
> While at it, I found "* lot fo unnecessary work." in pg13's
> procsignal.c
On Wed, Jul 27, 2022 at 5:10 PM Xing Guo wrote:
> The bounded heap sorting status flag is set twice in sort_bounded_heap()
> and tuplesort_performsort(). This patch helps remove one of them.
>
Revisiting this patch I think maybe it's better to remove the setting of
Tuplesort status from tuplesor
On Tue, Sep 6, 2022 at 12:39 AM Zhang Mingli wrote:
> Macro exec_subplan_get_plan is not used anymore.
> Attach a patch to remove it.
>
How about add it to the CF to not lose track of it?
Thanks
Richard
I have a question about displaying NestLoopParam. In the plan below,
# explain (costs off)
select * from a, lateral (select sum(i) as i from b where exists (select
sum(c.i) from c where c.j = a.j and c.i = b.i) ) ss where a.i = ss.i;
QUERY PLAN
On Fri, Sep 16, 2022 at 5:59 PM Richard Guo wrote:
> I'm not saying this is a bug, but just curious why param 0 cannot be
> displayed as the referenced expression. And I find the reason is that in
> function find_param_referent(), we have the 'in_same_plan_level' flag
On Wed, Sep 21, 2022 at 1:40 PM Michael Paquier wrote:
> In short, switching those code paths to use the linear search routines
> looks like a good thing in the long-term, so I would like to apply
> this patch. If you have any comments or objections, please feel
> free.
Yeah, I agree that the
On Thu, May 18, 2023 at 3:34 AM Tom Lane wrote:
> After some poking at it I hit on what seems like a really simple
> solution: we should be checking syn_righthand not min_righthand
> to see whether a Var should be considered nullable by a given OJ.
> Maybe that's still not quite right, but it see
On Thu, May 18, 2023 at 3:42 AM Tom Lane wrote:
> ... BTW, something I'd considered in an earlier attempt at fixing this
> was to change clause_is_computable_at's API to pass the clause's
> RestrictInfo not just the clause_relids, along the lines of
>
> @@ -541,9 +547,10 @@ extract_actual_join_cl
On Fri, May 19, 2023 at 12:33 AM Tom Lane wrote:
> Bleah. The other solution I'd been poking at involved adding an
> extra check for clone clauses, as attached (note this requires
> 8a2523ff3). This survives your example, but I wonder if it might
> reject all the clones in some cases. It seems
On Fri, May 19, 2023 at 1:57 PM Michael Paquier wrote:
> Thanks for the test case, issue reproduced here on HEAD and not v15.
> This causes an assertion failure here:
> #4 0x55a6f8faa776 in ExceptionalCondition
> (conditionName=0x55a6f915ac60 "bms_equal(rel->relids,
> root->all_query_rels)",
I came across $subject on master and here is the query I'm running.
create table t (a int unique, b int);
explain (costs off)
select 1 from t t1
left join t t2 on true
inner join t t3 on true
left join t t4 on t2.a = t4.a and t2.a = t3.a;
ERROR: no relation entry for relid 6
I looked
On Tue, May 23, 2023 at 2:38 PM Richard Guo wrote:
> I came across $subject on master and here is the query I'm running.
>
> create table t (a int unique, b int);
>
> explain (costs off)
> select 1 from t t1
> left join t t2 on true
>inner join t t3 on true
On Wed, May 24, 2023 at 2:48 AM Tom Lane wrote:
> Richard Guo writes:
> > Considering that clone clauses should always be outer-join clauses not
> > filter clauses, I'm wondering if we can add an additional check for that
> > in RINFO_IS_PUSHED_DOWN, som
Testing with SQLancer reports a wrong results issue on master and I
reduced it to the repro query below.
create table t (a int, b int);
explain (costs off)
select * from t t1 left join
(t t2 left join t t3 full join t t4 on false on false)
left join t t5 on t2.a = t5.a
on t2.b = 1;
On Thu, May 25, 2023 at 5:28 AM Tom Lane wrote:
> I tried this and it seems to work all right: it fixes the example
> you showed while not causing any new failures. (Doesn't address
> the broken join-removal logic you showed in the other thread,
> though.)
>
> While at it, I also changed make_re
On Fri, May 26, 2023 at 6:06 AM Tom Lane wrote:
> 1. The test case you give upthread only needs to ignore commute_below_l,
> that is it still passes without the lines
>
> +join_plus_commute = bms_add_members(join_plus_commute,
> +
> removable_sjinfo->commute_above_r);
>
> Based on what decons
On Sat, May 27, 2023 at 12:16 AM Tom Lane wrote:
> Ah. I realized that we could make the problem testable by adding
> assertions that a joinclause we're not removing doesn't contain
> any surviving references to the target rel or join. That turns
> out to fire (without the bug fix) in half a do
On Tue, May 30, 2023 at 10:28 AM Richard Guo wrote:
> I haven't thought through how to fix it, but I suspect that we may need
> to do more checking before we decide to remove PHVs in
> remove_rel_from_query.
>
Hmm, maybe we can additionally check if the PHV needs to be evaluat
On Tue, May 30, 2023 at 10:48 PM Markus Winand
wrote:
> I found an error similar to others before ([1]) that is still persists as
> of head right now (0bcb3ca3b9).
>
> CREATE TABLE t (
> n INTEGER
> );
>
> SELECT *
> FROM (VALUES (1)) t(c)
> LEFT JOIN t ljl1 ON true
> LEFT JOIN LATE
On Wed, May 31, 2023 at 10:47 AM Richard Guo wrote:
> On Tue, May 30, 2023 at 10:48 PM Markus Winand
> wrote:
>
>> I found an error similar to others before ([1]) that is still persists as
>> of head right now (0bcb3ca3b9).
>>
>> CREATE TABLE t (
>&g
On Wed, May 31, 2023 at 1:27 AM James Coleman wrote:
> This looks good to me.
Thanks for the review!
> A few small tweaks suggested to comment wording:
>
> +-- lateral reference for simple Var can escape PlaceHolderVar if the
> +-- referenced rel is under the same lowest nulling outer join
>
In make_outerjoininfo, I think we can additionally check a property
that's needed to apply OJ identity 3: the lower OJ in the RHS cannot be
a member of inner_join_rels because we do not try to commute with any of
lower inner joins.
--- a/src/backend/optimizer/plan/initsplan.c
+++ b/src/backend/opt
On Sun, Jun 4, 2023 at 8:42 PM Ranier Vilela wrote:
> Hi,
>
> Per Coverity.
>
> At function ExtendBufferedRelShared, has a always true test.
> eb.rel was dereferenced one line above, so in
> if (eb.rel) is always true.
>
> I think it's worth removing the test, because Coverity raises dozens of
>
On Fri, Jun 2, 2023 at 11:30 AM Suraj Kharage <
suraj.khar...@enterprisedb.com> wrote:
> +1 for fixing this in the backend code rather than FDW code.
>
> Thanks, Richard, for working on this. The patch looks good to me at
> a glance.
>
Thank you Suraj for the review!
Thanks
Richard
On Fri, Jun 2, 2023 at 8:31 PM Nishant Sharma <
nishant.sha...@enterprisedb.com> wrote:
> *I only had a minor comment on below change:-*
>
>
>
>
>
> *- gating_clauses = get_gating_quals(root, scan_clauses);+ if
> (best_path->pathtype == T_ForeignScan && IS_JOIN_REL(rel))+
> gating_clauses = ge
On Mon, Jun 5, 2023 at 9:19 PM Etsuro Fujita
wrote:
> If the patch is intended for HEAD only, I also think it goes in the
> right direction. But if it is intended for back branches as well, I
> do not think so, because it would cause ABI breakage due to changes
> made to the ForeignPath struct a
On Mon, Jan 23, 2023 at 10:00 PM James Coleman wrote:
> Which this patch we do in fact now see (as expected) rels with
> non-empty lateral_relids showing up in generate_[useful_]gather_paths.
> And the partial paths can now have non-empty required outer rels. I'm
> not able to come up with a plan
On Wed, Jun 7, 2023 at 4:22 AM Tom Lane wrote:
> Alvaro Herrera writes:
> > So, is this done? I see that you made other commits fixing related code
> > several days after this email, but none seems to match the changes you
> > posted in this patch; and also it's not clear to me that there's any
On Thu, Jun 8, 2023 at 7:37 AM David Rowley wrote:
> What the attached patch does is process each WindowClause and removes
> any items from the PARTITION BY clause that are columns or expressions
> relating to redundant PathKeys.
>
> Effectively, this allows the nodeWindowAgg.c code which stops
>
On Fri, Jun 9, 2023 at 5:13 AM Tom Lane wrote:
> Richard Guo writes:
> > Hmm, maybe we can additionally check if the PHV needs to be evaluated
> > above the join. If so it cannot be removed.
> Yeah, that seems to make sense, and it squares with the existing
> comment
501 - 600 of 934 matches
Mail list logo
