Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor QueryPlanSerde to Use Individual Classes for Expression Translation #1330

Open
andygrove opened this issue Jan 23, 2025 · 0 comments
Labels
enhancement New feature or request

Comments

@andygrove
Copy link
Member

What is the problem the feature request solves?

The QueryPlanSerde class in Comet currently contains a huge match statement that handles the logic for translating Spark expressions into protobuf.

Each match arm:

  • Performs type-checking to determine if the expression is supported.
  • Contains the code for converting the expression into its protobuf representation.

While functional, this approach has limitations:

  • Readability: The large match statement makes the code harder to navigate and maintain.
  • Testability: Testing specific expression logic requires interacting with the entire QueryPlanSerde implementation.
  • Extensibility: Adding support for new expressions or customizing behavior for specific expressions is non-trivial.

I would like to refactor QueryPlanSerde to move the logic for each expression into individual classes or objects. Each expression type would have its own class or object that performs type-checking and conversion to protobuf.

This would help with unit testing since we can write tests to interact with these new classes or objects without having to use the full QueryPlanSerde implementation.

Describe the potential solution

No response

Additional context

No response

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant