Skip to content

Conversation

@alexjbush
Copy link
Contributor

@alexjbush alexjbush commented Dec 8, 2017

What is this PR for?

This PR fixes the issue of newlines and tabs breaking results in the SQL interpreter in Livy.

The Livy interpreter will return incorrect results if a row contains \n or \t characters.
In the case of the newline, the result will be:
Line is missing from results if the \n appears anywhere but the end of a cell
String index out of range: 17 if it appears at the end of a cell
In the case of the tab, the result will be misaligned columns if the tab appears in the middle of a cell
The output showing these error is attached to the Jira.

I have changed the parsing and any newline or tab characters will be escaped

What type of PR is it?

Bug Fix

What is the Jira issue?

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

How should this be tested?

Unittests have been added

Screenshots (if appropriate)

Questions:

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

Copy link
Member

@felixcheung felixcheung left a comment

Choose a reason for hiding this comment

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

@zjffdu
Copy link
Contributor

zjffdu commented Dec 11, 2017

@bushnoh thanks for the contribution, could you check the CI, some tests fails.

@alexjbush
Copy link
Contributor Author

@zjffdu The CI errors look unrelated. They're for the IPythonInterpreter and are the same ones that are failing on master.

@zjffdu
Copy link
Contributor

zjffdu commented Dec 13, 2017

@bushnoh I have fixed the CI failure, could you rebase this PR and retrigger the travis build ?

@alexjbush
Copy link
Contributor Author

@zjffdu I have merged in the changed from master however there are still CI failures. They are again both unrelated and are the ones that are happening on master: https://travis-ci.org/apache/zeppelin/builds/315822086

@zjffdu
Copy link
Contributor

zjffdu commented Dec 14, 2017

@bushnoh https://travis-ci.org/apache/zeppelin/builds/315822086 This build doesn't include your PR, and some flaky test is known issue of zeppelin, but IPython related failure should be fixed.

@alexjbush
Copy link
Contributor Author

@zjffdu I was trying to show that the tests failing in master (that don't include my changes) are the same ones with the same errors that are failing on my branch with the changes

@felixcheung
Copy link
Member

I think test passed? it's too long and jenkins no longer has the build
@zjffdu

Merge latest changes from master
@alexjbush
Copy link
Contributor Author

@zjffdu @felixcheung I merged changes from master into the branch to trigger another build however it looks like master is failing in Jenkins again. The previous build did pass though.

@zjffdu
Copy link
Contributor

zjffdu commented Jan 11, 2018

@bushnoh Travis build issue has been fixed, could you retrigger the build of this PR ?

@asfgit asfgit closed this in 2d36f84 Jan 19, 2018
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.

3 participants