CuteChuanChuan commented on PR #22473: URL: https://github.com/apache/datafusion/pull/22473#issuecomment-4886633305
Thanks @comphead for the detailed and well-guided review — I've reworked to match your diagram. Two deviations below. Matches your diagram functions-nested: pub arrays_zip_inner_with_names(args, field_names) + pub arrays_zip_return_type(arg_types, field_names), with a private arrays_zip_inner(args) wrapper supplying the default 1-based ordinals (native UDF unchanged). spark: SparkArraysZip holds the names — with_field_names(Vec<String>) for callers, new() for none — and resolve_names() passes them into both native fns (single naming source, no post-hoc rename). A programmatic caller uses with_field_names directly and never hits the rewrite. Deviation 1 — None → 0-based ordinals, not arg_fields names Your new() derives from arg_fields[i].name(); I tried that and it panics, because those names aren't stable across optimizer passes (a literal arg gets renamed to lit): SELECT arrays_zip(a, a, a) FROM (VALUES ([1,2],[1,2],[1,2])) AS t(a, b, c); -- optimize_projections: schema mismatch -- declared: List<Struct<"0","1","2">> -- recomputed: List<Struct<"a","a","a">> So None produces positional ordinals (depend only on arg count). Spark-faithful names for SQL come from the rewrite below instead. Deviation 2 — the SQL rewrite is now an AnalyzerRule, not a FunctionRewrite ApplyFunctionRewrites rewrites expressions but never recomputes the plan node's schema, so the cached schema goes stale when a rewrite changes the output type (as pinning names does) — the same panic as above. An AnalyzerRule can recompute_schema() after pinning. This fixes every SLT case including a,a,a. Duplicate names are kept as-is per your Spark 4.1.1 check (arrays_zip(a,a) → struct<a,a>); I dropped the earlier de-dup. IMO mimicking Spark here brings no surprise to users. PTAL — happy to adjust the fallback. -- 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]
