From 6db3c464350cc112f45085bd7d82d7dfdc3e370f Mon Sep 17 00:00:00 2001 From: Tinkoff DWH Date: Sun, 19 Mar 2017 12:39:07 +0500 Subject: [PATCH 1/3] [ZEPPELIN-2279] excluded comments from SQL --- .../apache/zeppelin/jdbc/JDBCInterpreter.java | 44 ++++++++++++++++--- .../zeppelin/jdbc/JDBCInterpreterTest.java | 26 +++++++++++ 2 files changed, 65 insertions(+), 5 deletions(-) diff --git a/jdbc/src/main/java/org/apache/zeppelin/jdbc/JDBCInterpreter.java b/jdbc/src/main/java/org/apache/zeppelin/jdbc/JDBCInterpreter.java index 1080c2bb7cd..690e71b1a01 100644 --- a/jdbc/src/main/java/org/apache/zeppelin/jdbc/JDBCInterpreter.java +++ b/jdbc/src/main/java/org/apache/zeppelin/jdbc/JDBCInterpreter.java @@ -509,15 +509,32 @@ protected ArrayList splitSqlQueries(String sql) { Character character; Boolean antiSlash = false; + Boolean multiLineComment = false; + Boolean singleLineComment = false; Boolean quoteString = false; Boolean doubleQuoteString = false; for (int item = 0; item < sql.length(); item++) { character = sql.charAt(item); + if ((singleLineComment && (character.equals('\n') || item == sql.length() - 1)) + || (multiLineComment && character.equals('/') && sql.charAt(item - 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 (character.equals('\\')) { antiSlash = true; } + if (character.equals('\'')) { if (antiSlash) { antiSlash = false; @@ -527,6 +544,7 @@ protected ArrayList splitSqlQueries(String sql) { quoteString = true; } } + if (character.equals('"')) { if (antiSlash) { antiSlash = false; @@ -537,26 +555,42 @@ protected ArrayList splitSqlQueries(String sql) { } } + if (!quoteString && !doubleQuoteString && !multiLineComment && !singleLineComment + && sql.length() > item + 1) { + if (character.equals('-') && sql.charAt(item + 1) == '-') { + singleLineComment = true; + continue; + } + + if (character.equals('/') && sql.charAt(item + 1) == '*') { + multiLineComment = true; + continue; + } + } + if (character.equals(';') && !antiSlash && !quoteString && !doubleQuoteString) { - queries.add(query.toString()); + queries.add(StringUtils.trim(query.toString())); query = new StringBuilder(); } else if (item == sql.length() - 1) { query.append(character); - queries.add(query.toString()); + queries.add(StringUtils.trim(query.toString())); } else { query.append(character); } } + return queries; } private void executePrecode(Connection connection, String propertyKey) throws SQLException { String precode = getProperty(String.format(PRECODE_KEY_TEMPLATE, propertyKey)); if (StringUtils.isNotBlank(precode)) { - precode = StringUtils.trim(precode); - logger.info("Run SQL precode '{}'", precode); + List precodeQueries = splitSqlQueries(precode); try (Statement statement = connection.createStatement()) { - statement.execute(precode); + for (String precodeQuery : precodeQueries) { + logger.info("Run SQL precode '{}'", precodeQuery); + statement.execute(precodeQuery); + } if (!connection.getAutoCommit()) { connection.commit(); } diff --git a/jdbc/src/test/java/org/apache/zeppelin/jdbc/JDBCInterpreterTest.java b/jdbc/src/test/java/org/apache/zeppelin/jdbc/JDBCInterpreterTest.java index 2e7e1a5116e..26393020578 100644 --- a/jdbc/src/test/java/org/apache/zeppelin/jdbc/JDBCInterpreterTest.java +++ b/jdbc/src/test/java/org/apache/zeppelin/jdbc/JDBCInterpreterTest.java @@ -448,4 +448,30 @@ public void testPrecodeWithAnotherPrefix() throws SQLException, IOException { assertEquals(InterpreterResult.Type.TABLE, interpreterResult.message().get(0).getType()); assertEquals("@TESTVARIABLE\n2\n", interpreterResult.message().get(0).getData()); } + + @Test + public void testExcludingComments() throws SQLException, IOException { + Properties properties = new Properties(); + properties.setProperty("common.max_count", "1000"); + properties.setProperty("common.max_retry", "3"); + properties.setProperty("default.driver", "org.h2.Driver"); + properties.setProperty("default.url", getJdbcConnection()); + properties.setProperty("default.user", ""); + properties.setProperty("default.password", ""); + JDBCInterpreter t = new JDBCInterpreter(properties); + t.open(); + + String sqlQuery = "/* ; */\n" + + "--select * from test_table\n" + + "select * from test_table; /* some comment ; */\n" + + "/*\n" + + "select * from test_table;\n" + + "*/\n" + + "select * from test_table WHERE ID = ';--';\n" + + "select * from test_table WHERE ID = '/*' -- test"; + + InterpreterResult interpreterResult = t.interpret(sqlQuery, interpreterContext); + assertEquals(InterpreterResult.Code.SUCCESS, interpreterResult.code()); + assertEquals(3, interpreterResult.message().size()); + } } From f48f7d68750a098ff944c0309c0b68b425d95899 Mon Sep 17 00:00:00 2001 From: Tinkoff DWH Date: Thu, 23 Mar 2017 14:13:06 +0500 Subject: [PATCH 2/3] [ZEPPELIN-2279] improve test, revert precode execution --- .../java/org/apache/zeppelin/jdbc/JDBCInterpreter.java | 8 +++----- .../org/apache/zeppelin/jdbc/JDBCInterpreterTest.java | 1 + 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/jdbc/src/main/java/org/apache/zeppelin/jdbc/JDBCInterpreter.java b/jdbc/src/main/java/org/apache/zeppelin/jdbc/JDBCInterpreter.java index 690e71b1a01..5928fee0eac 100644 --- a/jdbc/src/main/java/org/apache/zeppelin/jdbc/JDBCInterpreter.java +++ b/jdbc/src/main/java/org/apache/zeppelin/jdbc/JDBCInterpreter.java @@ -585,12 +585,10 @@ protected ArrayList splitSqlQueries(String sql) { private void executePrecode(Connection connection, String propertyKey) throws SQLException { String precode = getProperty(String.format(PRECODE_KEY_TEMPLATE, propertyKey)); if (StringUtils.isNotBlank(precode)) { - List precodeQueries = splitSqlQueries(precode); + precode = StringUtils.trim(precode); + logger.info("Run SQL precode '{}'", precode); try (Statement statement = connection.createStatement()) { - for (String precodeQuery : precodeQueries) { - logger.info("Run SQL precode '{}'", precodeQuery); - statement.execute(precodeQuery); - } + statement.execute(precode); if (!connection.getAutoCommit()) { connection.commit(); } diff --git a/jdbc/src/test/java/org/apache/zeppelin/jdbc/JDBCInterpreterTest.java b/jdbc/src/test/java/org/apache/zeppelin/jdbc/JDBCInterpreterTest.java index 26393020578..608c657b16a 100644 --- a/jdbc/src/test/java/org/apache/zeppelin/jdbc/JDBCInterpreterTest.java +++ b/jdbc/src/test/java/org/apache/zeppelin/jdbc/JDBCInterpreterTest.java @@ -467,6 +467,7 @@ public void testExcludingComments() throws SQLException, IOException { "/*\n" + "select * from test_table;\n" + "*/\n" + + "-- a ; b\n" + "select * from test_table WHERE ID = ';--';\n" + "select * from test_table WHERE ID = '/*' -- test"; From 3f7496e835c9e78a77e06cb23efed96a7953493d Mon Sep 17 00:00:00 2001 From: Tinkoff DWH Date: Wed, 5 Apr 2017 12:32:44 +0500 Subject: [PATCH 3/3] [ZEPPELIN-2279] fix conditions, common format --- .../apache/zeppelin/jdbc/JDBCInterpreter.java | 18 +++++++++--------- .../zeppelin/jdbc/JDBCInterpreterTest.java | 1 + 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/jdbc/src/main/java/org/apache/zeppelin/jdbc/JDBCInterpreter.java b/jdbc/src/main/java/org/apache/zeppelin/jdbc/JDBCInterpreter.java index 5928fee0eac..a00c5458cf9 100644 --- a/jdbc/src/main/java/org/apache/zeppelin/jdbc/JDBCInterpreter.java +++ b/jdbc/src/main/java/org/apache/zeppelin/jdbc/JDBCInterpreter.java @@ -506,7 +506,7 @@ private boolean isDDLCommand(int updatedCount, int columnCount) throws SQLExcept protected ArrayList splitSqlQueries(String sql) { ArrayList queries = new ArrayList<>(); StringBuilder query = new StringBuilder(); - Character character; + char character; Boolean antiSlash = false; Boolean multiLineComment = false; @@ -517,8 +517,8 @@ protected ArrayList splitSqlQueries(String sql) { for (int item = 0; item < sql.length(); item++) { character = sql.charAt(item); - if ((singleLineComment && (character.equals('\n') || item == sql.length() - 1)) - || (multiLineComment && character.equals('/') && sql.charAt(item - 1) == '*')) { + if ((singleLineComment && (character == '\n' || item == sql.length() - 1)) + || (multiLineComment && character == '/' && sql.charAt(item - 1) == '*')) { singleLineComment = false; multiLineComment = false; if (item == sql.length() - 1 && query.length() > 0) { @@ -531,11 +531,11 @@ protected ArrayList splitSqlQueries(String sql) { continue; } - if (character.equals('\\')) { + if (character == '\\') { antiSlash = true; } - if (character.equals('\'')) { + if (character == '\'') { if (antiSlash) { antiSlash = false; } else if (quoteString) { @@ -545,7 +545,7 @@ protected ArrayList splitSqlQueries(String sql) { } } - if (character.equals('"')) { + if (character == '"') { if (antiSlash) { antiSlash = false; } else if (doubleQuoteString) { @@ -557,18 +557,18 @@ protected ArrayList splitSqlQueries(String sql) { if (!quoteString && !doubleQuoteString && !multiLineComment && !singleLineComment && sql.length() > item + 1) { - if (character.equals('-') && sql.charAt(item + 1) == '-') { + if (character == '-' && sql.charAt(item + 1) == '-') { singleLineComment = true; continue; } - if (character.equals('/') && sql.charAt(item + 1) == '*') { + if (character == '/' && sql.charAt(item + 1) == '*') { multiLineComment = true; continue; } } - if (character.equals(';') && !antiSlash && !quoteString && !doubleQuoteString) { + if (character == ';' && !antiSlash && !quoteString && !doubleQuoteString) { queries.add(StringUtils.trim(query.toString())); query = new StringBuilder(); } else if (item == sql.length() - 1) { diff --git a/jdbc/src/test/java/org/apache/zeppelin/jdbc/JDBCInterpreterTest.java b/jdbc/src/test/java/org/apache/zeppelin/jdbc/JDBCInterpreterTest.java index 608c657b16a..39c16ac4eee 100644 --- a/jdbc/src/test/java/org/apache/zeppelin/jdbc/JDBCInterpreterTest.java +++ b/jdbc/src/test/java/org/apache/zeppelin/jdbc/JDBCInterpreterTest.java @@ -462,6 +462,7 @@ public void testExcludingComments() throws SQLException, IOException { t.open(); String sqlQuery = "/* ; */\n" + + "-- /* comment\n" + "--select * from test_table\n" + "select * from test_table; /* some comment ; */\n" + "/*\n" +