Blazer-007 commented on code in PR #4185:
URL: https://github.com/apache/gobblin/pull/4185#discussion_r3055612481
##########
gobblin-data-management/src/main/java/org/apache/gobblin/data/management/copy/iceberg/IcebergSource.java:
##########
@@ -96,9 +96,11 @@
* # --- Recommended: configurable partition value format ---
* # iceberg.partition.value.datetime.format is a DateTimeFormatter pattern
applied to the output
* # partition value used in the filter expression.
- * # When CURRENT_DATE is used, the reference datetime is LocalDateTime.now(),
so a pattern
- * # with HH will embed the current hour automatically — no separate hour
config needed.
* # When set, it supersedes iceberg.hourly.partition.enabled.
+ * #
+ * # CURRENT_DATE behaviour:
+ * # - With this property set → LocalDateTime.now(), so HH embeds the live
clock-hour.
+ * # - Without this property (legacy) → LocalDate.now() at midnight, HH
stays -00 (backward compat).
Review Comment:
it should resolve only when hourly flag is enabled
##########
gobblin-data-management/src/main/java/org/apache/gobblin/data/management/copy/iceberg/IcebergSource.java:
##########
@@ -152,10 +154,18 @@ public class IcebergSource extends
FileBasedSource<String, FileAwareInputStream>
/**
* Optional {@link DateTimeFormatter} pattern controlling how the partition
value is rendered.
*
- * <p>When {@code iceberg.filter.date=CURRENT_DATE} the reference datetime is
- * {@link java.time.LocalDateTime#now()}, so a pattern that includes {@code
HH} will embed
- * the current clock-hour automatically — no separate hour config is needed.
- * For a specific date (e.g. {@code 2025-04-03}), the time defaults to
midnight (00:00).
+ * <p><b>CURRENT_DATE behaviour differs between the two paths:</b>
+ * <ul>
+ * <li>When this property <em>is</em> set, {@code CURRENT_DATE} resolves to
Review Comment:
Which property ?
##########
gobblin-data-management/src/main/java/org/apache/gobblin/data/management/copy/iceberg/IcebergSource.java:
##########
@@ -337,13 +347,27 @@ private List<IcebergTable.FilePathWithPartition>
discoverPartitionFilePaths(Sour
DateTimeFormatter partitionFormatter = resolvePartitionFormatter(state);
// Resolve the reference datetime for the filter.
- // CURRENT_DATE uses LocalDateTime.now() so a formatter pattern that
includes HH will
- // embed the current clock-hour automatically. For a specific date
(yyyy-MM-dd) the time
- // defaults to midnight (00:00).
+ // For a specific date (yyyy-MM-dd) the time always defaults to midnight
(00:00).
+ // For CURRENT_DATE:
+ // - Custom format path (iceberg.partition.value.datetime.format set):
LocalDateTime.now() so
+ // a pattern that includes HH will embed the live clock-hour
automatically.
+ // - Legacy path (no custom format): LocalDate.now().atStartOfDay()
(midnight) to preserve the
+ // pre-PR behavior where CURRENT_DATE always produced a -00 suffix.
Users who genuinely need
+ // the live hour should migrate to
iceberg.partition.value.datetime.format=yyyy-MM-dd-HH.
LocalDateTime startDateTime;
if (CURRENT_DATE_PLACEHOLDER.equalsIgnoreCase(dateValue)) {
- startDateTime = LocalDateTime.now();
- log.info("Resolved {} placeholder to current datetime: {}",
CURRENT_DATE_PLACEHOLDER, startDateTime);
+ boolean isCustomFormat =
state.contains(ICEBERG_PARTITION_VALUE_DATETIME_FORMAT);
+ if (isCustomFormat) {
+ startDateTime = LocalDateTime.now();
+ log.info("Resolved {} to current datetime with live hour (custom
format='{}'): {}",
+ CURRENT_DATE_PLACEHOLDER,
state.getProp(ICEBERG_PARTITION_VALUE_DATETIME_FORMAT), startDateTime);
+ } else {
+ // Legacy backward-compat: always midnight so the yyyy-MM-dd-HH
pattern keeps the old -00 suffix.
+ startDateTime = LocalDate.now().atStartOfDay();
+ log.info("Resolved {} to current date at midnight (legacy mode, -00
preserved): {}. "
+ + "Set {} to use the live hour.",
+ CURRENT_DATE_PLACEHOLDER, startDateTime,
ICEBERG_PARTITION_VALUE_DATETIME_FORMAT);
Review Comment:
Shouldn't this resolved only when hourly flag is true, although default
value is true but user can pass hourly enabled as false as well
##########
gobblin-data-management/src/main/java/org/apache/gobblin/data/management/copy/iceberg/IcebergSource.java:
##########
@@ -152,10 +154,18 @@ public class IcebergSource extends
FileBasedSource<String, FileAwareInputStream>
/**
* Optional {@link DateTimeFormatter} pattern controlling how the partition
value is rendered.
*
- * <p>When {@code iceberg.filter.date=CURRENT_DATE} the reference datetime is
- * {@link java.time.LocalDateTime#now()}, so a pattern that includes {@code
HH} will embed
- * the current clock-hour automatically — no separate hour config is needed.
- * For a specific date (e.g. {@code 2025-04-03}), the time defaults to
midnight (00:00).
+ * <p><b>CURRENT_DATE behaviour differs between the two paths:</b>
+ * <ul>
+ * <li>When this property <em>is</em> set, {@code CURRENT_DATE} resolves to
+ * {@link java.time.LocalDateTime#now()}, so a pattern that includes
{@code HH} embeds the
+ * live clock-hour automatically — useful for truly hourly-partitioned
tables.</li>
+ * <li>When this property is <em>absent</em> (legacy path), {@code
CURRENT_DATE} resolves to
+ * {@link java.time.LocalDate#now()} at midnight (00:00), preserving
the pre-PR behaviour
Review Comment:
Which property ?
--
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]
