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

Rationalize/consolidate type grammar #686

Open
jacques-n opened this issue Aug 19, 2024 · 5 comments
Open

Rationalize/consolidate type grammar #686

jacques-n opened this issue Aug 19, 2024 · 5 comments

Comments

@jacques-n
Copy link
Contributor

jacques-n commented Aug 19, 2024

There are several grammars for the same thing in Substrait subprojects.

  1. There was originally a type parser built in substrait-java
  2. Later, someone took that and reimplemented it in substrait-validator
  3. Later again, substrait-cpp created their own parser without antlr

We need to rationalize these two parsers (and any others that exist) and create a single parser in the core substrait repository.

I have no idea if they agree in behavior. I believe the java one also supports parsing to the other proto type definitions that aren't used in plans today (were intended for consumers informing producers of UDF behavior).

It's reasonably for each language binding to have their own specific implementations using a common grammar but we should avoid having distinct grammars across subprojects.

@EpsilonPrime
Copy link
Member

EpsilonPrime commented Aug 19, 2024 via email

@jacques-n
Copy link
Contributor Author

jacques-n commented Aug 19, 2024

picard

Type parsers? Can you provide pointers to them?

@EpsilonPrime
Copy link
Member

@mbrobbel
Copy link
Member

create a single parser

Shouldn't we add the grammar definition to the core repo and keep the implementations in the language repos?

@jacques-n
Copy link
Contributor Author

Shouldn't we add the grammar definition to the core repo and keep the implementations in the language repos?

Yes, this was what this ticket is trying to suggest. I'll update description make clearer.

@jacques-n jacques-n changed the title Rationalize/consolidate type parsers Rationalize/consolidate type grammar Aug 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants