[kudu-CR] [Java] KUDU-3800: Add diff-scan option to return unobservable rows

["Abhishek Chennaka (Code Review)" <ge...@cloudera.org>]

Abhishek Chennaka has posted comments on this change. ( http://gerrit.cloudera.org:8080/24725 )

Change subject: [Java] KUDU-3800: Add diff-scan option to return unobservable rows
......................................................................


Patch Set 3:

(2 comments)

> Looks like org.apache.kudu.client.TestScanToken and org.apache.kudu.client.TestKuduScanner are failing across build types.

The tests are now fixed. Thanks!

http://gerrit.cloudera.org:8080/#/c/24725/2/java/kudu-client/src/main/java/org/apache/kudu/client/AbstractKuduScannerBuilder.java
File java/kudu-client/src/main/java/org/apache/kudu/client/AbstractKuduScannerBuilder.java:

http://gerrit.cloudera.org:8080/#/c/24725/2/java/kudu-client/src/main/java/org/apache/kudu/client/AbstractKuduScannerBuilder.java@299
PS2, Line 299:   public S diffScan(long startTimestamp, long endTimestamp,
> Thanks for looking into this. But as far as I see for now we need the overl
Ah looks like the flink connector uses this method. Let me restore it to avoid complications. Will leave it as is without marking it deprecated. It's a little odd to mark something deprecated which is not public facing I think.


http://gerrit.cloudera.org:8080/#/c/24725/3/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduScanner.java
File java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduScanner.java:

http://gerrit.cloudera.org:8080/#/c/24725/3/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduScanner.java@754
PS3, Line 754:   public void testDiffScanNullRowVisibilityRejected() throws Exception {
> Could we mirror this on newScanTokenBuilder too? Note the two halves won't 
Good catch and the issue needed a bit more addressing than a missing guard. build() writes row_visibility only inside the startTimestamp != NO_TIMESTAMP branch, so INCLUDE_UNOBSERVABLE without a window was silently defaulted to to OBSERVABLE_ONLY at serialization time and nothing reached the server to reject. It's publicly reachable via diffScan(NO_TIMESTAMP, end, INCLUDE_UNOBSERVABLE). build() now repeats that check with the scanner's message.

I did not mirror the null check as it is unreachable on either path - diffScan() throws before assigning, the field initializer is OBSERVABLE_ONLY and there's no other mutator, so only reflection or an in-package edit can null it. A second guard would imply the field is nullable when it isn't.

Test mirrored and renamed to testDiffScanRowVisibilityValidation, covering both builders for the window check and for diffScan(…, null).



-- 
To view, visit http://gerrit.cloudera.org:8080/24725
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9d61b2e942c9b2de1b46007f40311d1fb373104a
Gerrit-Change-Number: 24725
Gerrit-PatchSet: 3
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Gabriella Lotz <lo...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Comment-Date: Thu, 01 Oct 2026 19:17:16 +0000
Gerrit-HasComments: Yes