You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by "xiedeyantu (via GitHub)" <gi...@apache.org> on 2026/04/18 16:08:24 UTC

[PR] [CALCITE-0000] Remove redundant aggregate group keys with FD [calcite]

xiedeyantu opened a new pull request, #4892:
URL: https://github.com/apache/calcite/pull/4892

   <!--
   Thanks for sending a pull request! Here are some critical tips for you:
   
   1. READ THE GUIDE FIRST: https://calcite.apache.org/develop/#contributing
      *For significant contributions, please discuss on the dev mailing list or Jira BEFORE coding.*
   
   2. JIRA IS USUALLY MANDATORY: Ensure you have created an issue on the Calcite Jira:
      https://issues.apache.org/jira/projects/CALCITE/issues
      *Check existing issues first to avoid duplicates.*
      *Note: A Jira is NOT required for typos and cosmetic changes (i.e., changes that are neither bugs nor features).*
   
   3. TIMING: Strongly recommended to create the Jira BEFORE you start writing code (e.g., a day or so before posting a PR).
      This gives others a chance to weigh in on your specification.
   
   4. CRITICAL CONSISTENCY RULE
      The following three items MUST match exactly in wording and meaning:
      (A) The Jira Issue Title
      (B) This Pull Request Title
      (C) Your Git Commit Message
   
      Format: [CALCITE-XXXX] <Description>
      Example: [CALCITE-0000] Add IF NOT EXISTS clause to CREATE TABLE
   
      Guidelines for a good Title:
      - Illustrate using SQL keywords (in ALL-CAPS) rather than Java method names.
      - Focus on the specification and user experience, not the implementation.
      - Make it clear whether it is a bug or a feature.
      - Use words like "should" to indicate desired behavior vs. current behavior (e.g., "Validator should not close model file").
   
   5. REPRODUCTION: If fixing a bug, please provide a concise SQL example or test case to reproduce the issue for a faster review.
   
   6. TESTING: Ensure `./gradlew build` passes and appropriate tests are added/updated.
   -->
   
   ## Jira Link
   
   [CALCITE-XXXX](https://issues.apache.org/jira/browse/CALCITE-XXXX)
   
   ## Changes Proposed
   <!--
   Please clarify what changes you are proposing. The purpose of this section is to outline the changes and how this PR fixes the issue.
   If possible, please consider writing useful notes for better and faster reviews in your PR.
   -->
   


-- 
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@calcite.apache.org

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


Re: [PR] [CALCITE-7479] Remove redundant aggregate group keys with FD [calcite]

Posted by "xiedeyantu (via GitHub)" <gi...@apache.org>.
xiedeyantu commented on PR #4892:
URL: https://github.com/apache/calcite/pull/4892#issuecomment-4284780314

   I'm concerned about whether this rewrite is meaningful, as it requires the introduction of 
   "single_value".


-- 
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@calcite.apache.org

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


Re: [PR] [CALCITE-7479] Remove redundant aggregate group keys with FD [calcite]

Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on code in PR #4892:
URL: https://github.com/apache/calcite/pull/4892#discussion_r3127364621


##########
core/src/main/java/org/apache/calcite/rel/rules/AggregateRemoveDuplicateKeysRule.java:
##########
@@ -0,0 +1,192 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.rel.rules;
+
+import org.apache.calcite.plan.RelOptRuleCall;
+import org.apache.calcite.plan.RelRule;
+import org.apache.calcite.rel.core.Aggregate;
+import org.apache.calcite.rel.logical.LogicalAggregate;
+import org.apache.calcite.rel.metadata.RelMetadataQuery;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.sql.fun.SqlStdOperatorTable;
+import org.apache.calcite.tools.RelBuilder;
+import org.apache.calcite.util.ImmutableBitSet;
+
+import org.immutables.value.Value;
+
+import java.util.ArrayList;
+import java.util.List;
+
+/**
+ * Planner rule that removes redundant grouping keys from an
+ * {@link Aggregate} when the later keys are functionally determined by the
+ * earlier retained keys.
+ *
+ * <p>The original output schema is preserved by adding {@code ANY_VALUE}
+ * aggregate calls for removed grouping keys and then projecting the row back
+ * into the original field order.
+ *
+ * <p>{@code ANY_VALUE} is chosen over {@code SINGLE_VALUE} because functional
+ * dependence only guarantees that a removed key maps to a single <em>value</em>
+ * per group, not that the group contains a single <em>row</em>.
+ * {@code SINGLE_VALUE} enforces the stricter, row-count constraint at runtime

Review Comment:
   I think this explanation is not necessary. If I was confused, it doesn't mean other people will as well. In fact, SINGLE_VALUE is documented properly.



-- 
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@calcite.apache.org

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


Re: [PR] [CALCITE-7479] Remove redundant aggregate group keys with FD [calcite]

Posted by "xiedeyantu (via GitHub)" <gi...@apache.org>.
xiedeyantu commented on PR #4892:
URL: https://github.com/apache/calcite/pull/4892#issuecomment-4300452541

   @mihaibudiu If you have time, could you please take another look at this PR?


-- 
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@calcite.apache.org

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


Re: [PR] [CALCITE-7479] Remove redundant aggregate group keys with FD [calcite]

Posted by "xiedeyantu (via GitHub)" <gi...@apache.org>.
xiedeyantu commented on code in PR #4892:
URL: https://github.com/apache/calcite/pull/4892#discussion_r3127426050


##########
core/src/main/java/org/apache/calcite/rel/rules/AggregateRemoveDuplicateKeysRule.java:
##########
@@ -0,0 +1,192 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.rel.rules;
+
+import org.apache.calcite.plan.RelOptRuleCall;
+import org.apache.calcite.plan.RelRule;
+import org.apache.calcite.rel.core.Aggregate;
+import org.apache.calcite.rel.logical.LogicalAggregate;
+import org.apache.calcite.rel.metadata.RelMetadataQuery;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.sql.fun.SqlStdOperatorTable;
+import org.apache.calcite.tools.RelBuilder;
+import org.apache.calcite.util.ImmutableBitSet;
+
+import org.immutables.value.Value;
+
+import java.util.ArrayList;
+import java.util.List;
+
+/**
+ * Planner rule that removes redundant grouping keys from an
+ * {@link Aggregate} when the later keys are functionally determined by the
+ * earlier retained keys.
+ *
+ * <p>The original output schema is preserved by adding {@code ANY_VALUE}
+ * aggregate calls for removed grouping keys and then projecting the row back
+ * into the original field order.
+ *
+ * <p>{@code ANY_VALUE} is chosen over {@code SINGLE_VALUE} because functional
+ * dependence only guarantees that a removed key maps to a single <em>value</em>
+ * per group, not that the group contains a single <em>row</em>.
+ * {@code SINGLE_VALUE} enforces the stricter, row-count constraint at runtime

Review Comment:
   What do you think about me attaching the examples and explanations from that PR to the Jira ticket, and then removing this section of explanation from the rules?



-- 
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@calcite.apache.org

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


Re: [PR] [CALCITE-7479] Remove redundant aggregate group keys with FD [calcite]

Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #4892:
URL: https://github.com/apache/calcite/pull/4892#issuecomment-4275039110

   ## [![Quality Gate Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png 'Quality Gate Passed')](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4892) **Quality Gate passed**  
   Issues  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [1 New issue](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4892&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/accepted-16px.png '') [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4892&issueStatuses=ACCEPTED)
   
   Measures  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=4892&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [93.3% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4892&metric=new_coverage&view=list)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4892&metric=new_duplicated_lines_density&view=list)  
     
   [See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4892)
   
   


-- 
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@calcite.apache.org

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


Re: [PR] [CALCITE-7479] Remove redundant aggregate group keys with FD [calcite]

Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #4892:
URL: https://github.com/apache/calcite/pull/4892#issuecomment-4301035182

   ## [![Quality Gate Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png 'Quality Gate Passed')](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4892) **Quality Gate passed**  
   Issues  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [1 New issue](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4892&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/accepted-16px.png '') [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4892&issueStatuses=ACCEPTED)
   
   Measures  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=4892&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [93.3% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4892&metric=new_coverage&view=list)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4892&metric=new_duplicated_lines_density&view=list)  
     
   [See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4892)
   
   


-- 
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@calcite.apache.org

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


Re: [PR] [CALCITE-7479] Remove redundant aggregate group keys with FD [calcite]

Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on PR #4892:
URL: https://github.com/apache/calcite/pull/4892#issuecomment-4284817774

   Doesn't even have to be a branch, you can run the experiment and report the results.


-- 
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@calcite.apache.org

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


Re: [PR] [CALCITE-7479] Remove redundant aggregate group keys with FD [calcite]

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


-- 
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@calcite.apache.org

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


Re: [PR] [CALCITE-7479] Remove redundant aggregate group keys with FD [calcite]

Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on PR #4892:
URL: https://github.com/apache/calcite/pull/4892#issuecomment-4284796544

   Can you make it default and see what happens?


-- 
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@calcite.apache.org

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


Re: [PR] [CALCITE-7479] Remove redundant aggregate group keys with FD [calcite]

Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on PR #4892:
URL: https://github.com/apache/calcite/pull/4892#issuecomment-4284819538

   There are lots of uses of SINGLE_VALUE, I think that one is fine.
   I am more worried about whether the functional dependency is sufficiently conservative.


-- 
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@calcite.apache.org

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


Re: [PR] [CALCITE-7479] Remove redundant aggregate group keys with FD [calcite]

Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #4892:
URL: https://github.com/apache/calcite/pull/4892#issuecomment-4275014093

   ## [![Quality Gate Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png 'Quality Gate Passed')](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4892) **Quality Gate passed**  
   Issues  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [1 New issue](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4892&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/accepted-16px.png '') [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4892&issueStatuses=ACCEPTED)
   
   Measures  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=4892&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [93.3% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4892&metric=new_coverage&view=list)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4892&metric=new_duplicated_lines_density&view=list)  
     
   [See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4892)
   
   


-- 
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@calcite.apache.org

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


Re: [PR] [CALCITE-7479] Remove redundant aggregate group keys with FD [calcite]

Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on PR #4892:
URL: https://github.com/apache/calcite/pull/4892#issuecomment-4284776961

   No other tests were modified


-- 
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@calcite.apache.org

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


Re: [PR] [CALCITE-7479] Remove redundant aggregate group keys with FD [calcite]

Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on PR #4892:
URL: https://github.com/apache/calcite/pull/4892#issuecomment-4300461688

   I have approved it already. Anything in particular you'd like me to check?


-- 
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@calcite.apache.org

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


Re: [PR] [CALCITE-7479] Remove redundant aggregate group keys with FD [calcite]

Posted by "xiedeyantu (via GitHub)" <gi...@apache.org>.
xiedeyantu commented on PR #4892:
URL: https://github.com/apache/calcite/pull/4892#issuecomment-4300510954

   Since there were some changes in the latest commit, I wanted to see if you need to confirm it again, even though it followed the solution discussed in the other PR.


-- 
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@calcite.apache.org

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


Re: [PR] [CALCITE-0000] Remove redundant aggregate group keys with FD [calcite]

Posted by "xiedeyantu (via GitHub)" <gi...@apache.org>.
xiedeyantu commented on PR #4892:
URL: https://github.com/apache/calcite/pull/4892#issuecomment-4274072753

   Just draft.


-- 
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@calcite.apache.org

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


Re: [PR] [CALCITE-7479] Remove redundant aggregate group keys with FD [calcite]

Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on code in PR #4892:
URL: https://github.com/apache/calcite/pull/4892#discussion_r3127430122


##########
core/src/main/java/org/apache/calcite/rel/rules/AggregateRemoveDuplicateKeysRule.java:
##########
@@ -0,0 +1,192 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.rel.rules;
+
+import org.apache.calcite.plan.RelOptRuleCall;
+import org.apache.calcite.plan.RelRule;
+import org.apache.calcite.rel.core.Aggregate;
+import org.apache.calcite.rel.logical.LogicalAggregate;
+import org.apache.calcite.rel.metadata.RelMetadataQuery;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.sql.fun.SqlStdOperatorTable;
+import org.apache.calcite.tools.RelBuilder;
+import org.apache.calcite.util.ImmutableBitSet;
+
+import org.immutables.value.Value;
+
+import java.util.ArrayList;
+import java.util.List;
+
+/**
+ * Planner rule that removes redundant grouping keys from an
+ * {@link Aggregate} when the later keys are functionally determined by the
+ * earlier retained keys.
+ *
+ * <p>The original output schema is preserved by adding {@code ANY_VALUE}
+ * aggregate calls for removed grouping keys and then projecting the row back
+ * into the original field order.
+ *
+ * <p>{@code ANY_VALUE} is chosen over {@code SINGLE_VALUE} because functional
+ * dependence only guarantees that a removed key maps to a single <em>value</em>
+ * per group, not that the group contains a single <em>row</em>.
+ * {@code SINGLE_VALUE} enforces the stricter, row-count constraint at runtime

Review Comment:
   Yes, that's fine.



-- 
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@calcite.apache.org

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


Re: [PR] [CALCITE-7479] Remove redundant aggregate group keys with FD [calcite]

Posted by "xiedeyantu (via GitHub)" <gi...@apache.org>.
xiedeyantu commented on PR #4892:
URL: https://github.com/apache/calcite/pull/4892#issuecomment-4284770586

   > This looks fine. How come it is never applicable? Are all our tests using optimal group by keys? Or we just don't have many tables with keys?
   
   "How come it is never applicable?" I didn't quite understand the meaning of this sentence. Because the functional dependencies we currently support are limited, I used the primary key as an example.


-- 
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@calcite.apache.org

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


Re: [PR] [CALCITE-7479] Remove redundant aggregate group keys with FD [calcite]

Posted by "xiedeyantu (via GitHub)" <gi...@apache.org>.
xiedeyantu commented on PR #4892:
URL: https://github.com/apache/calcite/pull/4892#issuecomment-4284790567

   > No other tests were modified
   
   Because this new rule is not set as a "default" rule, it doesn't have any other impact.


-- 
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@calcite.apache.org

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


Re: [PR] [CALCITE-7479] Remove redundant aggregate group keys with FD [calcite]

Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on PR #4892:
URL: https://github.com/apache/calcite/pull/4892#issuecomment-4284787100

   I think there are other uses of `SINGLE_VALUE` in the compiler.


-- 
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@calcite.apache.org

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


Re: [PR] [CALCITE-7479] Remove redundant aggregate group keys with FD [calcite]

Posted by "xiedeyantu (via GitHub)" <gi...@apache.org>.
xiedeyantu commented on PR #4892:
URL: https://github.com/apache/calcite/pull/4892#issuecomment-4284798109

   > I think there are other uses of `SINGLE_VALUE` in the compiler.
   
   Are you suggesting that I don't need to worry too much about "single_value"? Can the compiler do more?


-- 
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@calcite.apache.org

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


Re: [PR] [CALCITE-7479] Remove redundant aggregate group keys with FD [calcite]

Posted by "xiedeyantu (via GitHub)" <gi...@apache.org>.
xiedeyantu commented on PR #4892:
URL: https://github.com/apache/calcite/pull/4892#issuecomment-4284805570

   > Can you make it default and see what happens?
   
   Of course, I can create a new branch to add it as a default rule and observe the outcome later.


-- 
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@calcite.apache.org

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


Re: [PR] [CALCITE-7479] Remove redundant aggregate group keys with FD [calcite]

Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #4892:
URL: https://github.com/apache/calcite/pull/4892#issuecomment-4296575486

   ## [![Quality Gate Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png 'Quality Gate Passed')](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4892) **Quality Gate passed**  
   Issues  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [1 New issue](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4892&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/accepted-16px.png '') [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4892&issueStatuses=ACCEPTED)
   
   Measures  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=4892&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [93.3% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4892&metric=new_coverage&view=list)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4892&metric=new_duplicated_lines_density&view=list)  
     
   [See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4892)
   
   


-- 
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@calcite.apache.org

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