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

[ZEPPELIN-5798] Add multiline sql statement support for Livy Spark SQL interpreter #4443

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

zkytech
Copy link
Contributor

@zkytech zkytech commented Aug 14, 2022

What is this PR for?

  1. Add multiline sql statement support for Livy Spark SQL interpreter
  2. Bug Fix
  3. Bug Fix

What type of PR is it?

Feature & Bug Fix

What is the Jira issue?

https://issues.apache.org/jira/browse/ZEPPELIN-5798

How should this be tested?

execute multiline sql statement with ";", for example:

show databases;
use xxxx;
show tables;

Questions:

  • Does the licenses files need to update? No
  • Is there breaking changes for older versions? No
  • Does this needs documentation? No

@zkytech
Copy link
Contributor Author

zkytech commented Aug 15, 2022

I have found a new bug in this file.

message data like this will fail with exception com.google.gson.JsonSyntaxException: java.lang.IllegalStateException: Expected a string but was BEGIN_OBJECT at line 1 column 9 path $.

df: org.apache.spark.sql.DataFrame = [_id: struct<oid: string>, pv: bigint ... 2 more fields]
_id	pv	stats_date	uv	
{"_id":{"oid":"6295fdbd458288396d008163"},"pv":0,"stats_date":"20220530","uv":0}

which is caused by code in this place

I will fix this later.

@zjffdu
Copy link
Contributor

zjffdu commented Aug 31, 2022

ping @zkytech

@zkytech
Copy link
Contributor Author

zkytech commented Sep 1, 2022

ping @zkytech

What should I do next?

@zjffdu
Copy link
Contributor

zjffdu commented Sep 1, 2022

@zkytech Have you resolved the issue in the above comment ?#4443 (comment)

@zkytech
Copy link
Contributor Author

zkytech commented Sep 1, 2022

@zkytech Have you resolved the issue in the above comment ?#4443 (comment)

Yes, It has been fixed in this commit c31945d

The url in commit message is wrong, it should be https://github.com/apache/zeppelin/pull/4443#issuecomment-1214710538,

@zjffdu
Copy link
Contributor

zjffdu commented Sep 24, 2022

Sorry for late response @zkytech Could you retrigger the CI?

@zkytech
Copy link
Contributor Author

zkytech commented Sep 24, 2022

Sorry for late response @zkytech Could you retrigger the CI?

ok

@zjffdu
Copy link
Contributor

zjffdu commented Oct 10, 2022

@zkytech livy interpreter in CI is failed, could help fix it? LivySparkSQLInterpreter

@zkytech
Copy link
Contributor Author

zkytech commented Oct 12, 2022

@zjffdu fixed

@Reamer
Copy link
Contributor

Reamer commented Oct 12, 2022

Please rebase your PR to current master.

@zkytech
Copy link
Contributor Author

zkytech commented Oct 13, 2022

@Reamer Done

@Reamer
Copy link
Contributor

Reamer commented Oct 13, 2022

@Reamer Done

Let's wait for CI tests

@zkytech
Copy link
Contributor Author

zkytech commented Oct 14, 2022

Unit test is required to update. Because JSON number is no longer treat as string in this place

Add test case for multi level json string
@zkytech
Copy link
Contributor Author

zkytech commented Oct 14, 2022

@zjffdu To fix this bug, I have made a change in this place, JSON number is no longer treat as string . Since JSON doesn't distinguish between integer and floating point fields, I have made a change to unit test, all number in json will be convert to float value.

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.

4 participants