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-5898]fix:down csv data error #4591

Closed
wants to merge 0 commits into from

Conversation

zhugezifang
Copy link
Contributor

@zhugezifang zhugezifang commented Apr 26, 2023

What is this PR for?

This pull request fix the bug of download csv data error

What type of PR is it?

Bug Fix

Todos

  • - Task

What is the Jira issue?

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

How should this be tested?

  • CI

Screenshots (if appropriate)

Questions:

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

@Reamer
Copy link
Contributor

Reamer commented Apr 27, 2023

Still

Thanks for the contribution @zhugezifang , could you update the PR description?

@zhugezifang
Copy link
Contributor Author

Still

Thanks for the contribution @zhugezifang , could you update the PR description?

ok,updated it

@Reamer
Copy link
Contributor

Reamer commented Apr 27, 2023

I do not understand the fix. Could you please explain it.

@zhugezifang
Copy link
Contributor Author

zhugezifang commented Apr 28, 2023

I do not understand the fix. Could you please explain it.

let we look the example of https://issues.apache.org/jira/browse/ZEPPELIN-5898

when i query the result in zeppelin,the result is

image

and click download
image

but the data of downloaded has problems

image

the column of params_value is json,but open in csv, it is not ,so it has a bug

this pr is to fix the bug

you can reappearanced it if the data has a column of josn data

@zhugezifang zhugezifang reopened this Apr 28, 2023
@zhugezifang
Copy link
Contributor Author

zhugezifang commented Apr 28, 2023

I do not understand the fix. Could you please explain it.

excuce me,there is a webchat group or other group of zeppelin,we can discuss it more timely。

and i want to add the feature to zeppelin ,it is a brief description in https://issues.apache.org/jira/browse/ZEPPELIN-5896

i discuss the feature with @zjffdu ,he say i can try it

@Reamer
Copy link
Contributor

Reamer commented Apr 28, 2023

I don't think it's a specific JSON problem, but rather a delimiter problem.
The CSV is broken because in the JSON example there is a , in the text field and this is also used as a delimiter.
What do you think about it?

@zhugezifang
Copy link
Contributor Author

I don't think it's a specific JSON problem, but rather a delimiter problem. The CSV is broken because in the JSON example there is a , in the text field and this is also used as a delimiter. What do you think about it?

yes ,the code solve the ',' of delimiter, but it solved not perfect,it also exist bug

image

@zhugezifang
Copy link
Contributor Author

I do not understand the fix. Could you please explain it.

excuce me,there is a webchat group or other group of zeppelin,we can discuss it more timely。

and i want to add the feature to zeppelin ,it is a brief description in https://issues.apache.org/jira/browse/ZEPPELIN-5896

i discuss the feature with @zjffdu ,he say i can try it

@Reamer hi,i also want to add this feature into zeppelin,and could I get your support?

@Reamer
Copy link
Contributor

Reamer commented Apr 28, 2023

@Reamer hi,i also want to add this feature into zeppelin,and could I get your support?

Of course. I was able to reproduce the problem and understood the problem. Your change definitely solves the problem. It's just not a JSON specific problem, but a general masking problem.

I would remove the if statement. It is not necessary because replaceAll also looks for the quotes.

// Mask all quotes
stringValue = stringValue.replaceAll('"', '""');

@zhugezifang
Copy link
Contributor Author

@Reamer hi,i also want to add this feature into zeppelin,and could I get your support?

Of course. I was able to reproduce the problem and understood the problem. Your change definitely solves the problem. It's just not a JSON specific problem, but a general masking problem.

I would remove the if statement. It is not necessary because replaceAll also looks for the quotes.

// Mask all quotes
stringValue = stringValue.replaceAll('"', '""');

ok,i update it later and then add a new pr

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants