Skip to content

Fix issue in parsing TRY_CAST() function #1391

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

Merged
merged 6 commits into from
Nov 19, 2021

Conversation

prashantcsutar
Copy link
Contributor

MR request for issue #1390 Unable to parse snowflake's TRY_CAST() function

@manticore-projects
Copy link
Contributor

manticore-projects commented Oct 26, 2021

Prashant, thank you for the contribution and welcome to JSQLParser.
We appreciate you diving into the source code.

Please consider the following minimum requirement for accepting the PR:

  1. Amend the toString() method of the Java Object in order to reflect your new syntax or feature
  2. Amend the Deparser Classes in order to reflect your new syntax or feature
  3. Provide a Test Case for Parsing and Deparsing your new syntax or feature
  4. Ensure that all existing Use Case Tests still succeed (Please try not to break anything.)
  5. Ensure sufficient Test Coverage and Code Style compliance (The Maven/Gradle plugins would warn you.)

Right now, your query SELECT Try_cast( foo AS bar ) would be deparsed into SELECT Cast( foo AS bar ) which is not the same.

Do let me know if you will need any further assistance.

@prashantcsutar
Copy link
Contributor Author

Thanks for your review comments. I will work on those reviews and let you know once it's done.

@prashantcsutar
Copy link
Contributor Author

prashantcsutar commented Nov 8, 2021

Hi @manticore-projects ,

Can you please review this PR again? I have changes the implementation as suggested by you. Please let me know if it still needs any changes.

Best Regards,
Prashant

@manticore-projects
Copy link
Contributor

Thank you, it looks good to me and nicely done.
However, only @wumpz can decide about the final adoption. This may take a few days.

@prashantcsutar
Copy link
Contributor Author

Hi @wumpz , @manticore-projects ,
This fix is important for our project. Could you please merge it so that it will become part of 4.3 release?
Best Regards,
Prashant

@manticore-projects
Copy link
Contributor

Greetings.

Noted, we will see what we can do. @wumpz is author and owner of the software and only he can merge. I would be able to assist you with a Snapshot JAR if this helped anything.

@wumpz
Copy link
Member

wumpz commented Nov 19, 2021

Sorry for being late with merging. You know, there is some other stuff to do. However, your PR looks good.

@wumpz wumpz merged commit bfcf00f into JSQLParser:master Nov 19, 2021
@wumpz
Copy link
Member

wumpz commented Nov 19, 2021

I will deploy an actual snapshot during the next few hours.

@prashantcsutar
Copy link
Contributor Author

Hi @wumpz , Thanks a lot for merging this PR.

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.

3 participants