clintropolis commented on code in PR #18334:
URL: https://github.com/apache/druid/pull/18334#discussion_r2246456198
##########
processing/src/main/java/org/apache/druid/segment/virtual/ExpressionVirtualColumn.java:
##########
@@ -449,20 +456,21 @@ public boolean equals(Object o)
return false;
}
Expression that = (Expression) o;
- return Objects.equals(expressionString, that.expressionString) &&
Objects.equals(outputType, that.outputType);
+ return Objects.equals(parsed.get().stringify(),
that.parsed.get().stringify())
Review Comment:
its probably ok... for whatever reason I am hesitant to trust equals even
tho `Expr` is marked with `SubclassesMustOverrideEqualsAndHashCode` it has a
lot less test coverage than stringify which has a lot of coverage to ensure
that expr.stringify() makes a string that can be parsed back into the same
expr. I guess the solution to my feelings is to just add more coverage using
equals and hashcode instead of not using them.
Maybe I'll try to add some coverage and switch to equals.
--
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]
