Repository navigation
GH-54535: [C++] Quote nulls in single-column CSV output so rows aren't written as empty lines - #54683
CaptainAni187 wants to merge 2 commits into
Conversation
|
|
|
|
|
|
||
| const std::string null_string_value = | ||
| schema->num_fields() == 1 && options.null_string.empty() && | ||
| options.quoting_style != QuotingStyle::None |
There was a problem hiding this comment.
What if we write ["a", null, ""] with all_valid?
AllValid is documented to leave nulls unquoted. This change writes both null and the empty string as "", so a reader cannot distinguish them.
Can we add a round-trip test with strings_can_be_null=true, quoted_strings_can_be_null=false, and ignore_empty_lines=false, and preserve the distinction?
There was a problem hiding this comment.
Good point, thanks. A single string or binary column now keeps the empty line for nulls, since "" is already what a valid empty string becomes there. The "" for nulls only applies when the column's values can never be written as "" (ints, dates, etc.). Added the round trip you described: ["a", null, ""] written with Needed and AllValid and read back with strings_can_be_null=true, quoted_strings_can_be_null=false and ignore_empty_lines=false keeps all three values. Also adjusted the AllValid doc to point at the exception.
github-actions
Bot
added
awaiting committer review
… aren't written as empty lines
A valid empty string is already written as "", so writing a null the same way made the two indistinguishable. Only quote nulls when the column's valid values can never be written as "".
b6fe5f9 to
4b80c3f
Compare
|
|
2 similar comments
|
|
|
|
Rationale for this change
With a one-column table, a null gets written as the empty null_string followed by the eol, so the row is just an empty line. The CSV reader skips empty lines by default, so every null row is lost on a write_csv / read_csv round trip.
What changes are included in this PR?
When the schema has a single column, null_string is empty and the column's valid values can never be written as
"", the writer now writes nulls as""instead of nothing. A quoted empty value reads back as null for those columns with the default options (quoted_strings_can_be_null).String and binary columns (and zero-width fixed size binary) keep the empty line: a valid empty string is already written as
""there, so quoting the null too would make the two indistinguishable. A custom null_string, tables with more than one column, and QuotingStyle::None also keep the old output. I left None alone since it promises no quotes at all.... happy to change that if you'd rather it also writes"".Also updated the docs on WriteOptions::null_string and QuotingStyle::AllValid.
Are these changes tested?
Yes. Updated the existing SingleColumnWriteCSVTest case (it was asserting the empty line), added cases for Needed, AllValid, None, a custom null_string, string and dictionary string columns and two columns. Round trip tests write a one-column int64 table with nulls and read it back with default options, and write a string column
["a", null, ""]and read it back with ignore_empty_lines=false, strings_can_be_null=true and quoted_strings_can_be_null=false, for both Needed and AllValid.Are there any user-facing changes?
Yes, single-column CSV output with nulls changes from an empty line to
""for non-string columns (with the default empty null_string).Was AI used for this PR?
In accordance to the AI generation guidelines, please disclose below whether and how AI was used in this PR.
The fix, tests and this description were drafted with Claude Code.
PR code and description written by:
Reviewed before submission by:
read_csvskips by default, so null rows are lost on a round trip #54535