Copilot commented on code in PR #13241:
URL: https://github.com/apache/ignite/pull/13241#discussion_r3413844552


##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/CacheOperationContext.java:
##########
@@ -139,7 +151,27 @@ public CacheOperationContext keepBinary() {
             dataCenterId,
             recovery,
             readRepairStrategy,
-            appAttrs);
+            appAttrs,
+            handleBinaryInInterceptor);
+    }
+
+    /**
+     * See {@link IgniteInternalCache#keepBinary()}.
+     *
+     * @return New instance of CacheOperationContext with handle binary in 
interceptor flag.
+     */

Review Comment:
   Javadoc for withHandleBinaryInInterceptor() references 
IgniteInternalCache#keepBinary(), which is misleading and makes it harder to 
understand the purpose of this flag.



##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheProxyImpl.java:
##########
@@ -1592,7 +1597,33 @@ public IgniteInternalCache<K, V> delegate() {
                         null,
                         false,
                         null,
-                        null));
+                        null,
+                        false));
+        }
+        finally {
+            gate.leave(prev);
+        }
+    }
+
+    /**
+     * @return Cache with handle binary values during {@link CacheInterceptor} 
execution flag.
+     */
+    @Override public IgniteInternalCache<K, V> withHandleBinaryInInterceptor() 
{
+        CacheOperationContext prev = gate.enter(opCtx);
+
+        try {
+            return new GridCacheProxyImpl<>(ctx, delegate,
+                new CacheOperationContext(
+                    false,
+                    false,
+                    false,
+                    null,
+                    false,
+                    null,
+                    false,
+                    null,
+                    null,
+                    true));

Review Comment:
   withHandleBinaryInInterceptor() creates a brand-new CacheOperationContext 
and drops any existing per-call flags (notably keepBinary). This breaks call 
chaining like cache.keepBinary().withHandleBinaryInInterceptor(), which will 
silently lose keepBinary and can change DML execution behavior.



##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/distributed/dht/atomic/GridDhtAtomicAbstractUpdateRequest.java:
##########
@@ -58,6 +58,9 @@ public abstract class GridDhtAtomicAbstractUpdateRequest 
extends GridCacheIdMess
     /** Flag indicating recovery on read repair. */
     protected static final int DHT_ATOMIC_READ_REPAIR_RECOVERY_FLAG_MASK = 
0x80;
 
+    /** */
+    protected static final int 
DHT_ATOMIC_UNWRAP_VALUE_ON_INTERCEPTOR_FLAG_MASK = 0x100;
+

Review Comment:
   The flag mask name says "UNWRAP_VALUE" but it is set/read by 
handleBinaryInInterceptor(). This naming mismatch is confusing and may lead to 
incorrect future usage (it reads like the opposite of what the API name 
suggests). Consider renaming the constant to match the accessor name (e.g., 
*_HANDLE_BINARY_IN_INTERCEPTOR_*).



##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/transactions/IgniteTxEntry.java:
##########
@@ -618,12 +624,24 @@ public void skipReadThrough(boolean skipReadThrough) {
     }
 
     /**
-     * @return Skip store flag.
+     * @return Skip read through flag.
      */
     public boolean skipReadThrough() {
         return isFlag(TX_ENTRY_SKIP_READ_THROUGH_FLAG_MASK);
     }
 
+    /** Sets whenever to handle binary in interceptor flag. */

Review Comment:
   Typo in the new comment: "Sets whenever" should be "Sets whether".



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