You are viewing a plain text version of this content. The canonical link for it is here.

Posted to commits@gluten.apache.org by "lgbo-ustc (via GitHub)" <gi...@apache.org> on 2025/05/22 08:43:31 UTC

[PR] [GLUTEN-9728][CH] Fix: Inconsistent result from `JoinAggregateToAggregateUnion` [incubator-gluten]

lgbo-ustc opened a new pull request, #9729:
URL: https://github.com/apache/incubator-gluten/pull/9729

   
   Fix: Inconsistent result from `JoinAggregateToAggregateUnion`
   
   ## What changes were proposed in this pull request?
   
   (Please fill in changes proposed in this fix)
   
   Fixes: #9728 
   
   ## How was this patch tested?
   
   (Please explain how this patch was tested. E.g. unit tests, integration tests, manual tests)
   
    unit tests
   
   (If this patch involves UI changes, please attach a screenshot; otherwise, remove this)
   
   


-- 
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: commits-unsubscribe@gluten.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org

Re: [PR] [GLUTEN-9728][CH] Fix: Inconsistent result from `JoinAggregateToAggregateUnion` [incubator-gluten]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.

github-actions[bot] commented on PR #9729:
URL: https://github.com/apache/incubator-gluten/pull/9729#issuecomment-2900459019

   Run Gluten Clickhouse CI on x86


-- 
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: commits-unsubscribe@gluten.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org

Re: [PR] [GLUTEN-9728][CH] Fix: Inconsistent result from `JoinAggregateToAggregateUnion` [incubator-gluten]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.

github-actions[bot] commented on PR #9729:
URL: https://github.com/apache/incubator-gluten/pull/9729#issuecomment-2900399154

   Run Gluten Clickhouse CI on x86


-- 
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: commits-unsubscribe@gluten.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org

Re: [PR] [GLUTEN-9728][CH] Fix: Inconsistent result from `JoinAggregateToAggregateUnion` [incubator-gluten]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.

github-actions[bot] commented on PR #9729:
URL: https://github.com/apache/incubator-gluten/pull/9729#issuecomment-2902974291

   Run Gluten Clickhouse CI on x86


-- 
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: commits-unsubscribe@gluten.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org

Re: [PR] [GLUTEN-9728][CH] Fix: Inconsistent result from `JoinAggregateToAggregateUnion` [incubator-gluten]

Posted by "lgbo-ustc (via GitHub)" <gi...@apache.org>.

lgbo-ustc merged PR #9729:
URL: https://github.com/apache/incubator-gluten/pull/9729


-- 
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: commits-unsubscribe@gluten.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org

Re: [PR] [GLUTEN-9728][CH] Fix: Inconsistent result from `JoinAggregateToAggregateUnion` [incubator-gluten]

Posted by "lgbo-ustc (via GitHub)" <gi...@apache.org>.

lgbo-ustc commented on PR #9729:
URL: https://github.com/apache/incubator-gluten/pull/9729#issuecomment-2900442662

   @CodiumAI-Agent /review


-- 
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: commits-unsubscribe@gluten.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org

Re: [PR] [GLUTEN-9728][CH] Fix: Inconsistent result from `JoinAggregateToAggregateUnion` [incubator-gluten]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.

github-actions[bot] commented on PR #9729:
URL: https://github.com/apache/incubator-gluten/pull/9729#issuecomment-2900398414

   https://github.com/apache/incubator-gluten/issues/9728


-- 
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: commits-unsubscribe@gluten.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org

Re: [PR] [GLUTEN-9728][CH] Fix: Inconsistent result from `JoinAggregateToAggregateUnion` [incubator-gluten]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.

github-actions[bot] commented on PR #9729:
URL: https://github.com/apache/incubator-gluten/pull/9729#issuecomment-2903368970

   Run Gluten Clickhouse CI on x86


-- 
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: commits-unsubscribe@gluten.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org

Re: [PR] [GLUTEN-9728][CH] Fix: Inconsistent result from `JoinAggregateToAggregateUnion` [incubator-gluten]

Posted by "CodiumAI-Agent (via GitHub)" <gi...@apache.org>.

