[CH] Discriminate MergeTree/Range ExtensionTable reads by type_url, not isMergeTree=/isRange= marker strings
- Dominant language
- Scala
- Stars
- 1.6k
- Forks
- 657
- Avg merge
- 2d 14h
- Merged PRs (30d)
- 80
Description
### Background
Gluten's ClickHouse backend has three `ReadRel.ExtensionTable`-based reads: MergeTree, Range, and (as of #12841) Kafka. Kafka now discriminates its read the idiomatic way — by the `Any` type_url of `ExtensionTable.detail`, via `detail().Is()`, using a typed Gluten-owned payload message (`gluten.StreamKafka` in `kafka.proto`). This mirrors the already-merged Velox Iceberg idiom `enhancement().Is<::gluten::IcebergReadExtension>()`. MergeTree and Range still use a legacy workaround instead: a hand-rolled text marker string.
### Current state (the anti-pattern)
**Discriminator** — the producer stamps a text marker into `advanced_extension.optimization` as a `google.protobuf.StringValue`, and the native consumer discriminates by parsing its prefix:
- `isMergeTree=1\n` — produced at `CHMergeTreeWriterInjects.scala:195`; a per-scan `isMergeTree=$flag\n` (0 or 1) is also stamped onto **every** CH scan at `BasicScanExecTransformer.scala:187-190`.
- `isRange=1\n` — produced at `CHRangeExecTransformer.scala:92`.
- Consumer: `ReadRelParser::isReadRelFromMergeTree` / `isReadRelFromRange` at `ReadRelParser.cpp:130-162` (`checkString("isMergeTree=", ...)` / `checkString("isRange=", ...)`).
**Payload** — the actual table info rides in `ExtensionTable.detail`, also as a `StringValue` text blob: the MergeTree table string is hand-parsed by `doParseMergeTreeTableString` (`SparkMergeTreeMeta.cpp:130`); the Range payload is JSON, parsed at `ReadRelParser.cpp:234-250`.
Because both payloads are a generic `google.protobuf.StringValue` (type_url `type.googleapis.com/google.protobuf.StringValue` for both), the `detail` type_url can't tell them apart — which is precisely why the `isMergeTree=`/`isRange=` markers were bolted on.
### Proposal
Give MergeTree and Range typed, Gluten-owned payload messages — the same convention as `gluten.StreamKafka` (`package gluten`, `org.apache.gluten.proto`), e.g. `gluten.MergeTreeTable` and `gluten.RangeTable` — pack them into `ExtensionTable.detail`, and discriminate by type_url:
```cpp
rel.extension_table().detail().Is()
rel.extension_table().detail().Is()
```
This drops the `isMergeTree=`/`isRange=` marker strings (including the per-scan `isMergeTree=0\n` stamp on non-MergeTree scans) and the hand-rolled text/JSON parsing, converging all three CH `ExtensionTable` readers on one discrimination model.
### Scope / why this is a follow-up, not part of #12597
This is a behavioral rewrite of the core CH read path — new proto messages plus rewriting both producers and both native parsers — and it is orthogonal to the Substrait-0.98 proto rebase (#12597): these blobs are Gluten-internal payloads, not Substrait messages, so no rebase increment forces the change. #12841 (Kafka) establishes the target model this converges MergeTree/Range onto.
### Affected code
- Producers: `CHMergeTreeWriterInjects.scala:195`, `BasicScanExecTransformer.scala:187-190` (per-scan MergeTree flag), `CHRangeExecTransformer.scala:92`.
- Consumer: `ReadRelParser.cpp` (`isReadRelFromMergeTree`/`isReadRelFromRange` at `:130-162`, dispatch at `:64-89`, `parseReadRelWithRange` at `:222-250`), `SparkMergeTreeMeta.cpp` (`doParseMergeTreeTableString` at `:130`, plus the matching text serializer), and the Range JSON payload builder.
Part of the Substrait-0.98 consolidation follow-ups (#12597).
Contributor guide
Research direction
Start with the affected producers in CHMergeTreeWriterInjects.scala, BasicScanExecTransformer.scala, and CHRangeExecTransformer.scala, then read ReadRelParser.cpp and SparkMergeTreeMeta.cpp around the cited methods. Trace how MergeTree and Range payloads are built and parsed. Done means both reads use typed ExtensionTable.detail messages and type_url discrimination, with the legacy marker strings and text/JSON parsing removed.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp, scala
- Domain
- backend, databases
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 35/100