cwq222 commented on code in PR #13080:
URL: https://github.com/apache/gravitino/pull/13080#discussion_r4032769377
##########
catalogs-contrib/catalog-jdbc-clickhouse/src/main/java/org/apache/gravitino/catalog/clickhouse/operations/ClickHouseTableSqlUtils.java:
##########
@@ -178,49 +185,30 @@ private static String
normalizeShardingKeyExpression(String expression) {
return normalizeIndexExpression(current);
}
- private static Transform parsePartitionExpression(
- String expression, String originalPartitionKey) {
+ @Nullable
+ private static Transform parsePartitionExpression(String expression) {
String trimmedExpression = StringUtils.trim(expression);
Matcher toYearMatcher = TO_YEAR_PATTERN.matcher(trimmedExpression);
if (toYearMatcher.matches()) {
String identifier = normalizeIdentifier(toYearMatcher.group(1));
- Preconditions.checkArgument(
- StringUtils.isNotBlank(identifier),
- "Unsupported partition expression: " + originalPartitionKey);
- return Transforms.year(identifier);
+ return isStrictIdentifier(identifier) ? Transforms.year(identifier) :
null;
Review Comment:
Hi @yuqi1129 , Thanks for the review. The backtick-quoted column name
parsing issue is fixed: `toYYYYMM(\`event-time\`)` now structures correctly as
`month("event-time")` and round-trips back to `toYYYYMM(\`event-time\`)`. A
unit test covering the backtick case has been added.
Regarding nested type-conversion expressions such as
`toYYYYMM(toDate(event_time))`: the previous implementation returned
`month("toDate(event_time)")`, whose field name is the function expression
`toDate(event_time)` rather than a real column, which is incorrect. This
implementation now returns an empty `partitioning()` and preserves the original
expression verbatim in the `partition-key` property. Automatic extraction of
the inner column (e.g. `month("event_time")`) can be handled in a follow-up PR.
Thanks again for your careful review.
--
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]
