Copilot commented on code in PR #7319:
URL: https://github.com/apache/ignite-3/pull/7319#discussion_r2648306337


##########
modules/api/src/main/java/org/apache/ignite/table/TupleImpl.java:
##########
@@ -437,4 +437,12 @@ public String toString() {
 
         return b.toString();
     }
+
+    private static <T> T ensureNotNull(T value) {
+        if (value == null) {
+            throw new NullPointerException("The field value is null and cannot 
be converted to a primitive data type.");

Review Comment:
   The error message is hardcoded here instead of using 
`BinaryTupleReader.NULL_TO_PRIMITIVE_ERROR_MESSAGE` constant. While this may be 
due to module dependencies (api module doesn't depend on binary-tuple), it 
creates a maintenance burden where the message must be kept in sync manually. 
Consider either:
   1. Moving the constant to a common module accessible by both, or
   2. Adding a test that verifies both error messages are identical to catch 
any future drift.



##########
modules/schema/src/testFixtures/java/org/apache/ignite/internal/schema/SchemaTestUtils.java:
##########
@@ -213,4 +221,28 @@ private static Instant generateInstant(Random rnd, 
TemporalNativeType type) {
         return Instant.ofEpochMilli(minTs + (long) (rnd.nextDouble() * (maxTs 
- minTs))).truncatedTo(ChronoUnit.SECONDS)
                 .plusNanos(normalizeNanos(rnd.nextInt(1_000_000_000), 
type.precision()));
     }
+
+    private static void invoke(Object index, IntConsumer intConsumer, 
Consumer<String> strConsumer) {
+        if (index instanceof Integer) {
+            intConsumer.accept((int) index);
+        }
+
+        assert index instanceof String : index.getClass();
+
+        strConsumer.accept((String) index);
+    }

Review Comment:
   The control flow in this method is incorrect. When `index` is an Integer, 
the intConsumer is called and executed, but then the code continues to the 
assertion and strConsumer call. There should be a `return` statement after line 
227, or the code should use an if-else structure. Currently, if index is an 
Integer, both consumers will be invoked.



-- 
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