nielspardon opened a new pull request, #12917: URL: https://github.com/apache/gluten/pull/12917
## 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 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
