Tom Lane писал 2021-07-29 23:54:
Alexander Pyhalov <a.pyha...@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
