IGNITE-28737 Calcite. CacheOperationContext with keepBinary = true need to be used during dml operations#13198
Open
zstan wants to merge 6 commits into
Open
IGNITE-28737 Calcite. CacheOperationContext with keepBinary = true need to be used during dml operations#13198zstan wants to merge 6 commits into
zstan wants to merge 6 commits into
Conversation
Contributor
Author
|
No diff are found: |
7d05287 to
765fa5d
Compare
Contributor
TCBot Test Analysis
Possible Blockers (0)No blockers found. New Tests (1)
|
Contributor
TCBot Test Analysis
Possible Blockers (0)No blockers found. New Tests (2)
|
There was a problem hiding this comment.
Pull request overview
This PR targets Calcite DML execution with keepBinary=true, aiming to avoid unnecessary binary deserialization (especially around entry-processor-based DML) and to add coverage ensuring DML works via the public IgniteCache API in both regular and transactional modes.
Changes:
- Adjust
keepBinaryhandling for entry-processor/invoke results in several DHT/atomic update code paths. - Add a new Calcite integration test (
PublicApiIntegrationTest) and include it in the CalciteIntegrationTestSuite. - Add Calcite-side utility helpers for forcing/restoring
CacheOperationContextkeep-binary mode; plus minor build/benchmark updates.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| modules/core/src/main/java/org/apache/ignite/internal/processors/cache/distributed/dht/GridDhtTxPrepareFuture.java | Tweaks keep-binary behavior when recording entry-processor results during tx prepare. |
| modules/core/src/main/java/org/apache/ignite/internal/processors/cache/distributed/dht/atomic/GridNearAtomicAbstractUpdateFuture.java | Adjusts binary unwrapping behavior for atomic update completion (TRANSFORM path). |
| modules/core/src/main/java/org/apache/ignite/internal/processors/cache/distributed/dht/atomic/GridDhtAtomicCache.java | Changes how keep-binary is applied when adding computed entry-processor results. |
| modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/util/Commons.java | Adds helpers to force/restore keep-binary CacheOperationContext for Calcite operations. |
| modules/calcite/src/test/java/org/apache/ignite/testsuites/IntegrationTestSuite.java | Registers the new public API integration test in the Calcite suite. |
| modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/PublicApiIntegrationTest.java | New test covering DML via IgniteCache with and without withKeepBinary(), including tx case. |
| modules/calcite/pom.xml | Removes direct checker-qual dependency/property. |
| modules/benchmarks/src/main/java/org/apache/ignite/internal/benchmarks/jmh/sql/JmhSqlUdfBenchmark.java | Adds standard class-level JMH annotations. |
| modules/benchmarks/src/main/java/org/apache/ignite/internal/benchmarks/jmh/sql/JmhSqlSortBenchmark.java | Adds standard class-level JMH annotations. |
| modules/benchmarks/src/main/java/org/apache/ignite/internal/benchmarks/jmh/sql/JmhSqlSetOpBenchmark.java | Adds standard class-level JMH annotations. |
| modules/benchmarks/src/main/java/org/apache/ignite/internal/benchmarks/jmh/sql/JmhSqlScanBenchmark.java | Adds standard class-level JMH annotations. |
| modules/benchmarks/src/main/java/org/apache/ignite/internal/benchmarks/jmh/sql/JmhSqlDmlBenchmark.java | Adds standard class-level JMH annotations. |
| modules/benchmarks/src/main/java/org/apache/ignite/internal/benchmarks/jmh/sql/JmhSqlCorrelateBenchmark.java | Adds standard class-level JMH annotations. |
| modules/benchmarks/src/main/java/org/apache/ignite/internal/benchmarks/jmh/sql/JmhSqlAggBenchmark.java | Adds standard class-level JMH annotations. |
| modules/benchmarks/src/main/java/org/apache/ignite/internal/benchmarks/jmh/sql/JmhSqlInsertBenchmark.java | New JMH benchmark comparing SQL insert APIs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+82
to
+103
| private void runQuery(int begin, int end, boolean transactional, IgniteCache<?, ?> cache) { | ||
| Transaction tx = null; | ||
|
|
||
| cache.query(new SqlFieldsQuery("CREATE TABLE IF NOT EXISTS emp(empid INTEGER, deptid INTEGER, name VARCHAR, salary INTEGER, " + | ||
| "PRIMARY KEY(empid, deptid)) WITH \"AFFINITY_KEY=deptid" + (transactional ? ", ATOMICITY=transactional" : "") + "\"")); | ||
|
|
||
| if (transactional) { | ||
| //noinspection resource | ||
| tx = client.transactions().txStart(PESSIMISTIC, READ_COMMITTED); | ||
| } | ||
|
|
||
| for (int i = begin; i < end; i++) { | ||
| cache.query(new SqlFieldsQuery("INSERT INTO emp (empid, deptid, name, salary) VALUES (?, ?, ?, ?)").setArgs( | ||
| i, i % 2, "Employee " + i, i / 10)); | ||
|
|
||
| cache.query(new SqlFieldsQuery("UPDATE emp SET name = '' WHERE empid = ? AND deptid = ?").setArgs(i, i % 2)); | ||
| cache.query(new SqlFieldsQuery("DELETE FROM emp WHERE empid = ?").setArgs(i - 1)).getAll(); | ||
| } | ||
|
|
||
| if (transactional) | ||
| tx.commit(); | ||
| } |
Comment on lines
+581
to
+597
| public static CacheOperationContext setKeepBinaryContext(GridCacheContext<?, ?> cctx) { | ||
| CacheOperationContext opCtx = cctx.operationContextPerCall(); | ||
|
|
||
| // Force keepBinary for operation context to avoid binary deserialization inside entry processor | ||
| CacheOperationContext newOpCtx = null; | ||
|
|
||
| if (opCtx == null) | ||
| // Mimics behavior of GridCacheAdapter#keepBinary and GridCacheProxyImpl#keepBinary | ||
| newOpCtx = new CacheOperationContext(false, false, true, null, false, null, false, null, null); | ||
| else if (!opCtx.isKeepBinary()) | ||
| newOpCtx = opCtx.keepBinary(); | ||
|
|
||
| if (newOpCtx != null) | ||
| cctx.operationContextPerCall(newOpCtx); | ||
|
|
||
| return opCtx; | ||
| } |
Comment on lines
2693
to
+2698
| retVal.addEntryProcessResult(ctx, | ||
| k, | ||
| null, | ||
| compRes.get1(), | ||
| compRes.get2(), | ||
| req.keepBinary()); | ||
| true); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.