Skip to content

[GLUTEN-12597][CORE] Migrate ReadRel text read options to Substrait 0.98 (TextReadOptions -> DelimiterSeparatedTextReadOptions) - #12917

Draft
nielspardon wants to merge 1 commit into
apache:mainfrom
nielspardon:feat/substrait-0.98-readrel-text-options
Draft

[GLUTEN-12597][CORE] Migrate ReadRel text read options to Substrait 0.98 (TextReadOptions -> DelimiterSeparatedTextReadOptions)#12917
nielspardon wants to merge 1 commit into
apache:mainfrom
nielspardon:feat/substrait-0.98-readrel-text-options

Conversation

@nielspardon

Copy link
Copy Markdown
Contributor

What changes are proposed in this pull request?

Part of #12597.

Rebases the text read options in ReadRel.LocalFiles.FileOrFiles onto Substrait v0.98.0, where the fork's TextReadOptions became DelimiterSeparatedTextReadOptions. Upstream's six fields are adopted verbatim (field_delimiter 1, max_line_size 2, quote 3, header_lines_to_skip 4, escape 5, optional string value_treated_as_null 6). header_lines_to_skip and value_treated_as_null are the fork's header (was 5) and null_value (was 7) renamed with the same meaning, and the deprecated schema field is dropped (nothing reads or writes it — the setSchema call in LocalFilesNode targets the file-level FileOrFiles.schema, which is unrelated). Gluten's two genuinely-local knobs move to the 1000 graft range documented in SubstraitModifications.md: max_block_size to 1000 and empty_as_default to 1001. The file_format oneof keeps text at tag 14, which is also upstream's number.

The one subtlety this PR is careful about: the fork put max_block_size (rows per output block, a ClickHouse batching knob) on tag 2 — exactly where 0.98 puts max_line_size (max bytes per line, a parse limit). These are different concepts with the same wire type, so conflating them by renaming the field would compile and silently change meaning. Instead max_line_size is vendored verbatim at tag 2 (Gluten produces no value for it) and max_block_size is relocated to the 1000 graft, keeping the two as distinct fields.

Because tags 2 and 6 are reused with compatible wire types but different meanings, a JAR and a native library built from opposite sides of this change mis-read each other silently rather than failing — in particular old tag 6 (escape, which Gluten's Hive text path always sets) now parses as value_treated_as_null. The JAR and native library must therefore be rebuilt together; this is called out in the proto comment and in SubstraitModifications.md.

value_treated_as_null is optional in 0.98, so setting it — even to the empty string — declares "this value is null and the file is entirely nullable strings". The producer therefore leaves it unset when the reader supplied no nullValue, and the ClickHouse consumer keys off field presence (has_value_treated_as_null()) rather than emptiness. This preserves Gluten's existing "no nullValue supplied" behavior.

JsonReadOptions keeps its Gluten-local fork until upstream adds equivalent JSON read options.

How was this patch tested?

Standalone protoc compile of the proto (well-formed, no duplicate tags). Local JVM build and the new DelimiterSeparatedTextReadOptionsProtoSuite, which drives the real LocalFilesNode producer and pins the descriptor: the six upstream tags, value_treated_as_null's presence tracking, the max_block_size (1000) vs max_line_size (2) split, the dropped/renamed field names, the text oneof entry, and the optional-presence contract on value_treated_as_null. The ClickHouse consumer (ExcelTextFormatFile.cpp) is exercised only in ClickHouse CI (native build needs a Linux/Docker toolchain), so its edits were verified by inspection; the gtest_parser.cpp fixture was updated to the renamed proto-JSON field name. The Velox side has no text-options code path.

Was this patch authored or co-authored using generative AI tooling?

Generated-by: Claude Code (Claude Opus 4.8)

🤖 Generated with AI

….98 (TextReadOptions -> DelimiterSeparatedTextReadOptions)

Part of apache#12597.
@github-actions github-actions Bot added CORE works for Gluten Core CLICKHOUSE DOCS labels Aug 27, 2026
@github-actions

Copy link
Copy Markdown

Run Gluten Clickhouse CI on x86

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLICKHOUSE CORE works for Gluten Core DOCS

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant