tanclary commented on code in PR #3495:
URL: https://github.com/apache/calcite/pull/3495#discussion_r1379436990
##########
core/src/main/java/org/apache/calcite/plan/RelOptPredicateList.java:
##########
@@ -245,4 +248,36 @@ public boolean isEffectivelyNotNull(RexNode e) {
}
return false;
}
+
+ /** Return the set of columns that are set to a constant or scalar. */
+ public ImmutableBitSet getInvariantColumnSet() {
+ ImmutableBitSet.Builder builder = ImmutableBitSet.builder();
+ constantMap.keySet()
+ .stream()
+ .filter(RexInputRef.class::isInstance)
+ .map(RexInputRef.class::cast)
+ .map(RexSlot::getIndex)
+ .forEach(builder::set);
+
+ pulledUpPredicates.forEach(rex -> {
+ if (rex.getKind() == SqlKind.EQUALS
+ || rex.getKind() == SqlKind.IS_NOT_DISTINCT_FROM) {
+ List<RexNode> ops = ((RexCall) rex).getOperands();
+ RexNode op0 = ops.get(0);
+ RexNode op1 = ops.get(1);
+ if (op0 instanceof RexInputRef
Review Comment:
I see two very similar conditional blocks here, would a small helper method
help? It would reduce duplicate code and remove one level of nesting. Let me
know what you think.
##########
core/src/test/java/org/apache/calcite/test/RelMetadataTest.java:
##########
@@ -1141,14 +1141,88 @@ private void
checkColumnUniquenessForFilterWithConstantColumns(String sql) {
.assertThatAreColumnsUnique(bitSetOf(0, 1), is(false));
}
+ @Test void testColumnUniquenessForLimit1() {
+ final String sql = ""
+ + "select *\n"
+ + "from emp\n"
+ + "limit 1";
Review Comment:
Could we format this similar to other tests (e.g. remove empty string at
beginning)? I know there are some other tests in this class like this, but from
my experience it is not how most tests are formatted In Calcite
##########
core/src/main/java/org/apache/calcite/rel/metadata/RelMdColumnUniqueness.java:
##########
@@ -364,6 +381,25 @@ public Boolean areColumnsUnique(Intersect rel,
RelMetadataQuery mq,
return false;
}
+ if (Aggregate.isSimple(rel)) {
Review Comment:
5 levels of nesting might be a bit much, I know this is kind of a vague
comment but could you check if there's any opportunities for early returns or
just a way of splitting this up?
--
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]
