github-actions[bot] commented on code in PR #64862:
URL: https://github.com/apache/doris/pull/64862#discussion_r3479074128
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalOlapScan.java:
##########
@@ -990,6 +995,38 @@ && getTable().isMorTable()
}
builder.addUniqueSlot(uniqSlots.build());
}
+ // Add unique slots from primary key constraints
Review Comment:
This call is duplicating work that already happened at the top of the
method: `super.computeUnique(builder)` goes through
`LogicalCatalogRelation.computeUnique`, reads the same
`ConstraintManager.getPrimaryKeyConstraints(...)`, resolves each
`PrimaryKeyConstraint` to table `Column`s, and adds the matching slots as
unique. The new unit tests also do not isolate this helper, because their
mocked PK constraints and matching `originalColumn`s are consumed by that
superclass path before `addUniqueFromPk` runs, so they would still pass if this
helper were removed. Please remove the duplicate helper or replace it with a
case that the superclass path demonstrably cannot cover.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalOlapScan.java:
##########
@@ -990,6 +995,38 @@ && getTable().isMorTable()
}
builder.addUniqueSlot(uniqSlots.build());
}
+ // Add unique slots from primary key constraints
+ addUniqueFromPk(builder);
+ }
+
+ private void addUniqueFromPk(DataTrait.Builder builder) {
+ TableIf table = getTable();
+ if (!(table instanceof Table)) {
+ return;
+ }
Review Comment:
`t` is unused after the lookup was changed to
`TableNameInfoUtils.fromTableOrNull(table)`, so FE checkstyle will reject this
changed line because `UnusedLocalVariable` is enabled as an error in
`fe/check/checkstyle/checkstyle.xml`. Please remove this cast/local before
merging.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