CodiumAI-Agent commented on PR #9729:
URL: https://github.com/apache/incubator-gluten/pull/9729#issuecomment-2900462381

   **[Persistent review](https://github.com/apache/incubator-gluten/pull/9729#issuecomment-2900444756)** updated to latest commit https://github.com/apache/incubator-gluten/commit/f7b0c872df0dfb1c99cbb4697747874f13973645


-- 
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: commits-unsubscribe@gluten.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org

Re: [PR] [GLUTEN-9728][CH] Fix: Inconsistent result from `JoinAggregateToAggregateUnion` [incubator-gluten]

Posted by "CodiumAI-Agent (via GitHub)" <gi...@apache.org>.

CodiumAI-Agent commented on PR #9729:
URL: https://github.com/apache/incubator-gluten/pull/9729#issuecomment-2900444756

   ## PR Reviewer Guide 🔍
   
   Here are some key observations to aid the review process:
   
   <table>
   <tr><td>
   
   **🎫 Ticket compliance analysis ✅**
   
   
   
   **[9728](https://github.com/apache/incubator-gluten/issues/9728) - Fully compliant**
   
   Compliant requirements:
   
   * Fix inconsistent right join keys when using `JoinAggregateToAggregateUnion`.
   * Ensure `id2` values are `null` when there is no matching record.
   * Add unit tests to verify correct null assignment of right join keys.
   
   
   
   </td></tr>
   <tr><td>⏱️&nbsp;<strong>Estimated effort to review</strong>: 4 🔵🔵🔵🔵⚪</td></tr>
   <tr><td>🧪&nbsp;<strong>PR contains tests</strong></td></tr>
   <tr><td>🔒&nbsp;<strong>No security concerns identified</strong></td></tr>
   <tr><td>⚡&nbsp;<strong>Recommended focus areas for review</strong><br><br>
   
   <details><summary><a href='https://github.com/apache/incubator-gluten/pull/9729/files#diff-e6e40be5445408a6558af17cccae0bab087695234f075b90bc3c91f288a54298R813-R822'><strong>Indexing Logic</strong></a>
   
   The new slicing and indexing of grouping keys, aggregate expressions, and flag expressions in `buildMakeNotMatchedRowsNullProject` is complex and prone to off-by-one errors. Verify that the start and end indices correctly align for all cases.
   </summary>
   
   ```scala
   val keysNumber = analyzedAggregates.head.getGroupingKeys().length
   val keysStartIndex = keysNumber
   val aggregateExpressionsStatIndex = keysStartIndex + analyzedAggregates.length * keysNumber
   val flagExpressionsStartIndex = input.length - analyzedAggregates.length
   
   val dupPrimeKeys = input.slice(0, keysStartIndex)
   val keys = input.slice(keysStartIndex, aggregateExpressionsStatIndex)
   val aggregateExpressions = input.slice(aggregateExpressionsStatIndex, flagExpressionsStartIndex)
   val flagExpressions = input.slice(flagExpressionsStartIndex, input.length)
   
   ```
   
   </details>
   
   <details><summary><a href='https://github.com/apache/incubator-gluten/pull/9729/files#diff-e6e40be5445408a6558af17cccae0bab087695234f075b90bc3c91f288a54298R909-R920'><strong>Performance Impact</strong></a>
   
   The added duplication of join keys (`buildJoinRightKeysProject`) and extra projection steps could degrade query performance. Consider assessing optimization impacts or combining projections.
   </summary>
   
   ```scala
   def buildJoinRightKeysProject(
       plan: LogicalPlan,
       analyzedAggregates: Seq[JoinedAggregateAnalyzer]): LogicalPlan = {
     val input = plan.output
     val keysNum = analyzedAggregates.head.getGroupingKeys.length
     // Make a duplication of the prime aggregate keys, and put them in the front
     val projectList = input.slice(0, keysNum).map {
       case key =>
         RuleExpressionHelper.makeNamedExpression(key, "_dup_prime_" + key.name)
     }
     Project(projectList ++ input, plan)
   }
   ```
   
   </details>
   
   </td></tr>
   </table>
   


-- 
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: commits-unsubscribe@gluten.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org

Re: [PR] [GLUTEN-9728][CH] Fix: Inconsistent result from `JoinAggregateToAggregateUnion` [incubator-gluten]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.

github-actions[bot] commented on PR #9729:
URL: https://github.com/apache/incubator-gluten/pull/9729#issuecomment-2900440026

   Run Gluten Clickhouse CI on x86


-- 
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: commits-unsubscribe@gluten.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org

Re: [PR] [GLUTEN-9728][CH] Fix: Inconsistent result from `JoinAggregateToAggregateUnion` [incubator-gluten]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.

github-actions[bot] commented on PR #9729:
URL: https://github.com/apache/incubator-gluten/pull/9729#issuecomment-2903136493

   Run Gluten Clickhouse CI on x86


-- 
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: commits-unsubscribe@gluten.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org