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]