-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
fix: do not escape CSV output #24311
Conversation
CSV output is incorrectly escaped. Add a boolean flag to tag output functions to prevent this. closes #24309
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
Tests against Plutonium will probably fail after merge though, there's a (single) invocation of tags.HashKey
in plutonium's storage/merge/stream_reader_test.go
which'll need updating to pass true
.
@btasker - Thanks, will create plutonium issues for master and 1.11 branches to
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a definite improvement and addresses the original issue. One thing to note is that if a tag can contain a comma, double quote, or CR/LF then that field will require quoting. I'm not sure if it's even possible to get tags with those characters in it. I don't think we should attempt to address CSV quoting until we actually find an issue.
CSV output is incorrectly escaped. Add a boolean flag to tag output functions to prevent this. closes influxdata#24309
CSV output is incorrectly escaped. Add a boolean flag to tag output functions to prevent this. closes influxdata#24309
CSV output is incorrectly escaped.
Add a boolean flag to tag output
functions to prevent this.
closes #24309