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

[KYUUBI #4305][Bug] Backport HIVE-15820: comment at the head of beeline -e #4333

Closed
wants to merge 4 commits into from

Conversation

lsm1
Copy link
Contributor

@lsm1 lsm1 commented Feb 15, 2023

Why are the changes needed?

close #4305

manual tests

bin/beeline -u jdbc:hive2://X:10009 -e "
--asd
select 1 as a
"

image

How was this patch tested?

  • Add some test cases that check the changes thoroughly including negative and positive cases if possible

  • Add screenshots for manual tests if appropriate

  • Run test locally before make a pull request

@bowenliang123
Copy link
Contributor

bowenliang123 commented Feb 15, 2023

Can you add some unit tests for testing single comment line and multi single-line comments ?

@pan3793 pan3793 requested a review from cfmcgrady February 15, 2023 04:17
@codecov-commenter
Copy link

codecov-commenter commented Feb 15, 2023

Codecov Report

Merging #4333 (9a071fb) into master (3b0137a) will increase coverage by 0.12%.
The diff coverage is 0.00%.

@@             Coverage Diff              @@
##             master    #4333      +/-   ##
============================================
+ Coverage     53.40%   53.53%   +0.12%     
  Complexity       13       13              
============================================
  Files           560      562       +2     
  Lines         30569    30713     +144     
  Branches       4139     4143       +4     
============================================
+ Hits          16326    16441     +115     
- Misses        12706    12726      +20     
- Partials       1537     1546       +9     
Impacted Files Coverage Δ
...in/java/org/apache/hive/beeline/KyuubiBeeLine.java 0.00% <0.00%> (ø)
...n/java/org/apache/hive/beeline/KyuubiCommands.java 0.00% <0.00%> (ø)
...ain/scala/org/apache/kyuubi/util/RowSetUtils.scala 26.08% <0.00%> (-57.25%) ⬇️
.../kyuubi/server/mysql/constant/MySQLErrorCode.scala 13.84% <0.00%> (-6.16%) ⬇️
...ache/kyuubi/server/mysql/MySQLCommandHandler.scala 77.77% <0.00%> (-4.05%) ⬇️
...ache/kyuubi/sql/plan/trino/TrinoFeOperations.scala 39.13% <0.00%> (-3.73%) ⬇️
...g/apache/kyuubi/operation/BatchJobSubmission.scala 75.27% <0.00%> (-2.20%) ⬇️
...ache/kyuubi/server/mysql/MySQLGenericPackets.scala 76.59% <0.00%> (-2.13%) ⬇️
...kyuubi/engine/spark/operation/SparkOperation.scala 76.31% <0.00%> (-0.40%) ⬇️
...main/java/org/apache/kyuubi/client/RestClient.java 85.71% <0.00%> (ø)
... and 20 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@bowenliang123
Copy link
Contributor

bowenliang123 commented Feb 15, 2023

How about ut for multi single-line comments (also with some space in front)?
"--comment line 1 \n --comment line 2 \n show database;"

@bowenliang123
Copy link
Contributor

Run dev/reformat to fix the code style violations.

@pan3793
Copy link
Member

pan3793 commented Feb 16, 2023

@cfmcgrady would you please check will this change breaks the pyspark case?

@cfmcgrady
Copy link
Contributor

@cfmcgrady would you please check will this change breaks the pyspark case?

I think this should be a breaking change to run python snippets. I will check when I have time.

@cfmcgrady
Copy link
Contributor

test run multi-line python snippets with this change.

0: jdbc:hive2://192.168.206.48:10009/> for i in [1, 2, 3]:
. . . . . . . . . . . . . . . . . . .>     print(i);
2023-02-16 17:00:50.824 INFO org.apache.kyuubi.operation.ExecuteStatement: Processing anonymous's query[1215c099-cf9a-4c42-80c1-5290a456b8ea]: PENDING_STATE -> RUNNING_STATE, statement:
for i in [1, 2, 3]:
    print(i)
23/02/16 17:00:50 INFO ExecutePython: Processing anonymous's query[fdc19881-4918-47f3-8b39-c1e97d01269b]: PENDING_STATE -> RUNNING_STATE, statement:
for i in [1, 2, 3]:
    print(i)
23/02/16 17:00:50 INFO ExecutePython:
           Spark application name: kyuubi_USER_SPARK_SQL_anonymous_default_a5f44f5d-d3ec-49e1-a594-8b0460e9728d
                 application ID: local-1676537993814
                 application web UI: http://192.168.206.48:65528
                 master: local[*]
                 deploy mode: client
                 version: 3.3.1
           Start time: 2023-02-16T16:59:50.066
           User: anonymous
23/02/16 17:00:50 INFO ExecutePython: Processing anonymous's query[fdc19881-4918-47f3-8b39-c1e97d01269b]: RUNNING_STATE -> FINISHED_STATE, time taken: 0.002 seconds
2023-02-16 17:00:50.836 INFO org.apache.kyuubi.operation.ExecuteStatement: Query[1215c099-cf9a-4c42-80c1-5290a456b8ea] in FINISHED_STATE
2023-02-16 17:00:50.836 INFO org.apache.kyuubi.operation.ExecuteStatement: Processing anonymous's query[1215c099-cf9a-4c42-80c1-5290a456b8ea]: RUNNING_STATE -> FINISHED_STATE, time taken: 0.011 seconds
+---------+---------+--------+---------+------------+
| output  | status  | ename  | evalue  | traceback  |
+---------+---------+--------+---------+------------+
| 1
2
3   | ok      |        |         | []         |
+---------+---------+--------+---------+------------+
1 row selected (0.06 seconds)

@cfmcgrady cfmcgrady added this to the v1.6.2 milestone Feb 16, 2023
@cfmcgrady
Copy link
Contributor

thanks, merging to master(v1.8.0)/branch-1.7(v1.7.0)/branch-1.6(v1.6.2)

@cfmcgrady cfmcgrady closed this in c489e29 Feb 16, 2023
cfmcgrady pushed a commit that referenced this pull request Feb 16, 2023
…ne -e

### _Why are the changes needed?_

close [#4305](#4305)

### manual tests

```sql
bin/beeline -u jdbc:hive2://X:10009 -e "
--asd
select 1 as a
"
```

![image](https://user-images.githubusercontent.com/18713676/218910222-b829d447-e5b7-4d80-842b-2ddd4f47a26d.png)

### _How was this patch tested?_
- [ ] Add some test cases that check the changes thoroughly including negative and positive cases if possible

- [x] Add screenshots for manual tests if appropriate

- [ ] [Run test](https://kyuubi.readthedocs.io/en/master/develop_tools/testing.html#running-tests) locally before make a pull request

Closes #4333 from lsm1/fix/beeline_comment_header.

Closes #4305

f932d4e [senmiaoliu] reformat
9a071fb [senmiaoliu] added multi line ut
425c536 [senmiaoliu] added ut
d4dc21a [senmiaoliu] comment at the head of beeline -e

Authored-by: senmiaoliu <senmiaoliu@trip.com>
Signed-off-by: Fu Chen <cfmcgrady@gmail.com>
(cherry picked from commit c489e29)
Signed-off-by: Fu Chen <cfmcgrady@gmail.com>
cfmcgrady pushed a commit that referenced this pull request Feb 16, 2023
…ne -e

close [#4305](#4305)

```sql
bin/beeline -u jdbc:hive2://X:10009 -e "
--asd
select 1 as a
"
```

![image](https://user-images.githubusercontent.com/18713676/218910222-b829d447-e5b7-4d80-842b-2ddd4f47a26d.png)

- [ ] Add some test cases that check the changes thoroughly including negative and positive cases if possible

- [x] Add screenshots for manual tests if appropriate

- [ ] [Run test](https://kyuubi.readthedocs.io/en/master/develop_tools/testing.html#running-tests) locally before make a pull request

Closes #4333 from lsm1/fix/beeline_comment_header.

Closes #4305

f932d4e [senmiaoliu] reformat
9a071fb [senmiaoliu] added multi line ut
425c536 [senmiaoliu] added ut
d4dc21a [senmiaoliu] comment at the head of beeline -e

Authored-by: senmiaoliu <senmiaoliu@trip.com>
Signed-off-by: Fu Chen <cfmcgrady@gmail.com>
(cherry picked from commit c489e29)
Signed-off-by: Fu Chen <cfmcgrady@gmail.com>
@cfmcgrady
Copy link
Contributor

sorry for wrongly locally testing before, this is a breaking change for submitting multi-line python snippets.

log
0: jdbc:hive2://192.168.205.128:10009/> for i in [1, 2, 3]:
. . . . . . . . . . . . . . . . . . . >     print(i);
2023-02-23 17:26:12.280 INFO org.apache.kyuubi.operation.ExecuteStatement: Processing anonymous's query[d70621de-5a0b-4253-8f3f-a4c51cacc806]: PENDING_STATE -> RUNNING_STATE, statement:
for i in [1, 2, 3]:
print(i)
23/02/23 17:26:13 INFO ExecutePython: Processing anonymous's query[209c8f29-3e06-4ce6-9271-f2ce396741ba]: PENDING_STATE -> RUNNING_STATE, statement:
for i in [1, 2, 3]:
print(i)
23/02/23 17:26:13 INFO ExecutePython:
           Spark application name: kyuubi_USER_SPARK_SQL_anonymous_default_f7512afe-f0b3-449e-bf45-164af364616b
                 application ID: local-1677144339509
                 application web UI: http://192.168.205.128:59896
                 master: local[*]
                 deploy mode: client
                 version: 3.3.1
           Start time: 2023-02-23T17:25:37.933
           User: anonymous
23/02/23 17:26:13 INFO DAGScheduler: Asked to cancel job group 209c8f29-3e06-4ce6-9271-f2ce396741ba
23/02/23 17:26:13 ERROR ExecutePython: Error operating ExecutePython: org.apache.kyuubi.KyuubiSQLException: Interpret error:
for i in [1, 2, 3]:
print(i)
 Some(PythonResponse(execute_reply,PythonResponseContent(null,IndentationError,expected an indented block (<stdin>, line 2),List(  File "<stdin>", line 2
,     print(i)
,         ^
, IndentationError: expected an indented block
),error)))
        at org.apache.kyuubi.KyuubiSQLException$.apply(KyuubiSQLException.scala:69)
        at org.apache.kyuubi.engine.spark.operation.ExecutePython.$anonfun$executePython$1(ExecutePython.scala:96)
        at scala.runtime.java8.JFunction0$mcV$sp.apply(JFunction0$mcV$sp.java:23)
        at org.apache.kyuubi.engine.spark.operation.ExecutePython.withLocalProperties(ExecutePython.scala:160)
        at org.apache.kyuubi.engine.spark.operation.ExecutePython.org$apache$kyuubi$engine$spark$operation$ExecutePython$$executePython(ExecutePython.scala:81)
        at org.apache.kyuubi.engine.spark.operation.ExecutePython$$anon$1.run(ExecutePython.scala:111)
        at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
        at java.util.concurrent.FutureTask.run(FutureTask.java:266)
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
        at java.lang.Thread.run(Thread.java:748)

org.apache.kyuubi.KyuubiSQLException: Interpret error:
for i in [1, 2, 3]:
print(i)
 Some(PythonResponse(execute_reply,PythonResponseContent(null,IndentationError,expected an indented block (<stdin>, line 2),List(  File "<stdin>", line 2
,     print(i)
,         ^
, IndentationError: expected an indented block
),error)))
        at org.apache.kyuubi.KyuubiSQLException$.apply(KyuubiSQLException.scala:69)
        at org.apache.kyuubi.engine.spark.operation.ExecutePython.$anonfun$executePython$1(ExecutePython.scala:96)
        at scala.runtime.java8.JFunction0$mcV$sp.apply(JFunction0$mcV$sp.java:23)
        at org.apache.kyuubi.engine.spark.operation.ExecutePython.withLocalProperties(ExecutePython.scala:160)
        at org.apache.kyuubi.engine.spark.operation.ExecutePython.org$apache$kyuubi$engine$spark$operation$ExecutePython$$executePython(ExecutePython.scala:81)
        at org.apache.kyuubi.engine.spark.operation.ExecutePython$$anon$1.run(ExecutePython.scala:111)
        at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
        at java.util.concurrent.FutureTask.run(FutureTask.java:266)
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
        at java.lang.Thread.run(Thread.java:748)
23/02/23 17:26:13 INFO ExecutePython: Processing anonymous's query[209c8f29-3e06-4ce6-9271-f2ce396741ba]: RUNNING_STATE -> ERROR_STATE, time taken: 0.01 seconds
2023-02-23 17:26:13.096 INFO org.apache.kyuubi.operation.ExecuteStatement: Query[d70621de-5a0b-4253-8f3f-a4c51cacc806] in ERROR_STATE
2023-02-23 17:26:13.103 INFO org.apache.kyuubi.operation.ExecuteStatement: Processing anonymous's query[d70621de-5a0b-4253-8f3f-a4c51cacc806]: RUNNING_STATE -> ERROR_STATE, time taken: 0.823 seconds
Error: org.apache.kyuubi.KyuubiSQLException: org.apache.kyuubi.KyuubiSQLException: Error operating ExecutePython: org.apache.kyuubi.KyuubiSQLException: Interpret error:
for i in [1, 2, 3]:
print(i)
 Some(PythonResponse(execute_reply,PythonResponseContent(null,IndentationError,expected an indented block (<stdin>, line 2),List(  File "<stdin>", line 2
,     print(i)
,         ^
, IndentationError: expected an indented block
),error)))
        at org.apache.kyuubi.KyuubiSQLException$.apply(KyuubiSQLException.scala:69)
        at org.apache.kyuubi.engine.spark.operation.ExecutePython.$anonfun$executePython$1(ExecutePython.scala:96)
        at scala.runtime.java8.JFunction0$mcV$sp.apply(JFunction0$mcV$sp.java:23)
        at org.apache.kyuubi.engine.spark.operation.ExecutePython.withLocalProperties(ExecutePython.scala:160)
        at org.apache.kyuubi.engine.spark.operation.ExecutePython.org$apache$kyuubi$engine$spark$operation$ExecutePython$$executePython(ExecutePython.scala:81)
        at org.apache.kyuubi.engine.spark.operation.ExecutePython$$anon$1.run(ExecutePython.scala:111)
        at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
        at java.util.concurrent.FutureTask.run(FutureTask.java:266)
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
        at java.lang.Thread.run(Thread.java:748)

        at org.apache.kyuubi.KyuubiSQLException$.apply(KyuubiSQLException.scala:69)
        at org.apache.kyuubi.engine.spark.operation.SparkOperation$$anonfun$onError$1.applyOrElse(SparkOperation.scala:188)
        at org.apache.kyuubi.engine.spark.operation.SparkOperation$$anonfun$onError$1.applyOrElse(SparkOperation.scala:172)
        at scala.runtime.AbstractPartialFunction.apply(AbstractPartialFunction.scala:38)
        at org.apache.kyuubi.engine.spark.operation.ExecutePython.$anonfun$executePython$1(ExecutePython.scala:99)
        at scala.runtime.java8.JFunction0$mcV$sp.apply(JFunction0$mcV$sp.java:23)
        at org.apache.kyuubi.engine.spark.operation.ExecutePython.withLocalProperties(ExecutePython.scala:160)
        at org.apache.kyuubi.engine.spark.operation.ExecutePython.org$apache$kyuubi$engine$spark$operation$ExecutePython$$executePython(ExecutePython.scala:81)
        at org.apache.kyuubi.engine.spark.operation.ExecutePython$$anon$1.run(ExecutePython.scala:111)
        at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
        at java.util.concurrent.FutureTask.run(FutureTask.java:266)
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
        at java.lang.Thread.run(Thread.java:748)
Caused by: org.apache.kyuubi.KyuubiSQLException: Interpret error:
for i in [1, 2, 3]:
print(i)
 Some(PythonResponse(execute_reply,PythonResponseContent(null,IndentationError,expected an indented block (<stdin>, line 2),List(  File "<stdin>", line 2
,     print(i)
,         ^
, IndentationError: expected an indented block
),error)))
        at org.apache.kyuubi.KyuubiSQLException$.apply(KyuubiSQLException.scala:69)
        at org.apache.kyuubi.engine.spark.operation.ExecutePython.$anonfun$executePython$1(ExecutePython.scala:96)
        ... 9 more

        at org.apache.kyuubi.KyuubiSQLException$.apply(KyuubiSQLException.scala:69)
        at org.apache.kyuubi.operation.ExecuteStatement.waitStatementComplete(ExecuteStatement.scala:129)
        at org.apache.kyuubi.operation.ExecuteStatement.$anonfun$runInternal$1(ExecuteStatement.scala:161)
        at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
        at java.util.concurrent.FutureTask.run(FutureTask.java:266)
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
        at java.lang.Thread.run(Thread.java:748) (state=,code=0)

@pan3793
Copy link
Member

pan3793 commented Feb 23, 2023

@cfmcgrady, any solution? we should revert it if we can not fix it properly

@cfmcgrady
Copy link
Contributor

@cfmcgrady, any solution? we should revert it if we can not fix it properly

I don't have any thoughts on this issue, maybe we should revert it first?

@cxzl25
Copy link
Contributor

cxzl25 commented Feb 24, 2023

sorry for wrongly locally testing before, this is a breaking change for submitting multi-line python snippets.

We can add this python UT later?

pan3793 pushed a commit that referenced this pull request Feb 24, 2023
…nt at the head …

…of beeline -e"

This reverts commit c489e29.

### _Why are the changes needed?_

#4333 (comment)

### _How was this patch tested?_
- [ ] Add some test cases that check the changes thoroughly including negative and positive cases if possible

- [ ] Add screenshots for manual tests if appropriate

- [ ] [Run test](https://kyuubi.readthedocs.io/en/master/develop_tools/testing.html#running-tests) locally before make a pull request

Closes #4406 from cfmcgrady/revert-4333.

Closes #4406

Closes #4305

c7fff72 [Fu Chen] Revert "[KYUUBI #4305][Bug] Backport HIVE-15820: comment at the head of beeline -e"

Authored-by: Fu Chen <cfmcgrady@gmail.com>
Signed-off-by: Cheng Pan <chengpan@apache.org>
pan3793 pushed a commit that referenced this pull request Feb 24, 2023
…nt at the head …

…of beeline -e"

This reverts commit c489e29.

### _Why are the changes needed?_

#4333 (comment)

### _How was this patch tested?_
- [ ] Add some test cases that check the changes thoroughly including negative and positive cases if possible

- [ ] Add screenshots for manual tests if appropriate

- [ ] [Run test](https://kyuubi.readthedocs.io/en/master/develop_tools/testing.html#running-tests) locally before make a pull request

Closes #4406 from cfmcgrady/revert-4333.

Closes #4406

Closes #4305

c7fff72 [Fu Chen] Revert "[KYUUBI #4305][Bug] Backport HIVE-15820: comment at the head of beeline -e"

Authored-by: Fu Chen <cfmcgrady@gmail.com>
Signed-off-by: Cheng Pan <chengpan@apache.org>
(cherry picked from commit b39caed)
Signed-off-by: Cheng Pan <chengpan@apache.org>
cfmcgrady added a commit that referenced this pull request Mar 22, 2023
### _Why are the changes needed?_

to resolve #4333 (comment)

### _How was this patch tested?_
- [ ] Add some test cases that check the changes thoroughly including negative and positive cases if possible

- [ ] Add screenshots for manual tests if appropriate

- [x] [Run test](https://kyuubi.readthedocs.io/en/master/develop_tools/testing.html#running-tests) locally before make a pull request

Closes #4581 from cfmcgrady/beeline-parser-python-ut.

Closes #4581

9bffa03 [Fu Chen] add KyuubiCommands parse python snippets unit test

Authored-by: Fu Chen <cfmcgrady@gmail.com>
Signed-off-by: Fu Chen <cfmcgrady@gmail.com>
cfmcgrady added a commit that referenced this pull request Mar 22, 2023
to resolve #4333 (comment)

- [ ] Add some test cases that check the changes thoroughly including negative and positive cases if possible

- [ ] Add screenshots for manual tests if appropriate

- [x] [Run test](https://kyuubi.readthedocs.io/en/master/develop_tools/testing.html#running-tests) locally before make a pull request

Closes #4581 from cfmcgrady/beeline-parser-python-ut.

Closes #4581

9bffa03 [Fu Chen] add KyuubiCommands parse python snippets unit test

Authored-by: Fu Chen <cfmcgrady@gmail.com>
Signed-off-by: Fu Chen <cfmcgrady@gmail.com>
(cherry picked from commit acdfa6c)
Signed-off-by: Fu Chen <cfmcgrady@gmail.com>
Kiss736921 pushed a commit to Kiss736921/kyuubi that referenced this pull request Mar 23, 2023
…unit test

### _Why are the changes needed?_

to resolve apache#4333 (comment)

### _How was this patch tested?_
- [ ] Add some test cases that check the changes thoroughly including negative and positive cases if possible

- [ ] Add screenshots for manual tests if appropriate

- [x] [Run test](https://kyuubi.readthedocs.io/en/master/develop_tools/testing.html#running-tests) locally before make a pull request

Closes apache#4581 from cfmcgrady/beeline-parser-python-ut.

Closes apache#4581

9bffa03 [Fu Chen] add KyuubiCommands parse python snippets unit test

Authored-by: Fu Chen <cfmcgrady@gmail.com>
Signed-off-by: Fu Chen <cfmcgrady@gmail.com>
turboFei added a commit that referenced this pull request Jun 16, 2023
…ne -e

### _Why are the changes needed?_

Backport apache/hive#1814

related kyuubi issues
#4305
#4333
#4406

### _How was this patch tested?_
- [x] Add some test cases that check the changes thoroughly including negative and positive cases if possible

- [ ] Add screenshots for manual tests if appropriate

- [x] [Run test](https://kyuubi.readthedocs.io/en/master/develop_tools/testing.html#running-tests) locally before make a pull request

Closes #4972 from turboFei/beeline_head_command.

Closes #4305

04b235f [fwang12] [KYUUBI #4305][Bug] Backport HIVE-15820: comment at the head of beeline -e

Authored-by: fwang12 <fwang12@ebay.com>
Signed-off-by: fwang12 <fwang12@ebay.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] Backport HIVE-15820: comment at the head of beeline -e
6 participants