LantaoJin commented on code in PR #5348:
URL: https://github.com/apache/calcite/pull/5348#discussion_r4332541337
##########
core/src/main/java/org/apache/calcite/rel/RelCollationImpl.java:
##########
@@ -56,7 +71,9 @@ public class RelCollationImpl implements RelCollation {
protected RelCollationImpl(ImmutableList<RelFieldCollation> fieldCollations)
{
this.fieldCollations = fieldCollations;
- checkArgument(Util.isDistinct(RelCollations.ordinals(fieldCollations)),
+ checkArgument(
Review Comment:
Yes, it is needed for the fix. With this PR, the static initializer of
`RelCollationImpl` creates instances (EMPTY and PRESERVE), so this constructor
now runs during `RelCollationImpl.<clinit>`. `RelCollations.ordinals` is a
static method of `RelCollations`, so calling it here would start the
initialization of `RelCollations` from inside `RelCollationImpl.<clinit>`, and
the cycle would be back: a thread in `RelCollationImpl.<clinit>` waits for
`RelCollations`, while another thread in `RelCollations.<clinit>` (at `new
RelCollationImpl(...)`) waits for `RelCollationImpl`.
`Util.transform(fieldCollations, RelFieldCollation::getFieldIndex)` is the body
of `RelCollations.ordinals`, so the check is unchanged.
I added a short comment at this call site.
--
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]
