weiqingy commented on issue #1856: URL: https://github.com/apache/auron/issues/1856#issuecomment-4101960787
> hi [@weiqingy](https://github.com/weiqingy) First, Thank you so much for your work; your docs is excellent. then I’d like to share my previous design document, this includes my experience with POC. [google docs](https://docs.google.com/document/d/1MjT_Jia204OUg4jndvDam0KGW6zBIKYoswT8kWY5yuM/edit?tab=t.0) This might be helpful to you. Here are a few questions I’m interested : > > Q1: about FlinkNodeConverterFactory I think designing the `FlinkNodeConverterFactory` as a singleton would make it easier to use; in addition, we could draw on some best practices from the Gluten community. [RexCallConverterFactory](https://github.com/apache/gluten/blob/main/gluten-flink/planner/src/main/java/org/apache/gluten/rexnode/functions/RexCallConverterFactory.java) and [RexNodeConverter](https://github.com/apache/gluten/blob/main/gluten-flink/planner/src/main/java/org/apache/gluten/rexnode/RexNodeConverter.java) > > Q2: about ConverterContext I think we need to pass in the input schema. In my experience, when performing node transformations, we need to determine the data type of the input—for example, whether it involves mathematical or logical operations. If the types don’t match, we need to perform a type conversion first. > > Q3: about FlinkNodeConverter > > 1. it’s possible that ExecNode isn’t the only thing that needs to be converted; for example, in an Agg scenario, it would be AggregateCall. > 2. The return value of the convert API would be best suited for Auron's PhysicalPlanNode > > We can discuss these issues further, and please let me know if you have any questions. Thanks for the excellent feedback, @Tartarus0zm! I've revised the design doc based on all three points: Q1 (Singleton factory): Agreed. FlinkNodeConverterFactory is now a singleton with a register() method, following Gluten's RexCallConverterFactory pattern. Simpler to use and new converters in future PRs can register themselves without modifying setup code. Q2 (Input schema): Great catch — ConverterContext now includes RowType inputType. This is essential for resolving RexInputRef column references, checking type support, and determining when casts are needed. Richer than Gluten's List<String> attribute names since RowType carries both names and types. Q3 (Return type + expression scope): This was the impactful insight. FlinkNodeConverter.convert() now returns PhysicalPlanNode (native protobuf) instead of a replacement ExecNode<?>. The converter is responsible for translating internal expressions (RexNode, AggregateCall, etc.) within the node. This matches how Spark's native plan nodes build protobuf internally. Updated doc: https://github.com/weiqingy/auron/blob/AURON-1856-design/docs/PR-AURON-1856/AURON-1856-DESIGN.md I also looked at Gluten's RexNodeConverter and RexCallConverterFactory — the prior art comparison table in Section 5 now includes Gluten alongside Spark. Could you grant me access to the Google Docs design document you mentioned? I'd love to review your POC experience in detail. Thanks! -- 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]
