swuferhong commented on code in PR #3558:
URL: https://github.com/apache/fluss/pull/3558#discussion_r3593363206
##########
website/docs/table-design/data-distribution/partitioning.md:
##########
@@ -74,7 +75,17 @@ The time unit for the automatic partition table
`auto-partition.time-unit` can t
| MONTH | yyyyMM | 202409 |
| QUARTER | yyyyQ | 20241 |
| YEAR | yyyy | 2024 |
-
+
+The default format can be overridden with `table.auto-partition.time-format`.
Custom formats must satisfy all of the following rules:
+
+- Time fields are ordered from the largest to the smallest unit.
+- Every field that affects the configured partition granularity is present.
+- Numeric fields use a fixed width: four digits for year, two for month, day,
and hour, and one for quarter.
+
+For example, `yyyy-MM-dd` and `yyyy/MM/dd` are valid DAY formats. `MM-yyyy`
has the wrong field order, `yyyy-MM` omits the day, and `yyyy-M-dd` uses a
variable-width month, so they are rejected. Literal text can be quoted, as in
`yyyy-'Q'Q` for QUARTER. When the auto-partition key has DATE type, the time
unit must be DAY and the format must be `yyyy-MM-dd`.
Review Comment:
`yyyy/MM/dd` is a invalid format?
##########
fluss-common/src/main/java/org/apache/fluss/utils/PartitionUtils.java:
##########
@@ -171,39 +183,225 @@ public static ResolvedPartitionSpec
generateAutoPartition(
ZonedDateTime current,
int offset,
AutoPartitionTimeUnit timeUnit) {
- String autoPartitionFieldSpec = generateAutoPartitionTime(current,
offset, timeUnit);
+ return generateAutoPartition(
+ partitionKeys,
+ current,
+ offset,
+ timeUnit,
+ AutoPartitionStrategy.from(new Configuration()));
+ }
+
+ public static ResolvedPartitionSpec generateAutoPartition(
+ List<String> partitionKeys,
+ ZonedDateTime current,
+ int offset,
+ AutoPartitionTimeUnit timeUnit,
+ AutoPartitionStrategy autoPartitionStrategy) {
+ String autoPartitionFieldSpec =
+ generateAutoPartitionTime(current, offset, timeUnit,
autoPartitionStrategy);
return ResolvedPartitionSpec.fromPartitionName(partitionKeys,
autoPartitionFieldSpec);
}
public static String generateAutoPartitionTime(
ZonedDateTime current, int offset, AutoPartitionTimeUnit timeUnit)
{
+ return generateAutoPartitionTime(
+ current, offset, timeUnit, AutoPartitionStrategy.from(new
Configuration()));
+ }
+
+ public static String generateAutoPartitionTime(
+ ZonedDateTime current,
+ int offset,
+ AutoPartitionTimeUnit timeUnit,
+ AutoPartitionStrategy autoPartitionStrategy) {
String autoPartitionFieldSpec;
switch (timeUnit) {
case YEAR:
- autoPartitionFieldSpec =
getFormattedTime(current.plusYears(offset), YEAR_FORMAT);
+ autoPartitionFieldSpec =
+ getFormattedTime(
+ current.plusYears(offset),
+ getPartitionTimeFormat(timeUnit,
autoPartitionStrategy));
break;
case QUARTER:
autoPartitionFieldSpec =
- getFormattedTime(current.plusMonths(offset * 3L),
QUARTER_FORMAT);
+ getFormattedTime(
+ current.plusMonths(offset * 3L),
+ getPartitionTimeFormat(timeUnit,
autoPartitionStrategy));
break;
case MONTH:
- autoPartitionFieldSpec =
getFormattedTime(current.plusMonths(offset), MONTH_FORMAT);
+ autoPartitionFieldSpec =
+ getFormattedTime(
+ current.plusMonths(offset),
+ getPartitionTimeFormat(timeUnit,
autoPartitionStrategy));
break;
case DAY:
- autoPartitionFieldSpec =
getFormattedTime(current.plusDays(offset), DAY_FORMAT);
+ autoPartitionFieldSpec =
+ getFormattedTime(
+ current.plusDays(offset),
+ getPartitionTimeFormat(timeUnit,
autoPartitionStrategy));
break;
case HOUR:
- autoPartitionFieldSpec =
getFormattedTime(current.plusHours(offset), HOUR_FORMAT);
+ autoPartitionFieldSpec =
+ getFormattedTime(
+ current.plusHours(offset),
+ getPartitionTimeFormat(timeUnit,
autoPartitionStrategy));
break;
default:
throw new IllegalArgumentException("Unsupported time unit: " +
timeUnit);
}
return autoPartitionFieldSpec;
}
+ /**
+ * Validates that a custom time format preserves time order under string
comparison.
+ *
+ * @param timeUnit the auto-partition time unit
+ * @param autoPartitionStrategy the auto-partition strategy containing the
custom format
+ * @throws IllegalArgumentException if the pattern is invalid or does not
preserve time order
+ */
+ public static void validateTimeFormat(
+ AutoPartitionTimeUnit timeUnit, AutoPartitionStrategy
autoPartitionStrategy) {
+ String timeFormat = autoPartitionStrategy.timeFormat();
+ if (timeFormat == null) {
+ return;
+ }
+ try {
+ DateTimeFormatter.ofPattern(timeFormat);
+ } catch (IllegalArgumentException e) {
+ throw new IllegalArgumentException(
+ String.format("Invalid date-time format pattern '%s'.",
timeFormat), e);
+ }
+ validateTimeFields(timeFormat, timeUnit);
+ }
+
+ private static void validateTimeFields(String timeFormat,
AutoPartitionTimeUnit timeUnit) {
+ List<TimeFormatField> expectedFields =
getExpectedTimeFormatFields(timeUnit);
+ List<TimeFormatField> actualFields = new ArrayList<>();
+ boolean inLiteral = false;
+ for (int index = 0; index < timeFormat.length(); ) {
+ char patternChar = timeFormat.charAt(index);
+ if (patternChar == '\'') {
Review Comment:
The custom time format should also be validated against Fluss partition-name
restrictions.
For example, `yyyy/MM/dd` currently passes `validateTimeFormat()` and
generates a value such as `2026/07/16`, but `/` is not allowed in Fluss
partition values (the file system restrictions). As a result, the table can be
created successfully, but automatic partition creation will fail later.
Formats producing overlong values or values with reserved prefixes may have
the same problem. Could we format a representative timestamp here and validate
the result using the existing partition-name validation rules?
##########
fluss-common/src/main/java/org/apache/fluss/utils/PartitionUtils.java:
##########
@@ -171,39 +183,225 @@ public static ResolvedPartitionSpec
generateAutoPartition(
ZonedDateTime current,
int offset,
AutoPartitionTimeUnit timeUnit) {
- String autoPartitionFieldSpec = generateAutoPartitionTime(current,
offset, timeUnit);
+ return generateAutoPartition(
+ partitionKeys,
+ current,
+ offset,
+ timeUnit,
+ AutoPartitionStrategy.from(new Configuration()));
+ }
+
+ public static ResolvedPartitionSpec generateAutoPartition(
+ List<String> partitionKeys,
+ ZonedDateTime current,
+ int offset,
+ AutoPartitionTimeUnit timeUnit,
+ AutoPartitionStrategy autoPartitionStrategy) {
+ String autoPartitionFieldSpec =
+ generateAutoPartitionTime(current, offset, timeUnit,
autoPartitionStrategy);
return ResolvedPartitionSpec.fromPartitionName(partitionKeys,
autoPartitionFieldSpec);
}
public static String generateAutoPartitionTime(
ZonedDateTime current, int offset, AutoPartitionTimeUnit timeUnit)
{
+ return generateAutoPartitionTime(
+ current, offset, timeUnit, AutoPartitionStrategy.from(new
Configuration()));
+ }
+
+ public static String generateAutoPartitionTime(
+ ZonedDateTime current,
+ int offset,
+ AutoPartitionTimeUnit timeUnit,
+ AutoPartitionStrategy autoPartitionStrategy) {
String autoPartitionFieldSpec;
switch (timeUnit) {
case YEAR:
- autoPartitionFieldSpec =
getFormattedTime(current.plusYears(offset), YEAR_FORMAT);
+ autoPartitionFieldSpec =
+ getFormattedTime(
+ current.plusYears(offset),
+ getPartitionTimeFormat(timeUnit,
autoPartitionStrategy));
break;
case QUARTER:
autoPartitionFieldSpec =
- getFormattedTime(current.plusMonths(offset * 3L),
QUARTER_FORMAT);
+ getFormattedTime(
+ current.plusMonths(offset * 3L),
+ getPartitionTimeFormat(timeUnit,
autoPartitionStrategy));
break;
case MONTH:
- autoPartitionFieldSpec =
getFormattedTime(current.plusMonths(offset), MONTH_FORMAT);
+ autoPartitionFieldSpec =
+ getFormattedTime(
+ current.plusMonths(offset),
+ getPartitionTimeFormat(timeUnit,
autoPartitionStrategy));
break;
case DAY:
- autoPartitionFieldSpec =
getFormattedTime(current.plusDays(offset), DAY_FORMAT);
+ autoPartitionFieldSpec =
+ getFormattedTime(
+ current.plusDays(offset),
+ getPartitionTimeFormat(timeUnit,
autoPartitionStrategy));
break;
case HOUR:
- autoPartitionFieldSpec =
getFormattedTime(current.plusHours(offset), HOUR_FORMAT);
+ autoPartitionFieldSpec =
+ getFormattedTime(
+ current.plusHours(offset),
+ getPartitionTimeFormat(timeUnit,
autoPartitionStrategy));
break;
default:
throw new IllegalArgumentException("Unsupported time unit: " +
timeUnit);
}
return autoPartitionFieldSpec;
}
+ /**
+ * Validates that a custom time format preserves time order under string
comparison.
+ *
+ * @param timeUnit the auto-partition time unit
+ * @param autoPartitionStrategy the auto-partition strategy containing the
custom format
+ * @throws IllegalArgumentException if the pattern is invalid or does not
preserve time order
+ */
+ public static void validateTimeFormat(
+ AutoPartitionTimeUnit timeUnit, AutoPartitionStrategy
autoPartitionStrategy) {
+ String timeFormat = autoPartitionStrategy.timeFormat();
+ if (timeFormat == null) {
+ return;
+ }
+ try {
+ DateTimeFormatter.ofPattern(timeFormat);
+ } catch (IllegalArgumentException e) {
+ throw new IllegalArgumentException(
+ String.format("Invalid date-time format pattern '%s'.",
timeFormat), e);
+ }
+ validateTimeFields(timeFormat, timeUnit);
+ }
+
+ private static void validateTimeFields(String timeFormat,
AutoPartitionTimeUnit timeUnit) {
+ List<TimeFormatField> expectedFields =
getExpectedTimeFormatFields(timeUnit);
+ List<TimeFormatField> actualFields = new ArrayList<>();
+ boolean inLiteral = false;
Review Comment:
Could we reject optional sections (`[` and `]`) in custom time formats?
For example, `yyyy[-MM[-dd]]` currently passes the field validation because
it contains year, month, and day fields. However, `DateTimeFormatter` also
accepts `2026` and `2026-07` with this pattern.
This means a DAY-partitioned table may accept partition names that do not
identify a specific day. It also breaks the fixed-width assumption used by
partition retention ordering. Add a test to verify it.
--
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]
