Copilot commented on code in PR #7153:
URL: https://github.com/apache/ignite-3/pull/7153#discussion_r2590630635
##########
modules/runner/src/integrationTest/java/org/apache/ignite/internal/runner/app/client/ItThinClientComputeTest.java:
##########
@@ -1172,4 +1172,12 @@ public CompletableFuture<Void>
executeAsync(JobExecutionContext context, Tuple a
return
context.ignite().tables().table(TABLE_NAME).recordView().upsertAllAsync(null,
tuples);
}
}
+
+ /** Simple job which always return null value. */
Review Comment:
Grammar issue: "return" should be "returns" (third person singular present
tense) to match standard JavaDoc conventions.
```suggestion
/** Simple job which always returns null value. */
```
##########
modules/runner/src/integrationTest/java/org/apache/ignite/internal/runner/app/client/ItThinClientComputeTest.java:
##########
@@ -1172,4 +1172,12 @@ public CompletableFuture<Void>
executeAsync(JobExecutionContext context, Tuple a
return
context.ignite().tables().table(TABLE_NAME).recordView().upsertAllAsync(null,
tuples);
}
}
+
+ /** Simple job which always return null value. */
+ public static class ReturnNullJob implements ComputeJob<Void, String> {
+ @Override
+ public @Nullable CompletableFuture<String>
executeAsync(JobExecutionContext context, @Nullable Void arg) {
+ return null;
+ }
+ }
Review Comment:
The `ReturnNullJob` class is defined but never used in Java tests. Consider
adding a Java test case that verifies the null return behavior to ensure
comprehensive test coverage for both Java and C++ implementations. This would
help catch any issues with the Java client handling of null job results.
##########
modules/platforms/cpp/ignite/client/detail/compute/compute_impl.cpp:
##########
@@ -114,6 +114,11 @@ primitive
read_primitive_from_binary_tuple(protocol::reader &reader) {
* @return Value.
*/
primitive unpack_compute_result(protocol::reader &reader) {
+ // Check whether job returned null value
+ if (auto nil_val = reader.try_read_nil(); nil_val) {
Review Comment:
The variable `nil_val` is assigned but never used. The condition can be
simplified to `if (reader.try_read_nil())` like in `read_job_state_opt` at line
158. This follows the pattern used elsewhere in the file and avoids an unused
variable.
```suggestion
if (reader.try_read_nil()) {
```
--
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]