You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@phoenix.apache.org by "apurtell (via GitHub)" <gi...@apache.org> on 2026/06/12 21:03:36 UTC

[PR] PHOENIX-7916 Disclose atomic upsert flavors and RETURNING in EXPLAIN [phoenix]

apurtell opened a new pull request, #2524:
URL: https://github.com/apache/phoenix/pull/2524

   Disclose atomic upsert flavors and RETURNING in EXPLAIN.
   
   Replace the single opaque `PUT SINGLE ROW` operator line with a switch on `UpsertStatement.OnDuplicateKeyType`, surface the server-side update assignments under `ON DUPLICATE KEY UPDATE`, and disclose `RETURNING *` on every mutation plan whose parsed statement has `isReturningRow() == true` — that is, every concrete `UpsertCompiler` plan (`UpsertValuesMutationPlan`, `ServerUpsertSelectMutationPlan`, `ClientUpsertSelectMutationPlan`) and every `DeleteCompiler` plan (`SingleRowDeleteMutationPlan`, `ServerSelectDeleteMutationPlan`, `ClientSelectDeleteMutationPlan`).
   
   Add three new structured fields to `ExplainPlanAttributes` and matching fluent assertions on `ExplainPlanTestUtil`.
   
   Co-authored-by: Claude Opus 4.8[1m] <no...@anthropic.com>


-- 
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: issues-unsubscribe@phoenix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] PHOENIX-7916 Disclose atomic upsert flavors and RETURNING in EXPLAIN [phoenix]

Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #2524:
URL: https://github.com/apache/phoenix/pull/2524#discussion_r3406249551


##########
phoenix-core-client/src/main/java/org/apache/phoenix/compile/ExplainPlanAttributes.java:
##########
@@ -200,8 +209,9 @@ private ExplainPlanAttributes() {
 
   public ExplainPlanAttributes(String tenantId, String viewName, String viewBaseName,
     String cdcScopes, String txnProvider, List<String> rewrites, Long estimatedRows,
-    Long estimatedSizeInBytes, Long estimateInfoTs, String abstractExplainPlan, Hint hint,
-    String explainScanType, Consistency consistency, String tableName, String keyRanges,
+    Long estimatedSizeInBytes, Long estimateInfoTs, String abstractExplainPlan,
+    OnDuplicateKeyType onDuplicateKeyAction, List<String> serverUpdateSet, boolean returningRow,
+    Hint hint, String explainScanType, Consistency consistency, String tableName, String keyRanges,

Review Comment:
   The public ExplainPlanAttributes constructor signature was changed by inserting new parameters (onDuplicateKeyAction/serverUpdateSet/returningRow). Since this is a public API in phoenix-core-client, removing the previous signature is source/binary incompatible for any downstream callers that instantiate ExplainPlanAttributes directly. Consider restoring the previous constructor as an overload (deprecated if desired) that delegates to this new constructor with default mutation values.



##########
phoenix-core-client/src/main/java/org/apache/phoenix/compile/ExplainPlanAttributes.java:
##########
@@ -752,6 +787,22 @@ public ExplainPlanAttributesBuilder setAbstractExplainPlan(String abstractExplai
       return this;
     }
 
+    public ExplainPlanAttributesBuilder
+      setOnDuplicateKeyAction(OnDuplicateKeyType onDuplicateKeyAction) {
+      this.onDuplicateKeyAction = onDuplicateKeyAction;

Review Comment:
   The setOnDuplicateKeyAction() builder method signature is split across two lines even though it fits within the project's 100-character limit, unlike the surrounding builder setters. Keeping it on one line improves readability and consistency in this class.



-- 
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: issues-unsubscribe@phoenix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] PHOENIX-7916 Disclose atomic upsert flavors and RETURNING in EXPLAIN [phoenix]

Posted by "apurtell (via GitHub)" <gi...@apache.org>.
apurtell merged PR #2524:
URL: https://github.com/apache/phoenix/pull/2524


-- 
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: issues-unsubscribe@phoenix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] PHOENIX-7916 Disclose atomic upsert flavors and RETURNING in EXPLAIN [phoenix]

Posted by "apurtell (via GitHub)" <gi...@apache.org>.
apurtell commented on PR #2524:
URL: https://github.com/apache/phoenix/pull/2524#issuecomment-4695504724

   ## Test Results
   
   ### Unit tests (`*Test`) — all PASS
   
   | Test class | Tests | Failures | Errors | Skipped |
   | --- | --- | --- | --- | --- |
   | `ExplainPlanTest` | 94 | 0 | 0 | 0 |
   | `QueryCompilerTest` | 215 | 0 | 0 | 2 |
   | `QueryOptimizerTest` | 52 | 0 | 0 | 1 |
   | `QueryPlanTest` | 4 | 0 | 0 | 0 |
   | `StatementHintsCompilationTest` | 4 | 0 | 0 | 0 |
   | `JoinQueryCompilerTest` | 2 | 0 | 0 | 0 |
   | `TenantSpecificViewIndexCompileTest` | 6 | 0 | 0 | 0 |
   | **Total** | **377** | **0** | **0** | **3** |
   
   ### Integration tests (`*IT`) — all PASS
   
   | Test class | Tests | Failures | Errors | Skipped | Time (s) |
   | --- | --- | --- | --- | --- | --- |
   | `UpsertSelectIT` | 54 | 0 | 0 | 0 | 207.4 |
   | `DeleteIT` | 74 | 0 | 0 | 0 | 247.6 |
   | `OnDuplicateKeyIT` | 154 | 0 | 0 | 0 | 465.4 |
   | `GlobalIndexOptimizationIT` | 7 | 0 | 0 | 0 | 36.6 |
   | `CostBasedDecisionIT` | 20 | 0 | 0 | 0 | 176.6 |
   | `OnDuplicateKey2IT` | 252 | 0 | 0 | 32 | 422.5 |
   | **Total** | **561** | **0** | **0** | **32** | — |
   
   `OnDuplicateKeyIT` and `OnDuplicateKey2IT` directly exercise the rewritten atomic-upsert `getExplainPlan()` switch and the modified `UpsertValuesMutationPlan` constructor across the `NONE`/`UPDATE`/`UPDATE_ONLY`/`IGNORE` flavors. `UpsertSelectIT` exercises the modified `ServerUpsertSelectMutationPlan` / `ClientUpsertSelectMutationPlan` constructors. `DeleteIT` exercises all three modified `DeleteCompiler` plan constructors.
   


-- 
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: issues-unsubscribe@phoenix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org