Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CALCITE-6727] Column uniqueness constrain should only apply to inner… #4088

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -321,15 +321,16 @@ public Boolean areColumnsUnique(Intersect rel, RelMetadataQuery mq,
}

final JoinInfo joinInfo = rel.analyzeCondition();

// Joining with a singleton constrains the keys on the other table
final Double rightMaxRowCount = mq.getMaxRowCount(right);
if (rightMaxRowCount != null && rightMaxRowCount <= 1.0) {
leftColumns = leftColumns.union(joinInfo.leftSet());
}
final Double leftMaxRowCount = mq.getMaxRowCount(left);
if (leftMaxRowCount != null && leftMaxRowCount <= 1.0) {
rightColumns = rightColumns.union(joinInfo.rightSet());
if (rel.getJoinType() == JoinRelType.INNER) {
// Joining with a singleton constrains the keys on the other table
final Double rightMaxRowCount = mq.getMaxRowCount(right);
if (rightMaxRowCount != null && rightMaxRowCount <= 1.0) {
leftColumns = leftColumns.union(joinInfo.leftSet());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this correct even if maxRowCount is 0?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems to me that the constrain still works even when the build side is empty

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mihaibudiu Any more thoughts on this? Please take another look when you have a moment

}
final Double leftMaxRowCount = mq.getMaxRowCount(left);
if (leftMaxRowCount != null && leftMaxRowCount <= 1.0) {
rightColumns = rightColumns.union(joinInfo.rightSet());
}
}

// If the original column mask contains columns from both the left and
Expand Down
21 changes: 21 additions & 0 deletions core/src/test/java/org/apache/calcite/test/RelMetadataTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -1193,6 +1193,27 @@ private void checkColumnUniquenessForFilterWithConstantColumns(String sql) {
.assertThatUniqueKeysAre(bitSetOf());
}

/** Test case for
* <a href="https://issues.apache.org/jira/browse/CALCITE-6727">[CALCITE-6727]
* Column uniqueness constrain should only apply to inner join</a>. */
@Test void testColumnUniquenessForLeftJoinOnLimit1() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is customary to add a JavaDoc comment to the test indicating the JIRA issue that is being addressed. This makes it easier for maintainers to understand the rationale for a test. You can find lots of examples in the code about how that is supposed to be structured, please follow the pattern.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added the comment. Thanks for your reminder

final String sql = ""
+ "select A.empno as a_empno,\n"
+ " A.ename as a_ename,\n"
+ " B.empno as b_empno,\n"
+ " B.ename as b_ename\n"
+ "from emp A\n"
+ "left join (\n"
+ " select * from emp\n"
+ " limit 1) B\n"
+ "on A.empno = B.empno";
sql(sql)
.assertThatAreColumnsUnique(bitSetOf(0), is(true))
.assertThatAreColumnsUnique(bitSetOf(1), is(false))
.assertThatAreColumnsUnique(bitSetOf(2), is(false))
.assertThatAreColumnsUnique(bitSetOf(3), is(false));
}

@Test void testColumnUniquenessForJoinOnAggregation() {
final String sql = ""
+ "select *\n"
Expand Down
Loading