TEXT vs VARCHAR join qual push down diffrence, bug or expected?
Hi,
It is observed that, when we have one remote (huge) table and one local
(small) table and a join between them, then
1. If the column type is text, then we push the join qual to the remote
server, so that we will have less rows to fetch, and thus execution time
is very less.
2. If the column type is varchar, then we do not push the join qual to the
remote server, resulting into large number of data fetch and thus
execution time is very high.
Here is the EXPLAIN plan for such queries:
When VARCHAR column:
QUERY
PLAN
-------------------------------------------------------------------------------------------------------------------------------------------
Nested Loop (cost=100.15..4594935.73 rows=230 width=120) (actual
time=0.490..291.339 rows=1 loops=1)
Output: a.ename, d.dname
Join Filter: ((a.deptno)::text = (d.deptno)::text)
Rows Removed by Join Filter: 100099
-> Index Scan using emp2_pk on public.emp2 a (cost=0.15..8.17 rows=1
width=76) (actual time=0.009..0.013 rows=1 loops=1)
Output: a.empno, a.ename, a.deptno
Index Cond: (a.empno = '7369'::numeric)
-> Foreign Scan on public.fdw_dept2 d (cost=100.00..4594353.50
rows=45925 width=120) (actual time=0.466..274.990 rows=100100 loops=1)
Output: d.deptno, d.dname
Remote SQL: SELECT deptno, dname FROM public.dept2
Planning time: 0.697 ms
Execution time: 291.467 ms
(12 rows)
When TEXT column:
QUERY
PLAN
------------------------------------------------------------------------------------------------------------------------------
Nested Loop (cost=100.57..216.63 rows=238 width=120) (actual
time=0.375..0.378 rows=1 loops=1)
Output: a.ename, d.dname
-> Index Scan using emp3_pk on public.emp3 a (cost=0.15..8.17 rows=1
width=70) (actual time=0.010..0.011 rows=1 loops=1)
Output: a.empno, a.ename, a.deptno
Index Cond: (a.empno = '7369'::numeric)
-> Foreign Scan on public.fdw_dept3 d (cost=100.42..208.45 rows=1
width=114) (actual time=0.362..0.362 rows=1 loops=1)
Output: d.deptno, d.dname
Remote SQL: SELECT deptno, dname FROM public.dept3 WHERE
(($1::text = deptno))
Planning time: 1.220 ms
Execution time: 0.498 ms
(10 rows)
Attached test script to reproduce this theory.
I have observed that, since we do not have an equality operator for VARCHAR
type, we convert VARCHAR to TEXT using RelabelType and use texteq operator
function.
However in foreign_expr_walker(), for T_RelabelType case, we have these
conditions which do not allow us push the qual to remote.
/*
* RelabelType must not introduce a collation not derived
from
* an input foreign Var.
*/
collation = r->resultcollid;
if (collation == InvalidOid)
state = FDW_COLLATE_NONE;
else if (inner_cxt.state == FDW_COLLATE_SAFE &&
collation == inner_cxt.collation)
state = FDW_COLLATE_SAFE;
else
state = FDW_COLLATE_UNSAFE;
I guess, since we do push qual to remote in case of TEXT, we should do the
same for VARCHAR too.
Also given that RelabelType are just dummy wrapper for binary compatible
types, can we simply set collation and state from its inner context instead
on above check block. Like
/*
* Since RelabelType represents a "dummy" type coercion
between
* two binary-compatible datatypes, set collation and state
got
* from the inner_cxt.
*/
collation = inner_cxt.collation;
state = inner_cxt.state;
Inputs/Thought?
--
Jeevan B Chalke
Principal Software Engineer, Product Development
EnterpriseDB Corporation
The Enterprise PostgreSQL Company
Attachments:
pg_fdw_collation.sqltext/x-sql; charset=US-ASCII; name=pg_fdw_collation.sqlDownload
Jeevan Chalke <jeevan.chalke@enterprisedb.com> writes:
It is observed that, when we have one remote (huge) table and one local
(small) table and a join between them, then
1. If the column type is text, then we push the join qual to the remote
server, so that we will have less rows to fetch, and thus execution time
is very less.
2. If the column type is varchar, then we do not push the join qual to the
remote server, resulting into large number of data fetch and thus
execution time is very high.
Hmm ...
Also given that RelabelType are just dummy wrapper for binary compatible
types, can we simply set collation and state from its inner context instead
on above check block.
I think you're blaming the wrong code; RelabelType is handled basically
the same as most other cases.
It strikes me that this function is really going about things the wrong
way. Rather than trying to determine the output collation per se, what
we ought to be asking is "does every operator in the proposed expression
have an input collation that can be traced to some foreign Var further
down in the expression"? That is, given the example in hand,
RelabelType(ForeignVar) = RelabelType(LocalVar)
the logic ought to be like "the ForeignVar has collation X, and that
bubbles up without change through the RelabelType, and then the equals
operator's inputcollation matches that, so accept it --- regardless of
where the other operand's collation came from exactly". The key point
is that we want to validate operator input collations, not output
collations, as having something to do with what the remote side would do.
This would represent a fairly significant rewrite of foreign_expr_walker's
collation logic; although I think the end result would be no more
complicated, possibly simpler, than it is now.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Hi Tom,
On Tue, Sep 22, 2015 at 12:31 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
I think you're blaming the wrong code; RelabelType is handled basically
the same as most other cases.It strikes me that this function is really going about things the wrong
way. Rather than trying to determine the output collation per se, what
we ought to be asking is "does every operator in the proposed expression
have an input collation that can be traced to some foreign Var further
down in the expression"? That is, given the example in hand,RelabelType(ForeignVar) = RelabelType(LocalVar)
the logic ought to be like "the ForeignVar has collation X, and that
bubbles up without change through the RelabelType, and then the equals
operator's inputcollation matches that, so accept it --- regardless of
where the other operand's collation came from exactly". The key point
is that we want to validate operator input collations, not output
collations, as having something to do with what the remote side would do.This would represent a fairly significant rewrite of foreign_expr_walker's
collation logic; although I think the end result would be no more
complicated, possibly simpler, than it is now.regards, tom lane
IIUC, you are saying that collation check for output collation is not
necessary for all OpExpr/FuncExpr/ArrayRef etc.
Should we remove code blocks like
collation = r->resultcollid;
if (collation == InvalidOid)
state = FDW_COLLATE_NONE;
else if (inner_cxt.state == FDW_COLLATE_SAFE &&
collation == inner_cxt.collation)
state = FDW_COLLATE_SAFE;
else
state = FDW_COLLATE_UNSAFE;
and just bubble up the collation and state to the next level?
Here I see that, in the result collation validation, we missed the case when
result collation is default collation. For foreign var, we return collation
as is in inner context with the state set to SAFE. But in case of local var,
we are only allowing InvalidOid or Default oid for collation, however while
returning back, we set collation to InvalidOid and state to NONE even for
default collation. I think we are missing something here.
To handle this case, we need to either,
1. allow local var to set inner_cxt collation to what var actually has
(which
will be either Invalid or DEFAULT) and set state to NONE if non collable or
set state to SAFE if default collation. Like:
In T_Var, local var case
collation = var->varcollid;
state = OidIsValid(collation) ? FDW_COLLATE_SAFE : FDW_COLLATE_NONE;
OR
2. In above code block, which checks result collation, we need to handle
default collation. Like:
else if (collation == DEFAULT_COLLATION_OID)
state = inner_cxt.state;
Let me know if I missed any.
Thanks
--
Jeevan B Chalke
Principal Software Engineer, Product Development
EnterpriseDB Corporation
The Enterprise PostgreSQL Company
Jeevan Chalke <jeevan.chalke@enterprisedb.com> writes:
On Tue, Sep 22, 2015 at 12:31 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
It strikes me that this function is really going about things the wrong
way. Rather than trying to determine the output collation per se, what
we ought to be asking is "does every operator in the proposed expression
have an input collation that can be traced to some foreign Var further
down in the expression"?
IIUC, you are saying that collation check for output collation is not
necessary for all OpExpr/FuncExpr/ArrayRef etc.
Should we remove code blocks like
collation = r->resultcollid;
if (collation == InvalidOid)
state = FDW_COLLATE_NONE;
else if (inner_cxt.state == FDW_COLLATE_SAFE &&
collation == inner_cxt.collation)
state = FDW_COLLATE_SAFE;
else
state = FDW_COLLATE_UNSAFE;
and just bubble up the collation and state to the next level?
Removing that entirely would be quite incorrect, because then you'd be
lying to the parent node about what collation your node outputs.
After thinking a bit more about the existing special case for non-foreign
Vars, I wonder if what we should do is change these code blocks to look
like
collation = r->resultcollid;
if (collation == InvalidOid)
state = FDW_COLLATE_NONE;
else if (inner_cxt.state == FDW_COLLATE_SAFE &&
collation == inner_cxt.collation)
state = FDW_COLLATE_SAFE;
+ else if (collation == DEFAULT_COLLATION_OID)
+ state = FDW_COLLATE_NONE;
else
state = FDW_COLLATE_UNSAFE;
That is, only explicit introduction of a non-default collation causes
a subexpression to get labeled FDW_COLLATE_UNSAFE. Default collations
would lose out when getting merged with a nondefault collation from a
foreign Var, so they should work all right.
The core point here is that we're going to send the expression to the
remote without any COLLATE clauses, so the remote's parser has to
come to the same conclusions we did about which collation to apply.
We assume that default-collation-throughout will work all right.
Nondefault collations will work as long as they originate from foreign
Vars, because then the remote parser should see the equivalent far-end
collations originating from those Vars --- and those collations win when
combined with default collations.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
I wrote:
After thinking a bit more about the existing special case for non-foreign
Vars, I wonder if what we should do is change these code blocks to look
like
collation = r->resultcollid; if (collation == InvalidOid) state = FDW_COLLATE_NONE; else if (inner_cxt.state == FDW_COLLATE_SAFE && collation == inner_cxt.collation) state = FDW_COLLATE_SAFE; + else if (collation == DEFAULT_COLLATION_OID) + state = FDW_COLLATE_NONE; else state = FDW_COLLATE_UNSAFE;
On further thought, maybe it's the special case for non-foreign Vars
that is busted. That is, non-foreign Vars should do
/* Var belongs to some other table */
if (var->varcollid != InvalidOid &&
var->varcollid != DEFAULT_COLLATION_OID)
return false;
- /* We can consider that it doesn't set collation */
- collation = InvalidOid;
- state = FDW_COLLATE_NONE;
+ collation = var->varcollid;
+ state = OidIsValid(collation) ? FDW_COLLATE_SAFE : FDW_COLLATE_NONE;
and likewise for Consts, Params, etc.
This would basically mean clarifying the meaning of the state values as:
FDW_COLLATE_NONE: the expression is of a noncollatable type, period.
FDW_COLLATE_SAFE: the expression has default collation, or a nondefault
collation that is traceable to a foreign Var.
FDW_COLLATE_UNSAFE: the expression has a nondefault collation that is not
traceable to a foreign Var.
Hm ... actually, we probably need *both* types of changes if that's
what we believe the state values mean.
An alternative definition would be that FDW_COLLATE_NONE subsumes the
"collation doesn't trace to a foreign Var, but it's default so we don't
really care" case. I think the problem we've got is that the
non-foreign-Var code thinks that's what the definition is, but the rest
of the code isn't quite consistent with it.
In any case I think we want to end up with a clearer specification of
what the states mean.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
I wrote:
Hm ... actually, we probably need *both* types of changes if that's
what we believe the state values mean.
After a bit more thinking and experimentation, I propose the attached
patch.
regards, tom lane
Attachments:
improve-postgres-fdw-collation-handling.patchtext/x-diff; charset=us-ascii; name=improve-postgres-fdw-collation-handling.patchDownload+167-115
On Wed, Sep 23, 2015 at 7:29 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Removing that entirely would be quite incorrect, because then you'd be
lying to the parent node about what collation your node outputs.
Yes. I too thought so and thus wanted to fix that code block by
considering the default collation.
After thinking a bit more about the existing special case for non-foreign
Vars, I wonder if what we should do is change these code blocks to look
likecollation = r->resultcollid; if (collation == InvalidOid) state = FDW_COLLATE_NONE; else if (inner_cxt.state == FDW_COLLATE_SAFE && collation == inner_cxt.collation) state = FDW_COLLATE_SAFE; + else if (collation == DEFAULT_COLLATION_OID) + state = FDW_COLLATE_NONE; else state = FDW_COLLATE_UNSAFE;That is, only explicit introduction of a non-default collation causes
a subexpression to get labeled FDW_COLLATE_UNSAFE. Default collations
would lose out when getting merged with a nondefault collation from a
foreign Var, so they should work all right.
Agree.
I had suggested similar changes in approach (2)
but you put that check at exact required place.
regards, tom lane
--
Jeevan B Chalke
Principal Software Engineer, Product Development
EnterpriseDB Corporation
The Enterprise PostgreSQL Company
On Wed, Sep 23, 2015 at 10:15 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
I wrote:
Hm ... actually, we probably need *both* types of changes if that's
what we believe the state values mean.
I too was confused with the state explanations from the code-comments which
we have them now. With your explanation here clears that.
Thanks for explaining those.
After a bit more thinking and experimentation, I propose the attached
patch.
I had a look over the patch and reviewed it. It is in excellent state to
check-in.
regards, tom lane
Thanks
--
Jeevan B Chalke
Principal Software Engineer, Product Development
EnterpriseDB Corporation
The Enterprise PostgreSQL Company
Jeevan Chalke <jeevan.chalke@enterprisedb.com> writes:
On Wed, Sep 23, 2015 at 10:15 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
After a bit more thinking and experimentation, I propose the attached
patch.
I had a look over the patch and reviewed it. It is in excellent state to
check-in.
After further thought I decided that the base case for
Const/Param/non-foreign-Vars wasn't quite right either. If we don't like
the collation we should just set the state to UNSAFE not fail immediately,
because it might appear in a context where collation doesn't matter.
An example is "var IS NOT NULL".
So I've committed the attached modification of that patch.
regards, tom lane
Attachments:
improve-postgres-fdw-collation-handling-2.patchtext/x-diff; charset=us-ascii; name=improve-postgres-fdw-collation-handling-2.patchDownload+201-147
On Thu, Sep 24, 2015 at 10:22 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Jeevan Chalke <jeevan.chalke@enterprisedb.com> writes:
On Wed, Sep 23, 2015 at 10:15 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
After a bit more thinking and experimentation, I propose the attached
patch.I had a look over the patch and reviewed it. It is in excellent state to
check-in.After further thought I decided that the base case for
Const/Param/non-foreign-Vars wasn't quite right either. If we don't like
the collation we should just set the state to UNSAFE not fail immediately,
because it might appear in a context where collation doesn't matter.
An example is "var IS NOT NULL".
Make sense.
So I've committed the attached modification of that patch.
Thanks
regards, tom lane
--
Jeevan B Chalke
Principal Software Engineer, Product Development
EnterpriseDB Corporation
The Enterprise PostgreSQL Company