Postgres Pro
Downloads
Home   >   mailing lists

Re: Getting sorted data from foreign server for merge join - Mailing list pgsql-hackers

From Ashutosh Bapat
Subject Re: Getting sorted data from foreign server for merge join
Date
Msg-id CAFjFpRcST8jSziVtRd+BTeBfjy29F4T-Oz4iZEkkaoO0vTLSpQ@mail.gmail.com
Whole thread Raw
In response to Re: Getting sorted data from foreign server for merge join  (Robert Haas <robertmhaas@gmail.com>)
Responses Re: Getting sorted data from foreign server for merge join
List pgsql-hackers
Tree view
Hi,
In get_useful_ecs_for_relation(), while checking whether to use left or right argument of a mergejoinable operator, the arguments to bms_is_subset() are passed in reverse order. bms_is_subset() checks whether the first argument in subset of the second, but in this function the subset to be checked is passed as the second argument. Because of this following query when run in contrib_regression database after "make installcheck" in contrib/postgres_fdw trips assertion Assert(bms_is_subset(relids, restrictinfo->left_ec->ec_relids));

EXPLAIN (COSTS false, VERBOSE)
    SELECT t1."C 1" FROM "S 1"."T 1" t1 left join ft1 t2 join ft2 t3 on (t2.c1 = t3.c1) on (t3.c1 = t1."C 1") OFFSET 100 LIMIT 10;

PFA patch to fix it.

Reason why it was not caught earlier: this code is excercised when expressions in left/right join clauses are considered. For mergejoinable clauses, the left and right side are not merged into a single EC but appear as separate ECs. All tests in the postgres_fdw.sql that excercised this code involved only two relations, thus ec_relids and relids had only a single member and bms_is_subset() returned true. But in the above query the left/right EC has relids of t2 and t3, which caused bms_is_subset() to return false and thus trip the assertion.

I have added above query and the output to the tests. The output of EXPLAIN shows that ORDER BY clause is pushed down for ft2 but not ft1. This is because ft2 has use_remote_estimate true and ft1 has that false. So, we push down ORDER BY corresponding merge join for ft2 but not for ft1.

On Wed, Dec 23, 2015 at 12:24 AM, Robert Haas <robertmhaas@gmail.com> wrote:
On Mon, Dec 21, 2015 at 6:34 AM, Ashutosh Bapat
<ashutosh.bapat@enterprisedb.com> wrote:
>> I went over this patch in some detail today and did a lot of cosmetic
>> cleanup.  The results are attached.  I'm fairly happy with this
>> version, but let me know what you think.  Of course, feedback from
>> others is more than welcome also.
>>
>
> Attached patch with some cosmetic changes (listed here for your quick
> reference)
> 1. , was replaced with ; in comment "inner join, expressions in the " at one
> place, which is correct, but missed other place.
> 2. The comment "First, consider whether any each active EC is potentially"
> should use either "any" or "each". I have reworded it as "First, consider
> whether any of the active ECs is potentially ...". Or we can use "First,
> find all of the active ECs which are potentially ....".
> 3. "having the remote side due the sort generally won't be any worse ..." -
> instead of "due" we should use "do"?
> 4. Added static prototype of function get_useful_ecs_for_relation().
> 5. The comment "Extract unique EC for query, if any, so we don't consider it
> again." is too crisp. Phrase "Unique EC for query" is confusing; EC can not
> be associated with a query per say and EC's are always unique because of
> canonicalisation. May be we should reword it as "Extract single EC for
> ordering of query, if any, so we don't consider it again." Is that cryptic
> as well?

Thanks.  I committed this version with one small tweak.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company



--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company
Attachment

pgsql-hackers by date:

Previous
From: Dean Rasheed
Date:
Subject: Re: Add numeric_trim(numeric)
Next
From: "Shulgin, Oleksandr"
Date:
Subject: Re: Inconsistent error handling in START_REPLICATION command