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]

Reply via email to