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:23:50 UTC

[GitHub] [doris] wangshuo128 opened a new pull request, #11066: [Refactor](Nereids) Remove expression type.

wangshuo128 opened a new pull request, #11066:
URL: https://github.com/apache/doris/pull/11066

   # Proposed changes
   
   Issue Number: close #xxx
   
   `ExpressionType` is duplicated with Java class type info, so removed it.
   
   ## Problem Summary:
   
   Describe the overview of changes.
   
   ## Checklist(Required)
   
   1. Does it affect the original behavior: (Yes/No/I Don't know)
   2. Has unit tests been added: (Yes/No/No Need)
   3. Has document been added or modified: (Yes/No/No Need)
   4. Does it need to update dependencies: (Yes/No)
   5. Are there any changes that cannot be rolled back: (Yes/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 #11066: [Refactor](Nereids) Remove expression type.

Posted by GitBox <gi...@apache.org>.
englefly commented on code in PR #11066:
URL: https://github.com/apache/doris/pull/11066#discussion_r926844401


##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/Arithmetic.java:
##########
@@ -189,6 +165,24 @@ public int hashCode() {
 
     @Override
     public String toString() {
-        return toSql();
+        return stringBuilder(Object::toString);

Review Comment:
   why not throws UnboundedException now?
   it will return string like "java.lang.Object@d716361". it is useful?



-- 
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] wangshuo128 commented on a diff in pull request #11066: [Refactor](Nereids) Remove expression type.

Posted by GitBox <gi...@apache.org>.
wangshuo128 commented on code in PR #11066:
URL: https://github.com/apache/doris/pull/11066#discussion_r927222429


##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/Arithmetic.java:
##########
@@ -189,6 +165,24 @@ public int hashCode() {
 
     @Override
     public String toString() {
-        return toSql();
+        return stringBuilder(Object::toString);

Review Comment:
   The override implementation method in derived classes would be used.



-- 
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 #11066: [Refactor](Nereids) Remove expression type.

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #11066:
URL: https://github.com/apache/doris/pull/11066#issuecomment-1191240484

   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] jackwener commented on a diff in pull request #11066: [Refactor](Nereids) Remove expression type.

Posted by GitBox <gi...@apache.org>.
jackwener commented on code in PR #11066:
URL: https://github.com/apache/doris/pull/11066#discussion_r926307579


##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/Regexp.java:
##########
@@ -30,19 +30,14 @@
 public class Regexp extends StringRegexPredicate {
 
     public Regexp(Expression left, Expression right) {
-        super(ExpressionType.REGEXP, left, right);
+        super(left, right, "regexp");
     }
 
     @Override
     public boolean nullable() throws UnboundException {
         return left().nullable();
     }
 
-    @Override

Review Comment:
   Why delete many `toString`



-- 
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 a diff in pull request #11066: [Refactor](Nereids) Remove expression type.

Posted by GitBox <gi...@apache.org>.
924060929 commented on code in PR #11066:
URL: https://github.com/apache/doris/pull/11066#discussion_r926449371


##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/Arithmetic.java:
##########
@@ -189,6 +165,24 @@ public int hashCode() {
 
     @Override
     public String toString() {
-        return toSql();
+        return stringBuilder(Object::toString);

Review Comment:
   good refactor



-- 
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] morrySnow commented on a diff in pull request #11066: [Refactor](Nereids) Remove expression type.

Posted by GitBox <gi...@apache.org>.
morrySnow commented on code in PR #11066:
URL: https://github.com/apache/doris/pull/11066#discussion_r926439060


##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/StringRegexPredicate.java:
##########
@@ -29,15 +29,21 @@
  * Such as: like, regexp
  */
 public abstract class StringRegexPredicate extends Expression implements BinaryExpression {
+
+    /**
+     * like or regexp
+     */
+    protected final String symbol;
+
     /**
      * Constructor of StringRegexPredicate.
      *
-     * @param nodeType node type of expression
      * @param left     left child of string regex
      * @param right    right child of string regex
      */
-    public StringRegexPredicate(ExpressionType nodeType, Expression left, Expression right) {
-        super(nodeType, left, right);
+    public StringRegexPredicate(Expression left, Expression right, String symbol) {

Review Comment:
   nit: mod java doc parameter list



-- 
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 #11066: [Refactor](Nereids) Remove expression type.

Posted by GitBox <gi...@apache.org>.
englefly commented on code in PR #11066:
URL: https://github.com/apache/doris/pull/11066#discussion_r926844401


##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/Arithmetic.java:
##########
@@ -189,6 +165,24 @@ public int hashCode() {
 
     @Override
     public String toString() {
-        return toSql();
+        return stringBuilder(Object::toString);

Review Comment:
   为什么不抛未实现的异常,而是返回 java.lang.Object@d716361 这个样式的字符串?



-- 
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 #11066: [Refactor](Nereids) Remove expression type.

Posted by GitBox <gi...@apache.org>.
924060929 merged PR #11066:
URL: https://github.com/apache/doris/pull/11066


-- 
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] wangshuo128 commented on a diff in pull request #11066: [Refactor](Nereids) Remove expression type.

Posted by GitBox <gi...@apache.org>.
wangshuo128 commented on code in PR #11066:
URL: https://github.com/apache/doris/pull/11066#discussion_r926309324


##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/Regexp.java:
##########
@@ -30,19 +30,14 @@
 public class Regexp extends StringRegexPredicate {
 
     public Regexp(Expression left, Expression right) {
-        super(ExpressionType.REGEXP, left, right);
+        super(left, right, "regexp");
     }
 
     @Override
     public boolean nullable() throws UnboundException {
         return left().nullable();
     }
 
-    @Override

Review Comment:
   It's pulled up to base class.



-- 
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