You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zeppelin.apache.org by GitBox <gi...@apache.org> on 2021/06/19 04:37:56 UTC

[GitHub] [zeppelin] zjffdu opened a new pull request #4143: [ZEPPELIN-5311] Unable to run list & add hive statement

zjffdu opened a new pull request #4143:
URL: https://github.com/apache/zeppelin/pull/4143


   ### What is this PR for?
   
   Trivial PR to support list & add hive statement as well, just trim the whitespace otherwise hive will throw syntax error exception.
   
   ### What type of PR is it?
   [Improvement ]
   
   ### Todos
   * [ ] - Task
   
   ### What is the Jira issue?
   * https://issues.apache.org/jira/browse/ZEPPELIN-5311
   
   ### How should this be tested?
   * Manually tested
   
   ### Screenshots (if appropriate)
   
   ### Questions:
   * Does the licenses files need update? No
   * Is there breaking changes for older versions? No
   * Does this needs documentation? No
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [zeppelin] zjffdu commented on a change in pull request #4143: [ZEPPELIN-5311] Unable to run list & add hive statement

Posted by GitBox <gi...@apache.org>.
zjffdu commented on a change in pull request #4143:
URL: https://github.com/apache/zeppelin/pull/4143#discussion_r656730671



##########
File path: jdbc/src/main/java/org/apache/zeppelin/jdbc/JDBCInterpreter.java
##########
@@ -727,12 +727,14 @@ private InterpreterResult executeSql(String dbPrefix, String sql,
     try {
       List<String>  sqlArray = sqlSplitter.splitSql(sql);
       for (String sqlToExecute : sqlArray) {
-        LOGGER.info("Execute sql: " + sqlToExecute);
-        if (sqlToExecute.trim().toLowerCase().startsWith("set ")) {
+        if (sqlToExecute.trim().toLowerCase().startsWith("set ") ||
+                sqlToExecute.trim().toLowerCase().startsWith("list ") ||
+                sqlToExecute.trim().toLowerCase().startsWith("add ")) {

Review comment:
       Hive sql: `list jars`




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [zeppelin] cuspymd commented on a change in pull request #4143: [ZEPPELIN-5311] Unable to run list & add hive statement

Posted by GitBox <gi...@apache.org>.
cuspymd commented on a change in pull request #4143:
URL: https://github.com/apache/zeppelin/pull/4143#discussion_r656743061



##########
File path: jdbc/src/main/java/org/apache/zeppelin/jdbc/JDBCInterpreter.java
##########
@@ -727,12 +727,14 @@ private InterpreterResult executeSql(String dbPrefix, String sql,
     try {
       List<String>  sqlArray = sqlSplitter.splitSql(sql);
       for (String sqlToExecute : sqlArray) {
-        LOGGER.info("Execute sql: " + sqlToExecute);
-        if (sqlToExecute.trim().toLowerCase().startsWith("set ")) {
+        if (sqlToExecute.trim().toLowerCase().startsWith("set ") ||
+                sqlToExecute.trim().toLowerCase().startsWith("list ") ||
+                sqlToExecute.trim().toLowerCase().startsWith("add ")) {

Review comment:
       As [a Hive Manual](https://cwiki.apache.org/confluence/display/hive/languagemanual+cli), It seems be a commend of the interpreter shell mode. There is other command like `DELETE` also. 




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [zeppelin] cuspymd commented on a change in pull request #4143: [ZEPPELIN-5311] Unable to run list & add hive statement

Posted by GitBox <gi...@apache.org>.
cuspymd commented on a change in pull request #4143:
URL: https://github.com/apache/zeppelin/pull/4143#discussion_r655416528



##########
File path: jdbc/src/main/java/org/apache/zeppelin/jdbc/JDBCInterpreter.java
##########
@@ -727,12 +727,14 @@ private InterpreterResult executeSql(String dbPrefix, String sql,
     try {
       List<String>  sqlArray = sqlSplitter.splitSql(sql);
       for (String sqlToExecute : sqlArray) {
-        LOGGER.info("Execute sql: " + sqlToExecute);
-        if (sqlToExecute.trim().toLowerCase().startsWith("set ")) {
+        if (sqlToExecute.trim().toLowerCase().startsWith("set ") ||
+                sqlToExecute.trim().toLowerCase().startsWith("list ") ||
+                sqlToExecute.trim().toLowerCase().startsWith("add ")) {

Review comment:
       It would be good to move `sqlToExecute.trim().toLowerCase()` out of `if` statement.
   Anyway, which sql language has `list` keyword?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [zeppelin] cuspymd commented on a change in pull request #4143: [ZEPPELIN-5311] Unable to run list & add hive statement

Posted by GitBox <gi...@apache.org>.
cuspymd commented on a change in pull request #4143:
URL: https://github.com/apache/zeppelin/pull/4143#discussion_r655416528



##########
File path: jdbc/src/main/java/org/apache/zeppelin/jdbc/JDBCInterpreter.java
##########
@@ -727,12 +727,14 @@ private InterpreterResult executeSql(String dbPrefix, String sql,
     try {
       List<String>  sqlArray = sqlSplitter.splitSql(sql);
       for (String sqlToExecute : sqlArray) {
-        LOGGER.info("Execute sql: " + sqlToExecute);
-        if (sqlToExecute.trim().toLowerCase().startsWith("set ")) {
+        if (sqlToExecute.trim().toLowerCase().startsWith("set ") ||
+                sqlToExecute.trim().toLowerCase().startsWith("list ") ||
+                sqlToExecute.trim().toLowerCase().startsWith("add ")) {

Review comment:
       It would be good to move `sqlToExecute.trim().toLowerCase()` out of `if` statement.
   Anyway, which sql language has `list` keyword?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [zeppelin] Reamer commented on pull request #4143: [ZEPPELIN-5311] Unable to run list & add hive statement

Posted by GitBox <gi...@apache.org>.
Reamer commented on pull request #4143:
URL: https://github.com/apache/zeppelin/pull/4143#issuecomment-867354438


   LGTM


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [zeppelin] asfgit closed pull request #4143: [ZEPPELIN-5311] Unable to run list & add hive statement

Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #4143:
URL: https://github.com/apache/zeppelin/pull/4143


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [zeppelin] cuspymd commented on a change in pull request #4143: [ZEPPELIN-5311] Unable to run list & add hive statement

Posted by GitBox <gi...@apache.org>.
cuspymd commented on a change in pull request #4143:
URL: https://github.com/apache/zeppelin/pull/4143#discussion_r656743061



##########
File path: jdbc/src/main/java/org/apache/zeppelin/jdbc/JDBCInterpreter.java
##########
@@ -727,12 +727,14 @@ private InterpreterResult executeSql(String dbPrefix, String sql,
     try {
       List<String>  sqlArray = sqlSplitter.splitSql(sql);
       for (String sqlToExecute : sqlArray) {
-        LOGGER.info("Execute sql: " + sqlToExecute);
-        if (sqlToExecute.trim().toLowerCase().startsWith("set ")) {
+        if (sqlToExecute.trim().toLowerCase().startsWith("set ") ||
+                sqlToExecute.trim().toLowerCase().startsWith("list ") ||
+                sqlToExecute.trim().toLowerCase().startsWith("add ")) {

Review comment:
       As [a Hive Manual](https://cwiki.apache.org/confluence/display/hive/languagemanual+cli), It seems be a command of the interpreter shell mode. There is other command like `DELETE` also. 




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [zeppelin] Reamer removed a comment on pull request #4143: [ZEPPELIN-5311] Unable to run list & add hive statement

Posted by GitBox <gi...@apache.org>.
Reamer removed a comment on pull request #4143:
URL: https://github.com/apache/zeppelin/pull/4143#issuecomment-867354438


   LGTM


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [zeppelin] zjffdu commented on a change in pull request #4143: [ZEPPELIN-5311] Unable to run list & add hive statement

Posted by GitBox <gi...@apache.org>.
zjffdu commented on a change in pull request #4143:
URL: https://github.com/apache/zeppelin/pull/4143#discussion_r657230968



##########
File path: jdbc/src/main/java/org/apache/zeppelin/jdbc/JDBCInterpreter.java
##########
@@ -727,12 +727,14 @@ private InterpreterResult executeSql(String dbPrefix, String sql,
     try {
       List<String>  sqlArray = sqlSplitter.splitSql(sql);
       for (String sqlToExecute : sqlArray) {
-        LOGGER.info("Execute sql: " + sqlToExecute);
-        if (sqlToExecute.trim().toLowerCase().startsWith("set ")) {
+        if (sqlToExecute.trim().toLowerCase().startsWith("set ") ||
+                sqlToExecute.trim().toLowerCase().startsWith("list ") ||
+                sqlToExecute.trim().toLowerCase().startsWith("add ")) {

Review comment:
       That's right @cuspymd Let me complete the full list.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org