kosiew commented on PR #23188:
URL: https://github.com/apache/datafusion/pull/23188#issuecomment-4966552470
> One issue still is that datafusion-proto can't serialize
SqlSimilarToPattern but I would argue that this is out of scope for this PR.
Since this PR introduces `SqlSimilarToPattern` as a new built-in physical
expression used for SQL-visible plans, either that expression needs proto
support, or the PR should avoid introducing it into plans that need to
serialize. Otherwise distributed/serialized physical plans that include dynamic
`SIMILAR TO` regress.
Recommendation: if the goal is to keep this PR small, limit this PR's
SQL-correct translation to literal patterns and preserve the previous
dynamic-pattern representation for now. In practice, that means translating
only `Literal` RHS patterns and using the original `pattern` expression
unchanged for non-literal RHS patterns:
```rust
let translated_pattern = match
pattern.downcast_ref::<crate::expressions::Literal>() {
Some(literal) =>
Arc::new(crate::expressions::Literal::new(translate_scalar(
literal.value(),
)?)) as Arc<dyn PhysicalExpr>,
None => pattern,
};
```
Then in a follow up PR, add the SqlSimilarToPattern AND proto support.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
