IGNITE-28737 Calcite. CacheOperationContext with keepBinary = true need to be used during dml operations#13215
IGNITE-28737 Calcite. CacheOperationContext with keepBinary = true need to be used during dml operations#13215zstan wants to merge 13 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates cache operation context handling to ensure Calcite DML operations run with the correct binary semantics (avoid unintended binary deserialization/unwrap) by introducing a Calcite-engine execution flag in CacheOperationContext, propagating it through transactional/lock/atomic update flows, and using it during DML execution.
Changes:
- Introduces
CacheOperationContext.withCalciteEngine()and propagates acalciteOpCallflag through tx entries, lock requests/futures, and atomic update requests to influence binary unwrapping behavior. - Refactors several cache projection/context APIs (e.g.,
setSkipStore(boolean)→withSkipStore(),keepBinary()return type adjustments, builder usage) and updates call sites accordingly. - Adds Calcite public API integration coverage for DML +
withKeepBinary()and adds/updates SQL JMH benchmarks; removeschecker-qualfrom Calcite module POM.
Reviewed changes
Copilot reviewed 43 out of 43 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/dml/DmlUtils.java | Uses builder-based keep-binary context for H2 DML operation context. |
| modules/core/src/test/java/org/apache/ignite/util/TestStorageUtils.java | Adjusts test utility context typing around keepBinary() usage. |
| modules/core/src/main/java/org/apache/ignite/internal/processors/rest/handlers/cache/GridCacheCommandHandler.java | Updates skip-store projection usage to withSkipStore(). |
| modules/core/src/main/java/org/apache/ignite/internal/processors/datastructures/GridCacheQueueAdapter.java | Updates queue withKeepBinary() to use builder/withKeepBinary() context. |
| modules/core/src/main/java/org/apache/ignite/internal/processors/cache/transactions/IgniteTxLocalAdapter.java | Adds calciteOpCall parameter to tx entry enlistment to preserve binary behavior. |
| modules/core/src/main/java/org/apache/ignite/internal/processors/cache/transactions/IgniteTxEntry.java | Stores and exposes calciteOpCall flag on tx entries via bit mask. |
| modules/core/src/main/java/org/apache/ignite/internal/processors/cache/IgniteInternalCache.java | Updates internal cache projection APIs (skip-store, keep-binary signature) and adds withCalciteEngine(). |
| modules/core/src/main/java/org/apache/ignite/internal/processors/cache/IgniteCacheProxyImpl.java | Uses CacheOperationContext.instance() for gateway-wrapped proxies. |
| modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheProxyImpl.java | Updates projection methods to new context APIs and adds withCalciteEngine(). |
| modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheAdapter.java | Updates projection/context creation to builder style and adds withCalciteEngine(). |
| modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GatewayProtectedCacheProxy.java | Updates context mutation APIs used by public proxy projections (skip-store, keep-binary, etc.). |
| modules/core/src/main/java/org/apache/ignite/internal/processors/cache/distributed/near/GridNearTxLocal.java | Propagates calciteOpCall through enlist/write/read flows and lockAll path. |
| modules/core/src/main/java/org/apache/ignite/internal/processors/cache/distributed/near/GridNearTransactionalCache.java | Passes Calcite flag into get/lock paths. |
| modules/core/src/main/java/org/apache/ignite/internal/processors/cache/distributed/near/GridNearOptimisticTxPrepareFuture.java | Minor Javadoc grammar tweak. |
| modules/core/src/main/java/org/apache/ignite/internal/processors/cache/distributed/near/GridNearLockRequest.java | Extends lock request ctor with calciteOpCall. |
| modules/core/src/main/java/org/apache/ignite/internal/processors/cache/distributed/near/GridNearLockFuture.java | Carries and forwards calciteOpCall in near lock future mapping. |
| modules/core/src/main/java/org/apache/ignite/internal/processors/cache/distributed/GridDistributedTxPrepareRequest.java | Refactors flag read to helper (isFlag). |
| modules/core/src/main/java/org/apache/ignite/internal/processors/cache/distributed/GridDistributedLockRequest.java | Adds Calcite flag bit + normalizes flag set/get helpers. |
| modules/core/src/main/java/org/apache/ignite/internal/processors/cache/distributed/dht/GridDhtTxPrepareFuture.java | Uses calciteOpCall to drive keep-binary behavior for invoke results. |
| modules/core/src/main/java/org/apache/ignite/internal/processors/cache/distributed/dht/GridDhtTxLocalAdapter.java | Adds calciteOpCall plumbing to DHT lock acquisition path. |
| modules/core/src/main/java/org/apache/ignite/internal/processors/cache/distributed/dht/GridDhtTransactionalCacheAdapter.java | Propagates Calcite flag through transactional cache locking APIs. |
| modules/core/src/main/java/org/apache/ignite/internal/processors/cache/distributed/dht/GridDhtLockRequest.java | Extends lock request ctor with calciteOpCall. |
| modules/core/src/main/java/org/apache/ignite/internal/processors/cache/distributed/dht/GridDhtLockFuture.java | Carries and forwards calciteOpCall for DHT lock mapping. |
| modules/core/src/main/java/org/apache/ignite/internal/processors/cache/distributed/dht/colocated/GridDhtColocatedLockFuture.java | Carries/forwards calciteOpCall and simplifies mapping bookkeeping. |
| modules/core/src/main/java/org/apache/ignite/internal/processors/cache/distributed/dht/colocated/GridDhtColocatedCache.java | Propagates Calcite flag through colocated cache lock paths. |
| modules/core/src/main/java/org/apache/ignite/internal/processors/cache/distributed/dht/atomic/GridNearAtomicUpdateFuture.java | Adds Calcite flag to atomic update request flags and plumbs from opCtx. |
| modules/core/src/main/java/org/apache/ignite/internal/processors/cache/distributed/dht/atomic/GridNearAtomicSingleUpdateFuture.java | Adds Calcite flag to atomic single-update request flags and plumbs from opCtx. |
| modules/core/src/main/java/org/apache/ignite/internal/processors/cache/distributed/dht/atomic/GridNearAtomicAbstractUpdateRequest.java | Adds Calcite flag bit to atomic update request flags and exposes accessor. |
| modules/core/src/main/java/org/apache/ignite/internal/processors/cache/distributed/dht/atomic/GridNearAtomicAbstractUpdateFuture.java | Uses Calcite flag to control binary unwrap behavior and updates appAttrs context construction. |
| modules/core/src/main/java/org/apache/ignite/internal/processors/cache/distributed/dht/atomic/GridDhtAtomicCache.java | Passes Calcite flag through update futures and uses it when unwrapping invoke results. |
| modules/core/src/main/java/org/apache/ignite/internal/processors/cache/CacheOperationContext.java | Refactors operation context to builder-style creation and adds calciteEngine flag support. |
| modules/calcite/src/test/java/org/apache/ignite/testsuites/IntegrationTestSuite.java | Adds new Calcite public API integration test to suite. |
| modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/PublicApiIntegrationTest.java | New integration test validating DML via public API with/without withKeepBinary(). |
| modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/rel/ModifyNode.java | Ensures DML cache projection is marked with Calcite engine execution flag. |
| modules/calcite/pom.xml | Removes Checker Framework qualifier dependency/property. |
| modules/benchmarks/src/main/java/org/apache/ignite/internal/benchmarks/jmh/sql/JmhSqlUdfBenchmark.java | Adds standard JMH annotations/config for benchmark runs. |
| modules/benchmarks/src/main/java/org/apache/ignite/internal/benchmarks/jmh/sql/JmhSqlSortBenchmark.java | Adds standard JMH annotations/config for benchmark runs. |
| modules/benchmarks/src/main/java/org/apache/ignite/internal/benchmarks/jmh/sql/JmhSqlSetOpBenchmark.java | Adds standard JMH annotations/config for benchmark runs. |
| modules/benchmarks/src/main/java/org/apache/ignite/internal/benchmarks/jmh/sql/JmhSqlScanBenchmark.java | Adds standard JMH annotations/config for benchmark runs. |
| modules/benchmarks/src/main/java/org/apache/ignite/internal/benchmarks/jmh/sql/JmhSqlInsertBenchmark.java | New benchmark comparing SQL insert API usage patterns. |
| modules/benchmarks/src/main/java/org/apache/ignite/internal/benchmarks/jmh/sql/JmhSqlDmlBenchmark.java | Adds standard JMH annotations/config for benchmark runs. |
| modules/benchmarks/src/main/java/org/apache/ignite/internal/benchmarks/jmh/sql/JmhSqlCorrelateBenchmark.java | Adds standard JMH annotations/config for benchmark runs. |
| modules/benchmarks/src/main/java/org/apache/ignite/internal/benchmarks/jmh/sql/JmhSqlAggBenchmark.java | Adds standard JMH annotations/config for benchmark runs. |
Comments suppressed due to low confidence (1)
modules/core/src/test/java/org/apache/ignite/util/TestStorageUtils.java:55
corruptDataEntryswitchesGridCacheContext<?, ?>to the raw typeGridCacheContext, which drops generic type safety and can hide real type issues. It looks like this was done to makegetEntry(key)compile; consider keeping the wildcard generic and doing a local cast for the one call site instead.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 43 out of 43 changed files in this pull request and generated 9 comments.
Comments suppressed due to low confidence (1)
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/CacheOperationContext.java:29
- Unused import
org.apache.ignite.internal.util.typedef.internal.Ushould be removed to avoid style/lint failures (and it adds noise to future diffs).
import org.apache.ignite.internal.util.typedef.internal.S;
import org.jetbrains.annotations.Nullable;
| if (opCtx != null) { | ||
| if (opCtx.isKeepBinary()) | ||
| return this; | ||
| else | ||
| opCtx = opCtx.withKeepBinary(); |
| if (opCtx != null) { | ||
| if (opCtx.skipStore()) | ||
| return this; | ||
| else | ||
| opCtx = opCtx.withSkipStore(); |
| if (opCtx != null) { | ||
| if (opCtx.isKeepBinary()) | ||
| return this; | ||
| else | ||
| opCtx = opCtx.withKeepBinary(); |
| import org.jetbrains.annotations.Nullable; | ||
|
|
||
| import static org.junit.Assert.assertNotNull; | ||
|
|
No description provided.