GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — feat: add support for INTERPRET function parsing by matteosist · Pull Request #1816 · JSQLParser/JSqlParser

Conversation

@matteosist

@matteosist matteosist commented Jun 29, 2023

Copy link
Copy Markdown
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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
molo17inc reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
@matteosist-dev
feat: add support for INTERPRET function parsing
bcf1133 
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

manticore-projects commented Jun 29, 2023

Copy link
Copy Markdown
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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
manticore-projects
manticore-projects requested changes Jun 29, 2023
View reviewed changes

@manticore-projects manticore-projects left a comment

Copy link
Copy Markdown
Contributor

There was a problem hiding this 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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread src/main/java/net/sf/jsqlparser/util/TablesNamesFinder.java Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread src/main/java/net/sf/jsqlparser/util/deparser/ExpressionDeParser.java Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread src/main/java/net/sf/jsqlparser/util/validation/validator/ExpressionValidator.java Resolved

Uh oh!

There was an error while loading. Please reload this page.
@matteosist

matteosist commented Jun 29, 2023

Copy link
Copy Markdown
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
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@matteosist-dev
Add INTERPRET support using CastExpression instead adding a new class
54fbf55 
Remove unused code
Replace stat import with class-specific import
@manticore-projects

manticore-projects commented Jun 29, 2023

Copy link
Copy Markdown
Contributor

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@manticore-projects
manticore-projects merged commit 180ec68 into JSQLParser:master Jun 29, 2023
@manticore-projects

manticore-projects commented Jun 29, 2023

Copy link
Copy Markdown
Contributor

Thank you again for your contribution, merged.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@matteosist

matteosist commented Jun 29, 2023

Copy link
Copy Markdown
Contributor Author

Fantastic. Thank you for this very useful library
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@manticore-projects manticore-projects manticore-projects requested changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@matteosist @manticore-projects @matteosist-dev
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.