Skip to content

[format] Escape the escape character when writing CSV - #9278

Open
PDGGK wants to merge 1 commit into
apache:masterfrom
PDGGK:fix-csv-escape-character
Open

[format] Escape the escape character when writing CSV#9278
PDGGK wants to merge 1 commit into
apache:masterfrom
PDGGK:fix-csv-escape-character

Conversation

@PDGGK

@PDGGK PDGGK commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Purpose

The CSV writer and the CSV reader disagree about the escape character, so a value containing one does not survive a round trip through Paimon's own CSV format. With default options (csv.escape-character = \) — written, then read back:

written read back
Special\Characters SpecialCharacters
trailing\ trailing
a,b\ null
\\double double

Nothing is logged and nothing throws.

CsvFormatWriter.escapeField decides whether a field needs quoting, and escapes the quote character inside it — but the escape character is in neither step:

// CsvFormatWriter:103-116
boolean needsQuoting =
        field.indexOf(csvOptions.fieldDelimiter().charAt(0)) >= 0
                || field.indexOf(csvOptions.lineDelimiter().charAt(0)) >= 0
                || field.indexOf(csvOptions.quoteCharacter().charAt(0)) >= 0;

if (!needsQuoting) {
    return field;
}

String escaped =
        field.replace(
                csvOptions.quoteCharacter(),
                csvOptions.escapeCharacter() + csvOptions.quoteCharacter());
return csvOptions.quoteCharacter() + escaped + csvOptions.quoteCharacter();

CsvParser consumes an escape character unconditionally — it is only ever appended to the buffer when the character after it is a quote or another escape:

// CsvParser:108-118
if (c == escapeChar) {
    if (inQuotes || inField) {
        int nextCharacter = peekNextCharacter(line, position);
        if (nextCharacter == quoteChar || nextCharacter == escapeChar) {
            buffer.append(line.charAt(position + 1));
            position++;
        }
    }
} else if (c == quoteChar) {

So each row above fails a little differently:

  • Special\Characters and trailing\ are written verbatim, because a lone escape character does not trigger quoting. The reader then drops it.
  • \\double is written verbatim too, and both escape characters are lost — at the start of a field inQuotes || inField is still false, so the reader does not even take the "escaped escape" branch.
  • a,b\ does get quoted, for the comma. It is written "a,b\", the trailing \ is not doubled, and the reader takes the closing quote to be an escaped literal. The field never terminates and the row comes back with a null.

Why the existing test is green

testCsvEscapeCharacterWriteRead writes exactly this value and then does not look at it:

GenericRow.of(3, BinaryString.fromString("Special\\Characters"))
...
assertThat(result.get(2).getInt(0)).isEqualTo(3);

Three rows are written, and only the middle one — "Normal Value", which contains nothing that needs escaping — has its string asserted. Rows 1 and 3, the two carrying quotes and a backslash, are checked on their int column alone. This PR adds the two missing assertions; with the writer unchanged, the existing test then fails.

What changes

Writer only. Quote a field that contains the escape character, and escape the escape character before the quotes:

String quote = csvOptions.quoteCharacter();
String escape = csvOptions.escapeCharacter();
boolean escapable = !escape.isEmpty();

boolean needsQuoting =
        field.indexOf(csvOptions.fieldDelimiter().charAt(0)) >= 0
                || field.indexOf(csvOptions.lineDelimiter().charAt(0)) >= 0
                || field.indexOf(quote.charAt(0)) >= 0
                || (escapable && field.indexOf(escape.charAt(0)) >= 0);

if (!needsQuoting) {
    return field;
}

String escaped = escapable ? field.replace(escape, escape + escape) : field;
return quote + escaped.replace(quote, escape + quote) + quote;

The order matters: escaping the quotes first would then double the escape characters that step had just inserted, and the reader would decode \\" as a literal backslash followed by an unescaped quote.

escapable keeps an empty csv.escape-character behaving as it does today — field.replace("", ...) inserts between every character, so the substitution has to be skipped rather than run with an empty needle.

The reader is left alone. Once a field containing the escape character is quoted, inQuotes is true by the time the reader reaches it, so it takes the branch that already works; the five cases in the new test all round trip without touching CsvParser. Reading a lone escape character in a CSV file that Paimon did not write still drops it, which is the same behaviour as before this change.

Blast radius

Only fields that contain the escape character are written differently; every other field is byte-identical. A csv.escape-character set to a character that appears often in ordinary data — the / that testCsvEscapeCharacterWriteRead also exercises, say — will now see those fields quoted and the character doubled. That is the point: today they come back with the character missing.

Worth stating plainly: CSV files already written still contain the unescaped form and still read back short. This change stops new files from being written that way; it cannot repair existing ones.

Test evidence

testFieldsContainingTheEscapeCharacterRoundTrip writes five values — Special\Characters, trailing\, a,b\, \\double, \"quoteAfterEscape — and asserts each comes back equal to what went in, and non-null.

Mutation control, on a forced clean rebuild of paimon-format (rm -rf target/classes target/test-classes) so this is not an incremental-build artefact: with the tests kept and CsvFormatWriter reverted, two fail — the new one, and testCsvEscapeCharacterWriteRead with its restored assertions:

CsvFileFormatTest.testCsvEscapeCharacterWriteRead:378
expected: "Special\Characters"
 but was: "SpecialCharacters"

CsvFileFormatTest — 28 tests, and the whole of paimon-format — 551 tests, 0 failures.

API and Format

No change to any option or public signature. The on-disk bytes change only for fields containing the escape character, which are the fields that currently do not survive being read back.

CsvFormatWriter neither quotes a field because it contains the escape
character nor doubles it, but CsvParser consumes an escape character
unless it is followed by a quote or another escape. A value carrying
one therefore does not survive a round trip through the CSV format:
with the defaults, Special\Characters reads back as SpecialCharacters
and a,b\ reads back as null.

Quote such a field and escape the escape before the quotes -- doing it
the other way would double the escapes just inserted for them.

testCsvEscapeCharacterWriteRead already wrote one of these values but
asserted only its int column; its string assertions are added here.
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.

1 participant