Re: [HACKERS] Removing LEFT JOINs in more cases

From: David Rowley <david(dot)rowley(at)2ndquadrant(dot)com>
To: Ashutosh Bapat <ashutosh(dot)bapat(at)enterprisedb(dot)com>
Cc: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [HACKERS] Removing LEFT JOINs in more cases
Date: 2017-11-30 06:50:54
Message-ID: CAKJS1f9JTwRRLQcFCUDUGSncUnFvS1se6j==UG1q6xSMns3KCg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox
Thread:
Lists: pgsql-hackers

Thanks for looking over this and my apologies for the delay in my
response. I've been on leave and out of range of the internet for most
of that time.

On 23 November 2017 at 02:30, Ashutosh Bapat
<ashutosh(dot)bapat(at)enterprisedb(dot)com> wrote:
>
> @@ -597,15 +615,25 @@ rel_supports_distinctness(PlannerInfo *root,
> RelOptInfo *rel)
> + if (root->parse->distinctClause != NIL)
> + return true;
> +
> + if (root->parse->groupClause != NIL && !root->parse->hasAggs)
> + return true;
> +
>
> The other callers of rel_supports_distinctness() are looking for distinctness
> of the given relation, whereas the code change here are applicable to any
> relation, not just the given relation. I find that confusing. Looking at the
> way we are calling rel_supports_distinctness() in join_is_removable() this
> change looks inevitable, but still if we could reduce the confusion, that will
> be good. Also if we could avoid duplication of comment about unique index, that
> will be good.

When I put this together I'd considered leaving
rel_supports_distinctness() alone since the other uses of the function
can't make use of the new checks. Since you mention this, then I think
it's likely better just to go with that and just special case the code
in join_is_removable() to check for rel_supports_distinctness() and
the DISTINCT/GROUP BY clauses too. This will mean less false positives
for usages such as in innerrel_is_unique() which would end up causing
a bit of extra work before possibly failing a bit later on. I've made
changes in the attached to do things this way.

> May be you want to add a testcase with DISTINCT ON.

Good idea. I've also added a test case for this, and also one to
ensure non-RTE_RELATION relations can be removed too.

Updated patch attached.

Thanks again for the review.

--
David Rowley http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

Attachment Content-Type Size
remove_left_join_distinct_2017-11-30.patch application/octet-stream 7.6 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Alexey Chernyshov 2017-11-30 08:13:07 ASCII Null control character validation
Previous Message Michael Paquier 2017-11-30 05:56:07 Re: Commit fest 2017-11