[CALCITE-7757] RelMdFunctionalDependency can infer unsound dependencies and cause incorrect query results by darpan-14 · Pull Request #5235 · apache/calcite

darpan-14 · 2026-08-31T19:04:11Z

Jira Link

Changes Proposed

RelMdFunctionalDependency can currently expose unsound or incorrectly indexed dependencies, allowing AggregateRemoveDuplicateKeysRule to remove grouping keys that are still required.

This change:

Corrects equality-derived functional dependencies for INNER, LEFT, and RIGHT joins, respecting the null-generating side of outer joins.
Preserves valid input functional dependencies through outer joins without incorrectly deriving the reverse dependency from the join equality.
Maps aggregate input group ordinals to aggregate output ordinals before publishing functional-dependency metadata.
Avoids equality-derived dependencies for FLOAT, REAL, DOUBLE, and INTERVAL types, including nested occurrences.
Avoids unsafe expression-derived grouping dependencies for those same types.
Adds a reusable SqlTypeUtil predicate traversal for scalar and nested types.
Adds regression coverage for each correctness case.

A basic valid duplicate-key removal remains supported:

SELECT t1.col1, t2.col3, COUNT(*)
FROM t1
JOIN t2 ON t1.col1 = t2.col3
GROUP BY t1.col1, t2.col3;

For this inner join, either equivalent equality key may be removed. The new safeguards prevent that reasoning from being applied where null generation, ordinal mapping, or non-reflexive type semantics make it unsound.

Testing

./gradlew :core:test --tests org.apache.calcite.sql.type.SqlTypeUtilTest --tests org.apache.calcite.test.AggregateRemoveDuplicateKeysRuleTest --tests org.apache.calcite.test.RelMetadataTest.testFunctionalDependency*
./gradlew :core:checkstyleMain :core:checkstyleTest
git diff --check upstream/main...HEAD

The focused test run completed 49 tests with 0 failures.

mihaibudiu · 2026-08-31T21:29:08Z

CI doesn't like your changes

…oins

darpan-14 · 2026-09-01T06:35:24Z

CI is happy now. 😄

mihaibudiu · 2026-09-13T17:55:23Z

  }

+  /**
+   * Copies input dependencies into a join, optionally filtering dependencies


I don't understand this javadoc. Can you please say what each parameter is and what the function actually computes? There's no "join" to be seen of here.

mihaibudiu · 2026-09-13T17:56:03Z

+
+  /**
+   * Returns whether a dependency's determinant contains a non-nullable input
+   * field. Such a determinant cannot collide with the all-NULL determinant of


why does this comment mention a LEFT JOIN? There is no left join here. Please make the comment describe this function does.

mihaibudiu · 2026-09-13T17:56:37Z

-   * @param fdSet Functional dependency set
-   * @param offset Index offset
-   * @return Shifted functional dependency set
+   * Adds functional dependencies implied by a join condition.


where does it add the dependencies?

mihaibudiu · 2026-09-13T17:57:08Z

-   * Extracts functional dependencies from equality and AND conditions.
-   * Handles col1 = col2, col1 IS NOT DISTINCT FROM col2, and AND conditions.
+   * Adds bidirectional dependencies for input-reference equalities in a
+   * condition. Callers are responsible for ensuring that every output row


every output row of what?

darpan-14 · 2026-09-22T05:21:21Z

Updated all Javadocs raised in review. The helper methods now document their general behavior, parameters, return value where applicable, where inferred dependencies are recorded, and the relational-output requirement for equality-derived dependencies.

mihaibudiu · 2026-09-28T18:41:19Z

+  @Test void testKeepsDerivedGroupKeyForNestedDouble() {
+    // Determinism alone is insufficient for approximate values, including
+    // nested occurrences. Keep the derived key unless grouping equality is
+    // known to be congruent with the expression for ARRAY<DOUBLE>.


I cannot read this comment. What is "congruent with the expression"?

mihaibudiu · 2026-10-01T18:01:44Z

When this is ready please re-request a review

sonarqubecloud · 2026-10-02T13:38:09Z

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
90.7% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

darpan-14 · 2026-10-16T05:23:37Z

When this is ready please re-request a review

I guess this is ready for review again @mihaibudiu. All posted comments are addressed.

mihaibudiu

Can you add some quidem tests that produced wrong results without this fix?
These are the easiest to read by humans, especially if you cross-validate them using another database (e.g., postgres).

mihaibudiu · 2026-10-19T22:01:06Z

+   * Returns whether a type or any type nested within its fields, collection
+   * component, map key, or map value matches a predicate.
+   */
+  public static boolean containsType(RelDataType type,


containsMatchingType is a better name

mihaibudiu · 2026-10-19T22:02:18Z

    assertThat(mq.determines(relNode, sumSal, count), is(Boolean.FALSE));
  }

