Github user jtaylor-sfdc commented on a diff in the pull request:

    https://github.com/apache/incubator-phoenix/pull/8#discussion_r9980892
  
    --- Diff: 
phoenix-core/src/main/java/org/apache/phoenix/expression/ArrayConstructorExpression.java
 ---
    @@ -39,18 +31,27 @@
         private PDataType baseType;
         private int position = -1;
         private Object[] elements;
    -    
    -    
    +    private TrustedByteArrayOutputStream byteStream = null;
    +    private DataOutputStream oStream = null;
    +    private int estimatedSize = 0;
    +    // store the offset postion in this.  Later based on the total size 
move this to a byte[]
    +    // and serialize into byte stream
    +    private int[] offsetPos;
    +
         public ArrayConstructorExpression(List<Expression> children, PDataType 
baseType) {
             super(children);
             init(baseType);
    +        estimatedSize = PArrayDataType.estimateSize(this.children.size(), 
this.baseType);
    +        if (!this.baseType.isFixedWidth()) {
    +            offsetPos = new int[estimatedSize];
    --- End diff --
    
    But this is the same size you're using to estimate the entire serialized
    size of the array. Are you estimating one byte per element? At least use
    ValueSchema.ESTIMATED_VARIABLE_LENGTH_SIZE as a factor to multiply the
    number of elements by for the estimated size.
    
    
    On Sun, Feb 23, 2014 at 8:08 PM, ramkrish86 <[email protected]>wrote:
    
    > In
    > 
phoenix-core/src/main/java/org/apache/phoenix/expression/ArrayConstructorExpression.java:
    >
    > >      public ArrayConstructorExpression(List<Expression> children, 
PDataType baseType) {
    > >          super(children);
    > >          init(baseType);
    > > +        estimatedSize = 
PArrayDataType.estimateSize(this.children.size(), this.baseType);
    > > +        if (!this.baseType.isFixedWidth()) {
    > > +            offsetPos = new int[estimatedSize];
    >
    > Sorry i forgot to update on this here. Actually the estimateSize would
    > return children.size internally. Hence i used estimateSize. Let me change
    > it explicitly here.
    >
    > --
    > Reply to this email directly or view it on 
GitHub<https://github.com/apache/incubator-phoenix/pull/8/files#r9980773>
    > .
    >


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. To do so, please top-post your response.
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
[email protected] or file a JIRA ticket with INFRA.
---

Reply via email to