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

Fixes #1684: Support CREATE MATERIALIZED VIEW with AUTO REFRESH #1691

Closed
wants to merge 3 commits into from

Conversation

zaza
Copy link
Contributor

@zaza zaza commented Dec 11, 2022

Support parsing create view statements in Redshift with AUTO REFRESH option.

…RESH

Support parsing create view statements in Redshift with AUTO REFRESH
option.
@wumpz
Copy link
Member

wumpz commented Dec 22, 2022

please fix those branch conflicts first

@zaza
Copy link
Contributor Author

zaza commented Dec 22, 2022

please fix those branch conflicts first

Done with Github UI so fingers crossed.

@wumpz
Copy link
Member

wumpz commented Dec 27, 2022

You still have some PMD violations there. Look at the failing tests at GitHub.

Extract adding the force option into a dedicated method resulting in the
cyclomatic complexity reduction of the CreateView.toString method.
@zaza
Copy link
Contributor Author

zaza commented Jan 8, 2023

@wumpz can please help me understand how a failure in SpecialOracleTest.testAllSqlsParseDeparse for file query_factoring01.sql is related to this change? I'm confused.

@wumpz
Copy link
Member

wumpz commented Jan 12, 2023

@zaza In the log file is written:

INFO: tested 276 files. got 195 correct parse results, expected 196

This oracle test scans through a list of over 200 sqls. It expects 196 parse successes. Obviously your change somehow degraded the grammar so one sql could not be parsed anymore.

@zaza
Copy link
Contributor Author

zaza commented Jan 29, 2023

Thanks @wumpz, but I got that part. I just don't understand how my change (in how CREATE VIEW is parsed/deparsed) could possibly broke the test. Like I said in my previous comment, the affected test file is query_factoring01.sql and the error message is --@FAILURE: Encountered unexpected token: "," "," recorded first on Jan 29, 2023, 10:56:13 PM (just ran it again). What puzzles me is that this is not a CREATE VIEW statement. I'm sorry, I'm new to this project and I'm probably missing something obvious.

@@ -182,6 +182,11 @@ public static List<String> getReservedKeywords(int restriction) {

, { "NEXTVAL", RESTRICTED_JSQLPARSER }

, { "AUTO", RESTRICTED_JSQLPARSER }
Copy link
Contributor

Choose a reason for hiding this comment

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

Are those really restricted Keywords which must not been used?
Or are those just keywords? Why exactly have those Tokens been added?

manticore-projects added a commit to manticore-projects/JSqlParser that referenced this pull request Jan 30, 2023
@manticore-projects
Copy link
Contributor

@zaza: I have merged your changes into PR #1722 and fixed it by remove the unnecessary Reserved Keywords, you have introduced. You can check the new syntax here https://manticore-projects.github.io/JSqlParser/syntax.html#createview

In general, keywords must only be reserved, when absolutely needed to keep the parser working (and not in this case).

Thank you for your contribution! Please consider to close this PR.

@zaza
Copy link
Contributor Author

zaza commented Feb 9, 2023

Thanks @manticore-projects , closing the PR as requested.

@zaza zaza closed this Feb 9, 2023
@zaza zaza deleted the 1684-create-mv-auto-refresh branch February 9, 2023 09:21
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