alamb commented on code in PR #20272:
URL: https://github.com/apache/datafusion/pull/20272#discussion_r2788452502
##########
datafusion/optimizer/src/extract_leaf_expressions.rs:
##########
@@ -1357,63 +1306,21 @@ mod tests {
")
}
- /// A variant of MockLeafFunc with the same `name()` but a different
concrete type.
- /// Used to verify that deduplication uses `Expr` equality, not
`schema_name`.
- #[derive(Debug, PartialEq, Eq, Hash)]
- struct MockLeafFuncVariant {
- signature: Signature,
- }
-
- impl MockLeafFuncVariant {
- fn new() -> Self {
- Self {
- signature: Signature::new(
- TypeSignature::Any(2),
- datafusion_expr::Volatility::Immutable,
- ),
- }
- }
- }
-
- impl ScalarUDFImpl for MockLeafFuncVariant {
- fn as_any(&self) -> &dyn std::any::Any {
- self
- }
-
- fn name(&self) -> &str {
- "mock_leaf"
- }
-
- fn signature(&self) -> &Signature {
- &self.signature
- }
-
- fn return_type(&self, _args: &[DataType]) -> Result<DataType> {
- Ok(DataType::Utf8)
- }
-
- fn invoke_with_args(&self, _args: ScalarFunctionArgs) ->
Result<ColumnarValue> {
- unimplemented!("This is only used for testing optimization")
- }
-
- fn placement(&self, args: &[ExpressionPlacement]) ->
ExpressionPlacement {
- match args.first() {
- Some(ExpressionPlacement::Column)
- | Some(ExpressionPlacement::MoveTowardsLeafNodes) => {
- ExpressionPlacement::MoveTowardsLeafNodes
- }
- _ => ExpressionPlacement::KeepInPlace,
- }
- }
- }
-
/// Two UDFs with the same `name()` but different concrete types should
NOT be
/// deduplicated -- they are semantically different expressions that
happen to
/// collide on `schema_name()`.
#[test]
fn test_different_udfs_same_schema_name_not_deduplicated() -> Result<()> {
- let udf_a = Arc::new(ScalarUDF::new_from_impl(MockLeafFunc::new()));
- let udf_b =
Arc::new(ScalarUDF::new_from_impl(MockLeafFuncVariant::new()));
+ let udf_a = Arc::new(ScalarUDF::new_from_impl(
+ PlacementTestUDF::new()
+ .with_placement(ExpressionPlacement::MoveTowardsLeafNodes)
+ .with_id(1),
+ ));
+ let udf_b = Arc::new(ScalarUDF::new_from_impl(
+ PlacementTestUDF::new()
+ .with_placement(ExpressionPlacement::MoveTowardsLeafNodes)
+ .with_id(2),
+ ));
let expr_a = Expr::ScalarFunction(ScalarFunction::new_udf(
Review Comment:
I think you can make this less verbose via `expr_a =
udf_a.call(vec![col("user"), lit("field")])`
--
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]
