Skip to content

[format] Reject multi-character csv delimiter, quote and escape - #9904

Merged
JingsongLi merged 1 commit into
apache:masterfrom
jackylee-ch:csv-single-char-delimiter
Sep 17, 2026
Merged

JingsongLi merged 1 commit into
apache:masterfrom
jackylee-ch:csv-single-char-delimiter

Conversation

@jackylee-ch

Copy link
Copy Markdown
Contributor

Purpose

CsvParser keeps only charAt(0) of csv.field-delimiter, csv.quote-character and csv.escape-character, while the writer emits the whole string. A multi-character value was accepted at DDL and then split on its first character only, so column boundaries silently disagreed with what had been written; an empty value threw StringIndexOutOfBoundsException on read.

fileformat.md:152 has documented must be single character for the field delimiter since it was introduced; this enforces it where it is read and adds the note for quote and escape. csv.line-delimiter is not restricted — CustomLineReader matches all of its bytes.

This gates the Paimon implementation only; format-table.implementation=engine never constructs CsvOptions, and the implementation-independent check belongs in validateFormatTableOptions (#8328).

Tests

CsvFileFormatTest#testSingleCharacterOptionsAreEnforced. Reverting the fix:

Expecting code to raise a throwable.

paimon-format: 634 run, 0 failures.

Written with Claude Code; verification is mine.

@JingsongLi

Copy link
Copy Markdown
Contributor

The premise checks out: CsvFormatWriter appends the delimiter/quote/escape option strings verbatim, while CsvParser only ever reads charAt(0) — a multi-character value silently corrupted the read path, and there was no validation at all. Validating at option construction is the right place, and keeping LINE_DELIMITER exempt is correct since it is genuinely a multi-character token.

The test covers both a multi-char value and the empty string, and the docs are updated consistently.

@JingsongLi
JingsongLi merged commit 6e12229 into apache:master Sep 17, 2026
18 checks passed
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.

2 participants