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

    https://github.com/apache/incubator-carbondata/pull/450#discussion_r93943284
  
    --- Diff: 
core/src/main/java/org/apache/carbondata/core/carbon/datastore/chunk/store/impl/AbstractDimensionDataChunkStore.java
 ---
    @@ -0,0 +1,170 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing,
    + * software distributed under the License is distributed on an
    + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    + * KIND, either express or implied.  See the License for the
    + * specific language governing permissions and limitations
    + * under the License.
    + */
    +package org.apache.carbondata.core.carbon.datastore.chunk.store.impl;
    +
    +import 
org.apache.carbondata.core.carbon.datastore.chunk.store.DimensionDataChunkStore;
    +import org.apache.carbondata.core.constants.CarbonCommonConstants;
    +import org.apache.carbondata.core.memory.MemoryAllocatorFactory;
    +import org.apache.carbondata.core.memory.MemoryBlock;
    +import org.apache.carbondata.core.unsafe.CarbonUnsafe;
    +
    +/**
    + * Responsibility is to store dimension data in memory.
    + * storage can be on heap or offheap.
    + */
    +public abstract class AbstractDimensionDataChunkStore implements 
DimensionDataChunkStore {
    +
    +  /**
    +   * memory block for data page
    +   */
    +  protected MemoryBlock dataPageMemoryBlock;
    +
    +  /**
    +   * memory block for inverted index
    +   */
    +  protected MemoryBlock invertedIndexMemoryBlock;
    +
    +  /**
    +   * memory block for inverted index reverse
    +   */
    +  protected MemoryBlock invertedIndexReverseMemoryBlock;
    +
    +  /**
    +   * to check whether dimension column was explicitly sorted or not
    +   */
    +  protected boolean isExplictSorted;
    +
    +  /**
    +   * is memory released
    +   */
    +  protected boolean isMemoryReleased;
    +
    +  /**
    +   * Constructor
    +   *
    +   * @param totalSize      total size of the data to be kept
    +   * @param isInvertedIdex is inverted index present
    +   * @param numberOfRows   total number of rows
    +   */
    +  public AbstractDimensionDataChunkStore(int totalSize, boolean 
isInvertedIdex, int numberOfRows) {
    +    // allocating the data page
    +    this.dataPageMemoryBlock =
    +        
MemoryAllocatorFactory.INSATANCE.getMemoryAllocator().allocate(totalSize);
    +    this.isExplictSorted = isInvertedIdex;
    +    if (isInvertedIdex) {
    +      // allocating the inverted index page memory
    +      invertedIndexMemoryBlock = 
MemoryAllocatorFactory.INSATANCE.getMemoryAllocator()
    +          .allocate(CarbonCommonConstants.INT_SIZE_IN_BYTE * numberOfRows);
    +      // allocating the inverted index reverese page memory
    +      invertedIndexReverseMemoryBlock = 
MemoryAllocatorFactory.INSATANCE.getMemoryAllocator()
    +          .allocate(CarbonCommonConstants.INT_SIZE_IN_BYTE * numberOfRows);
    +    }
    +  }
    +
    +  /**
    +   * Below method will be used to put the rows and its metadata in offheap
    +   *
    +   * @param invertedIndex        inverted index to be stored
    +   * @param invertedIndexReverse inverted index reverse to be stored
    +   * @param data                 data to be stored
    +   */
    +  @Override public void putArray(final int[] invertedIndex, final int[] 
invertedIndexReverse,
    +      final byte[] data) {
    +    // copy the data to memory
    +    CarbonUnsafe.unsafe
    +        .copyMemory(data, CarbonUnsafe.BYTE_ARRAY_OFFSET, 
dataPageMemoryBlock.getBaseObject(),
    +            dataPageMemoryBlock.getBaseOffset(), 
dataPageMemoryBlock.size());
    --- End diff --
    
    Here, the copyMemory will not check whether the data is out of boundary. It 
is better to wrap it with a safe boundary check. Same to other xxxChunkStore 
class below and the getxxx function which read data from off-heap memory 
internally.


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