Skip to content

ZEPPELIN-3344. Revert comments in queries in JDBC interpreter#2876

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.

Sign up for GitHub

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

Closed
mebelousov wants to merge 3 commits into from
Closed

ZEPPELIN-3344. Revert comments in queries in JDBC interpreter #2876

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
eed54c8
ZEPPELIN-3344 Revert comments in JDBC interpreter
mebelousov Mar 16, 2018
83c8e8f
ZEPPELIN-3344 Rename test
mebelousov Mar 16, 2018
6980400
ZEPPELIN-3344 Fix checkstyle, add tests for comments
mebelousov Mar 20, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 6 additions & 15 deletions jdbc/src/main/java/org/apache/zeppelin/jdbc/JDBCInterpreter.java
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -585,18 +585,12 @@ protected ArrayList<String> splitSqlQueries(String sql) {
for (int item = 0; item < sql.length(); item++) {
character = sql.charAt(item);

if ((singleLineComment && (character == '\n' || item == sql.length() - 1))
|| (multiLineComment && character == '/' && sql.charAt(item - 1) == '*')) {
if (singleLineComment && (character == '\n' || item == sql.length() - 1)) {
singleLineComment = false;
multiLineComment = false;
if (item == sql.length() - 1 && query.length() > 0) {
queries.add(StringUtils.trim(query.toString()));
}
continue;
}

if (singleLineComment || multiLineComment) {
continue;
if (multiLineComment && character == '/' && sql.charAt(item - 1) == '*') {
multiLineComment = false;
}

if (character == '\'') {
Expand All@@ -619,16 +613,13 @@ protected ArrayList<String> splitSqlQueries(String sql) {
&& sql.length() > item + 1) {
if (character == '-' && sql.charAt(item + 1) == '-') {
singleLineComment = true;
continue;
}

if (character == '/' && sql.charAt(item + 1) == '*') {
} else if (character == '/' && sql.charAt(item + 1) == '*') {
multiLineComment = true;
continue;
}
}

if (character == ';' && !quoteString && !doubleQuoteString) {
if (character == ';' && !quoteString && !doubleQuoteString && !multiLineComment
&& !singleLineComment) {
queries.add(StringUtils.trim(query.toString()));
query = new StringBuilder();
} else if (item == sql.length() - 1) {
Expand Down
11 changes: 8 additions & 3 deletions jdbc/src/test/java/org/apache/zeppelin/jdbc/JDBCInterpreterTest.java
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -189,13 +189,16 @@ public void testSplitSqlQuery() throws SQLException, IOException {
"select '\n', ';';" +
"select replace('A\\;B', '\\', 'text');" +
"select '\\', ';';" +
"select '''', ';'";
"select '''', ';';" +
"select /*+ scan */ * from test_table;" +
"--singleLineComment\nselect * from test_table";

@mebelousovmebelousovMar 29, 2018

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

@zjffdu The last two lines are new tests for processing of the comments.


Properties properties = new Properties();
JDBCInterpreter t = new JDBCInterpreter(properties);
t.open();
List<String> multipleSqlArray = t.splitSqlQueries(sqlQuery);
assertEquals(8, multipleSqlArray.size());
assertEquals(10, multipleSqlArray.size());
assertEquals("insert into test_table(id, name) values ('a', ';\"')", multipleSqlArray.get(0));
assertEquals("select * from test_table", multipleSqlArray.get(1));
assertEquals("select * from test_table WHERE ID = \";'\"", multipleSqlArray.get(2));
Expand All@@ -204,6 +207,8 @@ public void testSplitSqlQuery() throws SQLException, IOException {
assertEquals("select replace('A\\;B', '\\', 'text')", multipleSqlArray.get(5));
assertEquals("select '\\', ';'", multipleSqlArray.get(6));
assertEquals("select '''', ';'", multipleSqlArray.get(7));
assertEquals("select /*+ scan */ * from test_table", multipleSqlArray.get(8));
assertEquals("--singleLineComment\nselect * from test_table", multipleSqlArray.get(9));
}

@Test
Expand DownExpand Up@@ -534,7 +539,7 @@ public void testPrecodeWithAnotherPrefix() throws SQLException, IOException {
}

@Test
public void testExcludingComments() throws SQLException, IOException {
public void testSplitSqlQueryWithComments() throws SQLException, IOException {

@zjffduzjffduMar 16, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No test case needs to add or change ?

@mebelousovmebelousovMar 20, 2018

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

@zjffdu done.
The tests are green.

Properties properties = new Properties();
properties.setProperty("common.max_count", "1000");
properties.setProperty("common.max_retry", "3");
Expand Down