Re: Case expression pushdown

Tom Lane писал 2021-07-29 23:54:
> Alexander Pyhalov <a.pyhalov@postgrespro.ru> writes:
>> [ 0001-Allow-pushing-CASE-expression-to-foreign-server-v7.patch ]
> 
> I looked this over.  It's better than before, but the collation
> handling is still not at all correct.  We have to consider that a
> CASE's arg expression supplies the collation for a contained
> CaseTestExpr, otherwise we'll come to the wrong conclusions about
> whether "CASE foreignvar WHEN ..." is shippable, if the foreignvar
> is what's determining collation of the comparisons.
> 
> This means that the CaseExpr level of recursion has to pass data down
> to the CaseTestExpr level.  In the attached, I did that by adding an
> additional argument to foreign_expr_walker().  That's a bit invasive,
> but it's not awful.  I thought about instead adding fields to the
> foreign_loc_cxt struct.  But that seemed considerably messier in the
> end, because we'd then have some fields that are information sourced
> at one recursion level and some that are info sourced at another
> level.
> 
> I also whacked the regression test cases around a lot.  They seemed
> to spend a lot of time on irrelevant combinations, while failing to
> check the things that matter, namely whether collation-based pushdown
> decisions are made correctly.
> 
>             regards, tom lane

Hi.

Overall looks good.
The only thing I'm confused about is in T_CaseTestExpr case - how can it 
be that CaseTestExpr collation doesn't match case_arg_cxt->collation ?
Do we we need to inspect only case_arg_cxt->state? Can we assert that 
collation == case_arg_cxt->collation?

-- 
Best regards,
Alexander Pyhalov,
Postgres Professional