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

feat: add support for INTERPRET function parsing #1816

Conversation

matteosist
Copy link
Contributor

Update JSqlParserCC.jjt adding properly definition Added InterpretExpression java class
Add visit function implementation to classes that implements ExpressionVisitor Added InterpretExpression tests

BREAKING CHANGE: No

Update JSqlParserCC.jjt adding properly definition
Added InterpretExpression java class
Add visit function implementation to classes that implements ExpressionVisitor
Added InterpretExpression tests

BREAKING CHANGE: No
@manticore-projects
Copy link
Contributor

Greetings!

Thank you your contribution and interest in JSQLParser.
One question though: what is the difference between a Cast() function and an Interpret() function (except the keyword Interpret). As far as I can see it, Interpretis a RDBMS specificCast()`, am I right?

If so, would it not be better to just to extend the existing Cast function to work with the Interprete keyword?
Saved us a lot of code.

Copy link
Contributor

@manticore-projects manticore-projects left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good in general, but I think it would be better merged into the existing Cast() function since only the keyword seems to be different.
Saves a lot of code for the Visitors and keeps the Grammar slim.

@matteosist
Copy link
Contributor Author

Greetings!

Thank you your contribution and interest in JSQLParser. One question though: what is the difference between a Cast() function and an Interpret() function (except the keyword Interpret). As far as I can see it, Interpretis a RDBMS specificCast()`, am I right?

If so, would it not be better to just to extend the existing Cast function to work with the Interprete keyword? Saved us a lot of code.

Thanks for the review.

Yes you are right, the behavior is similar to the CAST. It is database specific and was introduced in recent versions of the IBM i-series to replace the use of cast to parse journal content.

I will modify the PR by joining the code to the CAST function.

Thank you

Remove unused code
Replace stat import with class-specific import
@manticore-projects
Copy link
Contributor

Thanks! Looks good now and I would merge it when the QA tests will have ran through.

@manticore-projects manticore-projects merged commit 180ec68 into JSQLParser:master Jun 29, 2023
@manticore-projects
Copy link
Contributor

Thank you again for your contribution, merged.

@matteosist
Copy link
Contributor Author

Fantastic. Thank you for this very useful library

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

Successfully merging this pull request may close these issues.

2 participants