mihaibudiu commented on code in PR #4089:
URL: https://github.com/apache/calcite/pull/4089#discussion_r1882412593
##########
core/src/main/java/org/apache/calcite/rel/metadata/RelMdUniqueKeys.java:
##########
@@ -131,7 +159,7 @@ public Set<ImmutableBitSet> getUniqueKeys(Project rel,
RelMetadataQuery mq,
Util.transform(program.getProjectList(), program::expandLocalRef));
}
- private static Set<ImmutableBitSet> getProjectUniqueKeys(SingleRel rel,
RelMetadataQuery mq,
+ private Set<ImmutableBitSet> getProjectUniqueKeys(SingleRel rel,
RelMetadataQuery mq,
Review Comment:
I wonder whether there is any benefit in knowing whether the function has
hit the limit during a run.
If it hasn't you know the set is complete.
##########
core/src/test/java/org/apache/calcite/test/RelMetadataTest.java:
##########
@@ -1633,6 +1647,242 @@ private void
checkColumnUniquenessForFilterWithConstantColumns(String sql) {
.assertThatUniqueKeysAre(bitSetOf(0, 1));
}
+ @Test void testUniqueKeysWithLimitOnSortOneRow() {
+ sql("select ename, empno from emp order by ename limit 1")
+ .withCatalogReaderFactory(COMPOSITE_FACTORY)
+ .assertThatRel(is(instanceOf(Sort.class)))
+ .withMetadataConfig(uniqueKeyConfig(0))
+ .assertThatUniqueKeysAre()
+ .withMetadataConfig(uniqueKeyConfig(2))
+ .assertThatUniqueKeysAre(bitSetOf());
+ }
+
+ @Test void testUniqueKeysWithLimitOnFilter() {
+ sql("select * from s.passenger t1 where t1.age > 35")
+ .withCatalogReaderFactory(COMPOSITE_FACTORY)
+ .withRelTransform(project -> project.getInput(0))
+ .assertThatRel(is(instanceOf(Filter.class)))
+ .withMetadataConfig(uniqueKeyConfig(2))
+ .assertThatUniqueKeysAre(bitSetOf(0), bitSetOf(1));
+ }
+
+ @Test void
testUniqueKeysWithLimitOnProjectOverInputWithCompositeKeyAndRepeatedColumns() {
+ String cols = IntStream.range(0, 32).mapToObj(i -> "k" +
i).collect(Collectors.joining(","));
+ sql("select " + cols + ", " + cols + " from s.composite_keys_32_table")
+ .withCatalogReaderFactory(COMPOSITE_FACTORY)
+ .assertThatRel(is(instanceOf(Project.class)))
+ .withMetadataConfig(uniqueKeyConfig(2))
+ .assertThatUniqueKeysAre(ImmutableBitSet.range(0, 32),
+ ImmutableBitSet.range(0, 31).set(63));
Review Comment:
not an easy way for a reviewer to check that these results are correct, so I
have to trust you on this.
I don't know if it's worth having a utility that converts these sets into
readable lists of column names.
##########
core/src/main/java/org/apache/calcite/rel/metadata/RelMdUniqueKeys.java:
##########
@@ -358,7 +387,7 @@ public Set<ImmutableBitSet> getUniqueKeys(Aggregate rel,
RelMetadataQuery mq,
* other keys. Given {@code {0},{1},{1,2}}, returns {@code {0},{1}}.
*/
private static Set<ImmutableBitSet> filterSupersets(
- Set<ImmutableBitSet> uniqueKeys) {
+ Set<ImmutableBitSet> uniqueKeys, int limit) {
Review Comment:
I thought that uniqueKeys is always smaller than the limit here.
##########
core/src/main/java/org/apache/calcite/rel/metadata/RelMdUniqueKeys.java:
##########
@@ -184,10 +213,14 @@ private static Set<ImmutableBitSet>
getProjectUniqueKeys(SingleRel rel, RelMetad
// the resulting unique keys would be {{0},{4}}, {{1},{4}}
Iterable<List<Integer>> product = Linq4j.product(Util.transform(colMask,
mapInToOutPos::get));
-
- resultBuilder.addAll(Util.transform(product, ImmutableBitSet::of));
+ for (List<Integer> passKey : product) {
Review Comment:
are these loops deterministic (i.e., they do not depend on the memory layout
of a hashtable)?
I find it very desirable to have deterministic behavior in a compiler; makes
debugging much easier too.
##########
core/src/test/java/org/apache/calcite/test/RelMetadataTest.java:
##########
@@ -1633,6 +1647,242 @@ private void
checkColumnUniquenessForFilterWithConstantColumns(String sql) {
.assertThatUniqueKeysAre(bitSetOf(0, 1));
}
+ @Test void testUniqueKeysWithLimitOnSortOneRow() {
+ sql("select ename, empno from emp order by ename limit 1")
+ .withCatalogReaderFactory(COMPOSITE_FACTORY)
+ .assertThatRel(is(instanceOf(Sort.class)))
+ .withMetadataConfig(uniqueKeyConfig(0))
+ .assertThatUniqueKeysAre()
+ .withMetadataConfig(uniqueKeyConfig(2))
+ .assertThatUniqueKeysAre(bitSetOf());
+ }
+
+ @Test void testUniqueKeysWithLimitOnFilter() {
+ sql("select * from s.passenger t1 where t1.age > 35")
+ .withCatalogReaderFactory(COMPOSITE_FACTORY)
+ .withRelTransform(project -> project.getInput(0))
+ .assertThatRel(is(instanceOf(Filter.class)))
+ .withMetadataConfig(uniqueKeyConfig(2))
+ .assertThatUniqueKeysAre(bitSetOf(0), bitSetOf(1));
+ }
+
+ @Test void
testUniqueKeysWithLimitOnProjectOverInputWithCompositeKeyAndRepeatedColumns() {
+ String cols = IntStream.range(0, 32).mapToObj(i -> "k" +
i).collect(Collectors.joining(","));
+ sql("select " + cols + ", " + cols + " from s.composite_keys_32_table")
+ .withCatalogReaderFactory(COMPOSITE_FACTORY)
+ .assertThatRel(is(instanceOf(Project.class)))
+ .withMetadataConfig(uniqueKeyConfig(2))
+ .assertThatUniqueKeysAre(ImmutableBitSet.range(0, 32),
+ ImmutableBitSet.range(0, 31).set(63));
Review Comment:
I think these tests will only work if the generated keys are deterministic
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
