You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by GitBox <gi...@apache.org> on 2022/07/21 06:25:44 UTC
[GitHub] [doris] jackwener opened a new pull request, #11067: [feature](nereids): add equals for expression
jackwener opened a new pull request, #11067:
URL: https://github.com/apache/doris/pull/11067
# Proposed changes
Issue Number: close #xxx
## Problem Summary:
Describe the overview of changes.
add equals for expression
## Checklist(Required)
1. Does it affect the original behavior: No
2. Has unit tests been added: (No Need)
3. Has document been added or modified: (No Need)
4. Does it need to update dependencies: (No)
5. Are there any changes that cannot be rolled back: (No)
## Further comments
If this is a relatively large or complex change, kick off the discussion at [dev@doris.apache.org](mailto:dev@doris.apache.org) by explaining why you chose the solution you did and what alternatives you considered, etc...
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] englefly commented on a diff in pull request #11067: [feature](nereids): add equals for expression
Posted by GitBox <gi...@apache.org>.
englefly commented on code in PR #11067:
URL: https://github.com/apache/doris/pull/11067#discussion_r926816905
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/Alias.java:
##########
@@ -116,14 +115,13 @@ public String toString() {
return child().toString() + " AS `" + name + "`#" + exprId;
}
- public <R, C> R accept(ExpressionVisitor<R, C> visitor, C context) {
- return visitor.visitAlias(this, context);
- }
-
@Override
public Expression withChildren(List<Expression> children) {
Preconditions.checkArgument(children.size() == 1);
return new Alias(exprId, children.get(0), name);
}
+ public <R, C> R accept(ExpressionVisitor<R, C> visitor, C context) {
Review Comment:
why just move this function's position?
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/types/DataType.java:
##########
@@ -72,4 +74,19 @@ public static DataType convertFromCatalogDataType(Type catalogType) {
public abstract Type toCatalogDataType();
+ @Override
+ public boolean equals(Object o) {
+ if (this == o) {
+ return true;
+ }
+ if (o == null || getClass() != o.getClass()) {
+ return false;
+ }
+ return true;
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hash();
Review Comment:
it is better to return 0 directly. Objects.hash() will return 0.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/Alias.java:
##########
@@ -98,12 +98,11 @@ public boolean equals(Object o) {
if (o == null || getClass() != o.getClass()) {
return false;
}
- if (!super.equals(o)) {
- return false;
- }
- Alias alias = (Alias) o;
- return exprId.equals(alias.exprId) && name.equals(alias.name)
- && qualifier.equals(alias.qualifier) && children.equals(alias.children);
+ Alias that = (Alias) o;
+ return exprId.equals(that.exprId)
Review Comment:
how about remove "exprId.equals(that.exprId)" ?
do you have an example to illustrate its necessity?
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/types/VarcharType.java:
##########
@@ -38,4 +40,18 @@ public static VarcharType createVarcharType(int len) {
public Type toCatalogDataType() {
return ScalarType.createVarcharType(len);
}
+
+ @Override
+ public boolean equals(Object o) {
+ if (!super.equals(o)) {
+ return false;
+ }
+ VarcharType that = (VarcharType) o;
Review Comment:
this may throw java.lang.ClassCastException. It is better to compare their class before convert.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/ComparisonPredicate.java:
##########
@@ -77,7 +77,7 @@ public boolean equals(Object o) {
return false;
}
ComparisonPredicate other = (ComparisonPredicate) o;
- return (type == other.getType()) && Objects.equals(left(), other.left())
+ return Objects.equals(left(), other.left())
Review Comment:
Could you explain why we do not compare type?
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] 924060929 commented on pull request #11067: [feature](nereids): add equals for expression
Posted by GitBox <gi...@apache.org>.
924060929 commented on PR #11067:
URL: https://github.com/apache/doris/pull/11067#issuecomment-1193081342
please and more problem summary
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] jackwener commented on a diff in pull request #11067: [feature](nereids): add equals for expression
Posted by GitBox <gi...@apache.org>.
jackwener commented on code in PR #11067:
URL: https://github.com/apache/doris/pull/11067#discussion_r928078465
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/types/VarcharType.java:
##########
@@ -38,4 +40,18 @@ public static VarcharType createVarcharType(int len) {
public Type toCatalogDataType() {
return ScalarType.createVarcharType(len);
}
+
+ @Override
+ public boolean equals(Object o) {
+ if (!super.equals(o)) {
+ return false;
+ }
+ VarcharType that = (VarcharType) o;
Review Comment:
super.equals() include it.
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] github-actions[bot] commented on pull request #11067: [feature](nereids): add equals for expression
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #11067:
URL: https://github.com/apache/doris/pull/11067#issuecomment-1191568005
PR approved by anyone and no changes requested.
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] 924060929 merged pull request #11067: [feature](nereids): add equals for expression
Posted by GitBox <gi...@apache.org>.
924060929 merged PR #11067:
URL: https://github.com/apache/doris/pull/11067
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] jackwener commented on a diff in pull request #11067: [feature](nereids): add equals for expression
Posted by GitBox <gi...@apache.org>.
jackwener commented on code in PR #11067:
URL: https://github.com/apache/doris/pull/11067#discussion_r927681429
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/ComparisonPredicate.java:
##########
@@ -77,7 +77,7 @@ public boolean equals(Object o) {
return false;
}
ComparisonPredicate other = (ComparisonPredicate) o;
- return (type == other.getType()) && Objects.equals(left(), other.left())
+ return Objects.equals(left(), other.left())
Review Comment:
Because it will be deleted
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] jackwener commented on pull request #11067: [feature](nereids): add equals for expression
Posted by GitBox <gi...@apache.org>.
jackwener commented on PR #11067:
URL: https://github.com/apache/doris/pull/11067#issuecomment-1193087831
> please and more problem summary
@924060929 added it into summary
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] jackwener commented on a diff in pull request #11067: [feature](nereids): add equals for expression
Posted by GitBox <gi...@apache.org>.
jackwener commented on code in PR #11067:
URL: https://github.com/apache/doris/pull/11067#discussion_r927681918
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/Alias.java:
##########
@@ -116,14 +115,13 @@ public String toString() {
return child().toString() + " AS `" + name + "`#" + exprId;
}
- public <R, C> R accept(ExpressionVisitor<R, C> visitor, C context) {
- return visitor.visitAlias(this, context);
- }
-
@Override
public Expression withChildren(List<Expression> children) {
Preconditions.checkArgument(children.size() == 1);
return new Alias(exprId, children.get(0), name);
}
+ public <R, C> R accept(ExpressionVisitor<R, C> visitor, C context) {
Review Comment:
Style: we should put `override function` together
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] github-actions[bot] commented on pull request #11067: [feature](nereids): add equals for expression
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #11067:
URL: https://github.com/apache/doris/pull/11067#issuecomment-1193081066
PR approved by at least one committer and no changes requested.
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org