Abacn commented on code in PR #29715: URL: https://github.com/apache/beam/pull/29715#discussion_r1439110561
########## sdks/java/io/clickhouse/src/main/java/org/apache/beam/sdk/io/clickhouse/ClickHouseIO.java: ########## @@ -112,7 +113,8 @@ * </table> * * Nullable row columns are supported through Nullable type in ClickHouse. Low cardinality hint is - * supported through LowCardinality DataType in ClickHouse. + * supported through LowCardinality DataType in ClickHouse supported through Tuple DataType in Review Comment: Please check this sentence ########## CHANGES.md: ########## @@ -98,10 +98,10 @@ * TextIO now supports skipping multiple header lines (Java) ([#17990](https://github.com/apache/beam/issues/17990)). * Python GCSIO is now implemented with GCP GCS Client instead of apitools ([#25676](https://github.com/apache/beam/issues/25676)) -* Adding support for LowCardinality DataType in ClickHouse (Java) ([#29533](https://github.com/apache/beam/pull/29533)). * Added support for handling bad records to KafkaIO (Java) ([#29546](https://github.com/apache/beam/pull/29546)) * Add support for generating text embeddings in MLTransform for Vertex AI and Hugging Face Hub models.([#29564](https://github.com/apache/beam/pull/29564)) * NATS IO connector added (Go) ([#29000](https://github.com/apache/beam/issues/29000)). +* Adding support for LowCardinality and Tuples DataType in ClickHouse (Java) ([#29533](https://github.com/apache/beam/pull/29533), [#29715](https://github.com/apache/beam/pull/29715)). Review Comment: LowCardinality will go to 2.53.0 but Tuples does not, as 2.53.0 is already cut ########## sdks/java/io/clickhouse/src/main/java/org/apache/beam/sdk/io/clickhouse/ClickHouseIO.java: ########## @@ -475,6 +477,15 @@ abstract static class Builder<T> { } } + private static String tuplePreprocessing(String payload) { + List<String> l = + Arrays.stream(payload.trim().split(",")) + .map(s -> s.trim().replaceAll(" +", "':;")) + .collect(Collectors.toList()); + String content = + String.join(",", l).trim().replaceAll("Tuple\\(", "Tuple('").replaceAll(",", ",'"); Review Comment: I mean below when " if (type.toLowerCase().trim().startsWith("tuple(")) {" the this preprocessing will be executed. So Tuple\( (with backslash) is considered here but not the condition in the caller there. Coild this cause issue? -- 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]
