Github user JamesRTaylor commented on a diff in the pull request:

    https://github.com/apache/phoenix/pull/34#discussion_r23670722
  
    --- Diff: 
phoenix-core/src/main/java/org/apache/phoenix/expression/Expression.java ---
    @@ -88,4 +88,9 @@
          * @return
          */
         boolean requiresFinalEvaluation();
    +    
    +    /**
    +     * @return the estimated size in bytes of the expression when 
serialized
    +     */
    +    int getEstimatedByteSize();
    --- End diff --
    
    I think this may not be worth adding, as the more methods we have here, the 
higher the burden of adding new expressions. The only place we use this is for 
the IndexMaintainer to serialize it's bytes (true?), but we'll likely move 
toward Protobufs in the next major release. Could we have a crude estimate 
instead? Maybe based on total number of expressions in the tree (which we could 
track in ExpressionCompiler in this method (by just have a totalNodeCount 
member variable, incrementing it here by one, and resetting it to zero in 
reset()):
    
         @Override
        public void addElement(List<Expression> l, Expression element) {
            nodeCount--;
            l.add(element);
        }
    
    Thoughts? 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

Reply via email to