+  @Test void testFunctionalDependencyRollupAggregate() {


Is this a test case for this issue? Then it needs JavaDoc with a specific format. There are lots of examples to follow.

mihaibudiu · 2026-10-19T22:02:35Z

    assertThat(mq.determines(relNode, empNo, dname), is(Boolean.FALSE));
  }

+  @Test void testFunctionalDependencyOuterJoinEqualityDirection() {


Missing JavaDoc on test.

mihaibudiu · 2026-10-19T22:03:31Z

+  }
+
+  @Test void testKeepsFdFromNullGeneratingInputOfLeftJoin() {
+    // This FD comes from the right input rather than from the join condition.


Which FD? Maybe you can explain what FDs are inferred.

mihaibudiu · 2026-10-19T22:04:15Z

  }

+  @Test void testKeepsGroupKeyFromPreservedSideOfLeftJoin() {
+    // The null-generating right key cannot determine the preserved left key:


For a reader it's not obvious at all how this test relates to this comment.
You need longer explanations on these tests.

mihaibudiu · 2026-10-19T22:04:56Z

+
+  @Test void testMapsFdForNonContiguousAggregateGroupSet() {
+    // AggregateProjectMergeRule changes the input group set to {1, 2, 3},
+    // whose keys occupy output positions {0, 1, 2}. Map the input FD 1 -> 2


Is this comment valid before or after the fix? If it's before the fix, the comment is already wrong.

mihaibudiu · 2026-10-19T22:05:05Z

        .check();
  }

+  @Test void testKeepsGroupKeyFromPreservedSideOfLeftJoin() {


no JavaDoc on tests.

darpan-e6 · 2026-10-26T05:43:20Z

Addressed all test related comments. I tested new QuidemTest results on Postgres 18.6. If you want me to verify on any other db engines, please let me know, I can do that as well.

mihaibudiu · 2026-10-26T18:23:18Z

@xiedeyantu you wrote most of this code, can you please check the core changes to RelMdFunctionalDependency?

darpan-14 added 6 commits September 1, 2026 11:15

[CALCITE-7757] Functional dependency inference is unsound for outer j…

75adeec

…oins

[CALCITE-7757] Map aggregate functional dependencies to output ordinals

84a8e60

[CALCITE-7757] Avoid unsafe equality-derived functional dependencies

f3aa45c

[CALCITE-7757] Avoid unsafe derived grouping key dependencies

d905926

[CALCITE-7757] Add utility for matching nested types

3a73679

[CALCITE-7757] Apply Java formatting

60bf738

darpan-14 force-pushed the CALCITE-7757 branch from 32526d0 to 60bf738 Compare September 1, 2026 06:04

mihaibudiu reviewed Sep 2, 2026

View reviewed changes

Comment thread core/src/main/java/org/apache/calcite/rel/metadata/RelMdFunctionalDependency.java

[CALCITE-7757] Allow interval grouping key inference

0250d6b

mihaibudiu requested changes Sep 13, 2026

View reviewed changes

[CALCITE-7757] Improve functional dependency Javadocs

cbe536e

mihaibudiu reviewed Sep 28, 2026

View reviewed changes

[CALCITE-7757] Avoid aggregate FDs for grouping sets

1348183

darpan-14 force-pushed the CALCITE-7757 branch from 503afd4 to 1348183 Compare October 1, 2026 13:47

darpan-14 requested a review from mihaibudiu October 16, 2026 05:22

mihaibudiu reviewed Oct 19, 2026

View reviewed changes

[CALCITE-7757] Improve functional dependency documentation and tests

f4c33fc

iwanttobepowerful requested a review from xiedeyantu October 26, 2026 05:45

mihaibudiu requested review from xiedeyantu and removed request for xiedeyantu October 26, 2026 18:22

Conversation

darpan-14 commented Aug 31, 2026

Jira Link

Changes Proposed

Testing

Uh oh!

mihaibudiu commented Aug 31, 2026

Uh oh!

darpan-14 commented Sep 1, 2026

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

darpan-14 commented Sep 22, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mihaibudiu commented Oct 1, 2026

Uh oh!

sonarqubecloud Bot commented Oct 2, 2026

Quality Gate passed

Uh oh!

darpan-14 commented Oct 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mihaibudiu left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

darpan-e6 commented Oct 26, 2026

Uh oh!

mihaibudiu commented Oct 26, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

darpan-14 commented Oct 16, 2026 •

edited

Loading