Re: TEXT vs VARCHAR join qual push down diffrence, bug or expected? - Mailing list pgsql-hackers
| From | Jeevan Chalke |
|---|---|
| Subject | Re: TEXT vs VARCHAR join qual push down diffrence, bug or expected? |
| Date | |
| Msg-id | CAM2+6=UhSgoTcLUwhQ0kdOroGc8A15KeU_U0JoHY15=ByzB+1w@mail.gmail.com Whole thread |
| In response to | Re: TEXT vs VARCHAR join qual push down diffrence, bug or expected? (Tom Lane <tgl@sss.pgh.pa.us>) |
| Responses |
Re: TEXT vs VARCHAR join qual push down diffrence, bug or expected?
|
| List | pgsql-hackers |
Hi Tom,
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
--
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
Principal Software Engineer, Product Development
EnterpriseDB Corporation
The Enterprise PostgreSQL Company
pgsql-hackers by date: