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

Moved "AT" keyword from reserved to non reserved #13

Merged
merged 3 commits into from
Oct 6, 2016
Merged

Moved "AT" keyword from reserved to non reserved #13

merged 3 commits into from
Oct 6, 2016

Conversation

nmorais
Copy link
Contributor

@nmorais nmorais commented Oct 5, 2016

The AT keyword is allowed to be used as a table alias on DML commands, so it is not a reserved keyword.

@nmorais nmorais mentioned this pull request Oct 5, 2016
@nmorais
Copy link
Contributor Author

nmorais commented Oct 5, 2016

@felipebz, it seems that my attempt to make a second pull request merged the two commits. Feel free to reject it if you wish.

What I really like to know is what gone wrong with Travis build. It did passed on ci build and failed on the three its builds. What is the difference between then?

Copy link
Owner

@felipebz felipebz left a comment

Choose a reason for hiding this comment

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

Hi @nmorais.

Since last week I've added some integration tests to verify the quality of the parser against some open source projects and avoid regressions (see #4).

The Travis build is failing because your change causes a unexpected parsing error in ut_receq.pks, ftp_util_pkg and ftp_util_pkg.pkb.

Please see my comments in the code..

@@ -37,6 +37,7 @@
BY("by", true),
CASE("case", true),
CHECK("check", true),
COMPILE("compile", true),
Copy link
Owner

Choose a reason for hiding this comment

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

"compile" is not a reserved keyword, it can be used as an identifier. See ut_receq.compile from the utPLSQL project.

@@ -87,6 +88,7 @@
OVERLAPS("overlaps", true),
PROCEDURE("procedure", true),
PUBLIC("public", true),
RENAME("rename", true),
Copy link
Owner

Choose a reason for hiding this comment

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

"rename" is not a reserved keyword too. Example: ftp_util_pkg.rename from the alexandria-plsql-utils.

assertThat(p).matches("alter package foo compile body;");
}

@Test
Copy link
Owner

Choose a reason for hiding this comment

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

Since there are some changes to fix in PlSqlKeyword.java, you could fix the spacing here too.

Fixed spacing in AlterPlsqlUnitTest.java
@felipebz
Copy link
Owner

felipebz commented Oct 6, 2016

👍 Good work. Thank you.

@felipebz felipebz merged commit 059e730 into felipebz:master Oct 6, 2016
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.

None yet

2 participants