wuchong commented on code in PR #3558:
URL: https://github.com/apache/fluss/pull/3558#discussion_r3566711846
##########
fluss-common/src/main/java/org/apache/fluss/config/ConfigOptions.java:
##########
@@ -1604,14 +1583,24 @@ public class ConfigOptions {
+ "If the value is `HOUR`, the partition
format for "
+ "auto created is yyyyMMddHH. "
+ "If the value is `DAY`, the partition
format for "
- + "auto created is yyyyMMdd. "
+ + "auto created is yyyyMMdd by default. "
+ "If the value is `MONTH`, the partition
format for "
+ "auto created is yyyyMM. "
+ "If the value is `QUARTER`, the
partition format for "
+ "auto created is yyyyQ. "
+ "If the value is `YEAR`, the partition
format for "
+ "auto created is yyyy.");
+ public static final ConfigOption<String> TABLE_AUTO_PARTITION_DAY_FORMAT =
+ key("table.auto-partition.day-format")
Review Comment:
Would it make sense to generalize this option to
`table.auto-partition.time-format` instead of introducing a DAY-specific public
key? The formatting path already handles `HOUR`, `DAY`, `MONTH`, `QUARTER`, and
`YEAR`. Once `day-format` becomes public, supporting formats such as `yyyy-MM`
or `yyyy-MM-dd-HH` later would require additional per-unit options or
deprecating this key.
If we generalize it, however, I don't think we should accept arbitrary
`DateTimeFormatter` patterns without validation. Partition retention currently
relies on the lexicographical ordering of formatted partition values through
`compareTo()` and `TreeMap.headMap()`, and the format must preserve the
configured granularity. For example, an HOUR format without `HH` would cause
multiple hours to map to the same partition, while `MM-yyyy` or non-zero-padded
fields would not sort chronologically.
Could we introduce `table.auto-partition.time-format`, use the existing
unit-specific formats when the option is absent for backward compatibility, and
validate supported formats per time unit so that they are fixed-width, ordered
from the most significant field to the least significant field, and include the
required time-unit field?
Example:
https://help.aliyun.com/zh/hologres/developer-reference/dynamic-partition-management?spm=a2c4g.11186623.help-menu-113622.d_5_0_5_2_3.613f2519QaKdYr
--
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]
