You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hudi.apache.org by "fhan688 (via GitHub)" <gi...@apache.org> on 2026/07/21 10:31:16 UTC

[PR] feat(common): query LAYOUT_VERSION_1 timelines in CompletionTimeQueryView by using instant time as completion time [hudi]

fhan688 opened a new pull request, #19338:
URL: https://github.com/apache/hudi/pull/19338

   ### Describe the issue this Pull Request addresses
   
   The 1.x incremental read stack (`IncrementalQueryAnalyzer` → `CompletionTimeQueryView.getInstantTimes`) resolves the query range by completion time. For a `LAYOUT_VERSION_1` timeline (table versions 5–7, releases 0.12–0.16), `CompletionTimeQueryViewV1.getInstantTimes` currently throws:
   
   ```
   RuntimeException("Incremental query view for timeline version 1 not yet implemented")
   ```
   
   As a result, any incremental (and streaming) read against these older tables via the 1.x reader fails outright.
   
   The key observation — as raised in review — is that this is **not** a "missing completion time" problem: for a `LAYOUT_VERSION_1` timeline, an instant's completion time *is* its instant (request) time, and the semantics are backward compatible. This is already how the V1 view behaves elsewhere (`load()` maps `requestedTime → completionTime`, and `getCompletionTime` returns the begin time for archived instants). This PR applies that same equivalence to the range query, so V1 falls out as the "completion time = instant time" special case of the existing V2 range logic rather than a separate implementation.
   
   ### Summary and Changelog
   
   Implements `CompletionTimeQueryViewV1.getInstantTimes(...)` by mirroring the branch structure already used in `CompletionTimeQueryViewV2`, driven by requested (instant) time instead of a persisted completion time:
   
   - `(_, end]` — returns the single latest instant whose requested time is `<= end`.
   - `['earliest', _)` — clears the start bound so it degenerates to consuming from the earliest instant.
   - `(_, _)` — returns the latest snapshot instant.
   - otherwise — filters instants through a shared `InstantRange` (honoring the `OPEN_CLOSED` / `CLOSED_CLOSED` bounds and nullable boundaries), sorted ascending.
   
   Notes:
   
   - The candidate instants are taken from the `HoodieTimeline` passed in by the caller (already filtered per user configs such as `skipCompaction` / `skipClustering`), consistent with the V2 code path.
   - The archived timeline is intentionally **not** consulted for V1 (it is not loaded), which is the only V1-specific boundary; the range/resume semantics are the same logic as V2, not a fork. The `earliestInstantTimeFunc` parameter is unused for V1 (it only serves V2's archive lazy-load boundary).
   
   Tests: adds `TestCompletionTimeQueryViewV1` covering closed/closed first read, open/closed streaming resume (excluding the last issued offset), `earliest` start, end-only, and no-bounds latest-snapshot cases.
   
   No code was copied verbatim; the implementation follows the shape of `CompletionTimeQueryViewV2#getInstantTimes`.
   
   ### Impact
   
   - Enables incremental and streaming reads on table versions 5–7 (`LAYOUT_VERSION_1`) through the 1.x reader; these previously threw.
   - No behavior change for table version ≥ 8 (the V2 view is untouched).
   - No public API change: only the previously-throwing package-private `getInstantTimes` body is implemented.
   
   ### Risk Level
   
   Medium. This changes read semantics for older tables from "throws" to "returns instants". Mitigated by:
   
   - New unit tests exercising the range and streaming-resume boundaries (`OPEN_CLOSED` must exclude the issued offset, `CLOSED_CLOSED` first read includes the start commit, `earliest` / end-only / no-bounds).
   - Verified by compiling `hudi-common` and running `TestCompletionTimeQueryViewV1` locally — 5/5 tests pass.
   
   ### Documentation Update
   
   None. No new config, and no user-facing option changes; this makes an existing documented feature (incremental/streaming query) work on `LAYOUT_VERSION_1` tables.
   
   ### Contributor's checklist
   
   - [x] Read through [contributor's guide](https://hudi.apache.org/contribute/how-to-contribute)
   - [x] Enough context is provided in the sections above
   - [x] Adequate tests were added if applicable


-- 
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: commits-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(common): query LAYOUT_VERSION_1 timelines in CompletionTimeQueryView by using instant time as completion time [hudi]

Posted by "hudi-agent (via GitHub)" <gi...@apache.org>.
hudi-agent commented on code in PR #19338:
URL: https://github.com/apache/hudi/pull/19338#discussion_r3728097566


##########
hudi-common/src/main/java/org/apache/hudi/common/table/timeline/versioning/v1/CompletionTimeQueryViewV1.java:
##########
@@ -205,7 +217,57 @@ private List<String> getInstantTimes(
       Option<String> rangeEnd,
       InstantRange.RangeType rangeType,
       Function<String, String> earliestInstantTimeFunc) {
-    throw new RuntimeException("Incremental query view for timeline version 1 not yet implemented");
+    final boolean startFromEarliest = START_COMMIT_EARLIEST.equalsIgnoreCase(rangeStart.orElse(null));
+    final Option<String> effectiveStart = startFromEarliest ? Option.empty() : rangeStart;
+    final HoodieTimeline completedTimeline = timeline.filterCompletedInstants();
+
+    if (rangeStart.isEmpty() && rangeEnd.isPresent()) {
+      // (_, end]: returns the last instant whose requested time is at or before 'end'.
+      Option<String> latestActiveInstant = Option.fromJavaOptional(completedTimeline.getInstantsAsStream()
+          .filter(instant -> InstantComparison.compareTimestamps(instant.requestedTime(), LESSER_THAN_OR_EQUALS, rangeEnd.get()))
+          .max(Comparator.comparing(HoodieInstant::requestedTime))
+          .map(HoodieInstant::requestedTime));
+      if (latestActiveInstant.isPresent()) {
+        return Collections.singletonList(latestActiveInstant.get());
+      }
+      // The archived timeline is not filtered by user configs yet, so return all candidates and
+      // let IncrementalQueryAnalyzer select the last eligible instant after applying those filters.
+      return getArchivedInstantTimes(Option.empty())
+          .filter(instant -> InstantComparison.compareTimestamps(instant, LESSER_THAN_OR_EQUALS, rangeEnd.get()))
+          .sorted()
+          .collect(Collectors.toList());
+    }
+
+    if (effectiveStart.isEmpty() && rangeEnd.isEmpty()) {
+      // (_, _): read the latest snapshot.
+      return completedTimeline.lastInstant().map(instant -> Collections.singletonList(instant.requestedTime())).orElse(Collections.emptyList());
+    }
+
+    final InstantRange instantRange = InstantRange.builder()
+        .rangeType(rangeType)
+        .startInstant(effectiveStart.orElse(null))
+        .endInstant(rangeEnd.orElse(null))
+        .nullableBoundary(true)
+        .build();
+    Stream<String> candidateInstants = completedTimeline.getInstantsAsStream().map(HoodieInstant::requestedTime);
+    final boolean needsArchivedInstants = startFromEarliest
+        || (effectiveStart.isPresent() && isArchived(effectiveStart.get()));
+    if (needsArchivedInstants) {
+      candidateInstants = Stream.concat(getArchivedInstantTimes(effectiveStart), candidateInstants);
+    }
+    return candidateInstants
+        .filter(instantRange::isInRange)
+        .distinct()
+        .sorted()
+        .collect(Collectors.toList());
+  }
+
+  private Stream<String> getArchivedInstantTimes(Option<String> startInstant) {

Review Comment:
   🤖 When the query range includes archived instants, the archived timeline ends up loaded twice per `analyze()` — once here with `useCache=false`, and again in `IncrementalQueryAnalyzer.getArchivedReadTimeline` (also `useCache=false`). The `getArchivedTimeline` javadoc flags this as costly, and the `(_, end]` fallback loads from `""` (the whole archived timeline). Since V2 amortizes this via its in-memory map, is there a way to avoid the second full load on the catch-up path (e.g. `useCache=true`, or letting the view/analyzer share one load)?
   
   <sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag quality.</i></sub>



-- 
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: commits-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(common): query LAYOUT_VERSION_1 timelines in CompletionTimeQueryView by using instant time as completion time [hudi]

Posted by "hudi-agent (via GitHub)" <gi...@apache.org>.
hudi-agent commented on code in PR #19338:
URL: https://github.com/apache/hudi/pull/19338#discussion_r3624553404


##########
hudi-common/src/main/java/org/apache/hudi/common/table/timeline/versioning/v1/CompletionTimeQueryViewV1.java:
##########
@@ -205,7 +216,39 @@ private List<String> getInstantTimes(
       Option<String> rangeEnd,
       InstantRange.RangeType rangeType,
       Function<String, String> earliestInstantTimeFunc) {
-    throw new RuntimeException("Incremental query view for timeline version 1 not yet implemented");
+    final boolean startFromEarliest = START_COMMIT_EARLIEST.equalsIgnoreCase(rangeStart.orElse(null));
+    HoodieTimeline completedTimeline = timeline.filterCompletedInstants();
+
+    if (rangeStart.isEmpty() && rangeEnd.isPresent()) {
+      // (_, end]: returns the last instant whose requested time is at or before 'end'.
+      return completedTimeline.getInstantsAsStream()
+          .filter(instant -> InstantComparison.compareTimestamps(instant.requestedTime(), LESSER_THAN_OR_EQUALS, rangeEnd.get()))
+          .max(Comparator.comparing(HoodieInstant::requestedTime))
+          .map(instant -> Collections.singletonList(instant.requestedTime()))
+          .orElse(Collections.emptyList());
+    }
+
+    if (startFromEarliest) {
+      // ['earliest', _): clear the start so it degenerates to consuming from the earliest instant.
+      rangeStart = Option.empty();
+    }
+
+    if (rangeStart.isEmpty() && rangeEnd.isEmpty()) {

Review Comment:
   🤖 nit: could you avoid mutating the `rangeStart` parameter here? When a reader reaches the second `if (rangeStart.isEmpty() && rangeEnd.isEmpty())` check they have to mentally track whether the earlier reassignment fired. Something like `Option<String> effectiveStart = startFromEarliest ? Option.empty() : rangeStart;` introduced upfront, then used in the downstream checks and the builder, would make the control flow self-evident.
   
   <sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag quality.</i></sub>



-- 
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: commits-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(common): query LAYOUT_VERSION_1 timelines in CompletionTimeQueryView by using instant time as completion time [hudi]

Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #19338:
URL: https://github.com/apache/hudi/pull/19338#issuecomment-5203503800

   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "db8d3a46afc8de0ff795d41ff8ec87acb759f3e9",
       "status" : "SUCCESS",
       "url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=15593",
       "triggerID" : "db8d3a46afc8de0ff795d41ff8ec87acb759f3e9",
       "triggerType" : "PUSH"
     }, {
       "hash" : "5779e8b8fec78fa16e6189bbe4d2c000da7742ec",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "5779e8b8fec78fa16e6189bbe4d2c000da7742ec",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * db8d3a46afc8de0ff795d41ff8ec87acb759f3e9 Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=15593) 
   * 5779e8b8fec78fa16e6189bbe4d2c000da7742ec UNKNOWN
   
   <details>
   <summary>Bot commands</summary>
     @hudi-bot supports the following commands:
   
    - `@hudi-bot run azure` re-run the last Azure build
   </details>


-- 
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: commits-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(common): query LAYOUT_VERSION_1 timelines in CompletionTimeQueryView by using instant time as completion time [hudi]

Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #19338:
URL: https://github.com/apache/hudi/pull/19338#issuecomment-5033952949

   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "db8d3a46afc8de0ff795d41ff8ec87acb759f3e9",
       "status" : "SUCCESS",
       "url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=15593",
       "triggerID" : "db8d3a46afc8de0ff795d41ff8ec87acb759f3e9",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * db8d3a46afc8de0ff795d41ff8ec87acb759f3e9 Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=15593) 
   
   <details>
   <summary>Bot commands</summary>
     @hudi-bot supports the following commands:
   
    - `@hudi-bot run azure` re-run the last Azure build
   </details>


-- 
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: commits-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(common): query LAYOUT_VERSION_1 timelines in CompletionTimeQueryView by using instant time as completion time [hudi]

Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #19338:
URL: https://github.com/apache/hudi/pull/19338#issuecomment-5203553780

   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "db8d3a46afc8de0ff795d41ff8ec87acb759f3e9",
       "status" : "SUCCESS",
       "url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=15593",
       "triggerID" : "db8d3a46afc8de0ff795d41ff8ec87acb759f3e9",
       "triggerType" : "PUSH"
     }, {
       "hash" : "5779e8b8fec78fa16e6189bbe4d2c000da7742ec",
       "status" : "PENDING",
       "url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16138",
       "triggerID" : "5779e8b8fec78fa16e6189bbe4d2c000da7742ec",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * db8d3a46afc8de0ff795d41ff8ec87acb759f3e9 Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=15593) 
   * 5779e8b8fec78fa16e6189bbe4d2c000da7742ec Azure: [PENDING](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16138) 
   
   <details>
   <summary>Bot commands</summary>
     @hudi-bot supports the following commands:
   
    - `@hudi-bot run azure` re-run the last Azure build
   </details>


-- 
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: commits-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(common): query LAYOUT_VERSION_1 timelines in CompletionTimeQueryView by using instant time as completion time [hudi]

Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #19338:
URL: https://github.com/apache/hudi/pull/19338#issuecomment-5032964828

   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "db8d3a46afc8de0ff795d41ff8ec87acb759f3e9",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "db8d3a46afc8de0ff795d41ff8ec87acb759f3e9",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * db8d3a46afc8de0ff795d41ff8ec87acb759f3e9 UNKNOWN
   
   <details>
   <summary>Bot commands</summary>
     @hudi-bot supports the following commands:
   
    - `@hudi-bot run azure` re-run the last Azure build
   </details>


-- 
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: commits-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(common): query LAYOUT_VERSION_1 timelines in CompletionTimeQueryView by using instant time as completion time [hudi]

Posted by "fhan688 (via GitHub)" <gi...@apache.org>.
fhan688 commented on code in PR #19338:
URL: https://github.com/apache/hudi/pull/19338#discussion_r3727906689


##########
hudi-common/src/main/java/org/apache/hudi/common/table/timeline/versioning/v1/CompletionTimeQueryViewV1.java:
##########
@@ -205,7 +216,39 @@ private List<String> getInstantTimes(
       Option<String> rangeEnd,
       InstantRange.RangeType rangeType,
       Function<String, String> earliestInstantTimeFunc) {
-    throw new RuntimeException("Incremental query view for timeline version 1 not yet implemented");
+    final boolean startFromEarliest = START_COMMIT_EARLIEST.equalsIgnoreCase(rangeStart.orElse(null));
+    HoodieTimeline completedTimeline = timeline.filterCompletedInstants();

Review Comment:
   > This turns the archived-offset case from a hard failure into a silent skip. Everything below only sees the active timeline, so if a streaming job was down long enough for archival to pass its resume offset, the commits between `rangeStart` and the first active instant just vanish from the read. The V2 view covers that window by lazily loading the archive; the old `RuntimeException` here at least made the gap loud. (Same story for the `(_, end]` branch: V2 falls back to the archived map, this returns empty.)
   > 
   > The view already has `isArchived()`, so a cheap guard keeps the fail-stop semantics:
   > 
   > ```java
   > if (rangeStart.isPresent() && !startFromEarliest && isArchived(rangeStart.get())) {
   >   throw new HoodieException("Start instant " + rangeStart.get()
   >       + " is already archived; incremental read on a LAYOUT_VERSION_1 timeline does not consult the archived timeline");
   > }
   > ```
   > 
   > Reading the archived timeline would be even better (`analyze()` already handles archived instants via `getArchivedReadTimeline`), but at minimum a loud failure beats silent data loss on resume.
   
   Thanks for catching this. I updated the V1 query view to load archived instants on demand, so archived offsets are no longer silently skipped. I also added tests covering archived/active ranges and the end-only fallback.



-- 
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: commits-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(common): query LAYOUT_VERSION_1 timelines in CompletionTimeQueryView by using instant time as completion time [hudi]

Posted by "danny0405 (via GitHub)" <gi...@apache.org>.
danny0405 merged PR #19338:
URL: https://github.com/apache/hudi/pull/19338


-- 
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: commits-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(common): query LAYOUT_VERSION_1 timelines in CompletionTimeQueryView by using instant time as completion time [hudi]

Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #19338:
URL: https://github.com/apache/hudi/pull/19338#issuecomment-5033011171

   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "db8d3a46afc8de0ff795d41ff8ec87acb759f3e9",
       "status" : "PENDING",
       "url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=15593",
       "triggerID" : "db8d3a46afc8de0ff795d41ff8ec87acb759f3e9",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * db8d3a46afc8de0ff795d41ff8ec87acb759f3e9 Azure: [PENDING](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=15593) 
   
   <details>
   <summary>Bot commands</summary>
     @hudi-bot supports the following commands:
   
    - `@hudi-bot run azure` re-run the last Azure build
   </details>


-- 
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: commits-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(common): query LAYOUT_VERSION_1 timelines in CompletionTimeQueryView by using instant time as completion time [hudi]

Posted by "fhan688 (via GitHub)" <gi...@apache.org>.
fhan688 commented on code in PR #19338:
URL: https://github.com/apache/hudi/pull/19338#discussion_r3727908220


##########
hudi-common/src/main/java/org/apache/hudi/common/table/timeline/versioning/v1/CompletionTimeQueryViewV1.java:
##########
@@ -205,7 +216,39 @@ private List<String> getInstantTimes(
       Option<String> rangeEnd,
       InstantRange.RangeType rangeType,
       Function<String, String> earliestInstantTimeFunc) {
-    throw new RuntimeException("Incremental query view for timeline version 1 not yet implemented");
+    final boolean startFromEarliest = START_COMMIT_EARLIEST.equalsIgnoreCase(rangeStart.orElse(null));
+    HoodieTimeline completedTimeline = timeline.filterCompletedInstants();
+
+    if (rangeStart.isEmpty() && rangeEnd.isPresent()) {
+      // (_, end]: returns the last instant whose requested time is at or before 'end'.
+      return completedTimeline.getInstantsAsStream()
+          .filter(instant -> InstantComparison.compareTimestamps(instant.requestedTime(), LESSER_THAN_OR_EQUALS, rangeEnd.get()))
+          .max(Comparator.comparing(HoodieInstant::requestedTime))
+          .map(instant -> Collections.singletonList(instant.requestedTime()))
+          .orElse(Collections.emptyList());
+    }
+
+    if (startFromEarliest) {
+      // ['earliest', _): clear the start so it degenerates to consuming from the earliest instant.
+      rangeStart = Option.empty();
+    }
+
+    if (rangeStart.isEmpty() && rangeEnd.isEmpty()) {

Review Comment:
   Good suggestion. I replaced the rangeStart reassignment with an effectiveStart variable to make the range handling clearer.



-- 
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: commits-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(common): query LAYOUT_VERSION_1 timelines in CompletionTimeQueryView by using instant time as completion time [hudi]

Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #19338:
URL: https://github.com/apache/hudi/pull/19338#issuecomment-5204425160

   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "db8d3a46afc8de0ff795d41ff8ec87acb759f3e9",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=15593",
       "triggerID" : "db8d3a46afc8de0ff795d41ff8ec87acb759f3e9",
       "triggerType" : "PUSH"
     }, {
       "hash" : "5779e8b8fec78fa16e6189bbe4d2c000da7742ec",
       "status" : "SUCCESS",
       "url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16138",
       "triggerID" : "5779e8b8fec78fa16e6189bbe4d2c000da7742ec",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * 5779e8b8fec78fa16e6189bbe4d2c000da7742ec Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16138) 
   
   <details>
   <summary>Bot commands</summary>
     @hudi-bot supports the following commands:
   
    - `@hudi-bot run azure` re-run the last Azure build
   </details>


-- 
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: commits-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(common): query LAYOUT_VERSION_1 timelines in CompletionTimeQueryView by using instant time as completion time [hudi]

Posted by "voonhous (via GitHub)" <gi...@apache.org>.
voonhous commented on code in PR #19338:
URL: https://github.com/apache/hudi/pull/19338#discussion_r3643762519


##########
hudi-common/src/main/java/org/apache/hudi/common/table/timeline/versioning/v1/CompletionTimeQueryViewV1.java:
##########
@@ -205,7 +216,39 @@ private List<String> getInstantTimes(
       Option<String> rangeEnd,
       InstantRange.RangeType rangeType,
       Function<String, String> earliestInstantTimeFunc) {
-    throw new RuntimeException("Incremental query view for timeline version 1 not yet implemented");
+    final boolean startFromEarliest = START_COMMIT_EARLIEST.equalsIgnoreCase(rangeStart.orElse(null));
+    HoodieTimeline completedTimeline = timeline.filterCompletedInstants();

Review Comment:
   This turns the archived-offset case from a hard failure into a silent skip. Everything below only sees the active timeline, so if a streaming job was down long enough for archival to pass its resume offset, the commits between `rangeStart` and the first active instant just vanish from the read. The V2 view covers that window by lazily loading the archive; the old `RuntimeException` here at least made the gap loud. (Same story for the `(_, end]` branch: V2 falls back to the archived map, this returns empty.)
   
   The view already has `isArchived()`, so a cheap guard keeps the fail-stop semantics:
   
   ```java
   if (rangeStart.isPresent() && !startFromEarliest && isArchived(rangeStart.get())) {
     throw new HoodieException("Start instant " + rangeStart.get()
         + " is already archived; incremental read on a LAYOUT_VERSION_1 timeline does not consult the archived timeline");
   }
   ```
   
   Reading the archived timeline would be even better (`analyze()` already handles archived instants via `getArchivedReadTimeline`), but at minimum a loud failure beats silent data loss on resume.



-- 
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: commits-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(common): query LAYOUT_VERSION_1 timelines in CompletionTimeQueryView by using instant time as completion time [hudi]

Posted by "voonhous (via GitHub)" <gi...@apache.org>.
voonhous commented on code in PR #19338:
URL: https://github.com/apache/hudi/pull/19338#discussion_r3643772805


##########
hudi-common/src/main/java/org/apache/hudi/common/table/timeline/versioning/v1/CompletionTimeQueryViewV1.java:
##########
@@ -205,7 +216,39 @@ private List<String> getInstantTimes(
       Option<String> rangeEnd,
       InstantRange.RangeType rangeType,
       Function<String, String> earliestInstantTimeFunc) {
-    throw new RuntimeException("Incremental query view for timeline version 1 not yet implemented");
+    final boolean startFromEarliest = START_COMMIT_EARLIEST.equalsIgnoreCase(rangeStart.orElse(null));
+    HoodieTimeline completedTimeline = timeline.filterCompletedInstants();
+
+    if (rangeStart.isEmpty() && rangeEnd.isPresent()) {
+      // (_, end]: returns the last instant whose requested time is at or before 'end'.
+      return completedTimeline.getInstantsAsStream()
+          .filter(instant -> InstantComparison.compareTimestamps(instant.requestedTime(), LESSER_THAN_OR_EQUALS, rangeEnd.get()))
+          .max(Comparator.comparing(HoodieInstant::requestedTime))
+          .map(instant -> Collections.singletonList(instant.requestedTime()))
+          .orElse(Collections.emptyList());
+    }
+
+    if (startFromEarliest) {
+      // ['earliest', _): clear the start so it degenerates to consuming from the earliest instant.
+      rangeStart = Option.empty();
+    }
+
+    if (rangeStart.isEmpty() && rangeEnd.isEmpty()) {

Review Comment:
   +1



-- 
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: commits-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(common): query LAYOUT_VERSION_1 timelines in CompletionTimeQueryView by using instant time as completion time [hudi]

Posted by "danny0405 (via GitHub)" <gi...@apache.org>.
danny0405 commented on PR #19338:
URL: https://github.com/apache/hudi/pull/19338#issuecomment-5211158987

   A potential improvement for v1 archived timeline:
   
   The latest PR already performs a start-instant-based archive lookup:
   
   ```java
   metaClient.getArchivedTimeline(startInstant, false)
   ```
   
   For V1, the missing improvement is to make that the actual incremental-query path, not just the candidate-discovery path. Currently:
   
   1. `CompletionTimeQueryViewV1` loads archived instants from the requested start time.
   2. `IncrementalQueryAnalyzer` discards that timeline and loads it again.
   
   A cleaner V1 design would have `IncrementalQueryAnalyzer` load the filtered archived timeline once, apply `InstantRange` directly to `HoodieInstant.requestedTime()`, and reuse that same timeline in `QueryContext`. This is valid because V1 has:
   
   ```text
   completion time == requested/start instant time
   ```
   
   That would preserve V2-compatible range semantics while eliminating the redundant archive scan. Simply adding another start-time lookup inside `CompletionTimeQueryViewV1` would not address the concern—the PR already has that part.


-- 
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: commits-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(common): query LAYOUT_VERSION_1 timelines in CompletionTimeQueryView by using instant time as completion time [hudi]

Posted by "fhan688 (via GitHub)" <gi...@apache.org>.
fhan688 commented on PR #19338:
URL: https://github.com/apache/hudi/pull/19338#issuecomment-5177007057

   please help review, thanks! @danny0405 


-- 
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: commits-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org