mcvsubbu commented on a change in pull request #5503:
URL: https://github.com/apache/incubator-pinot/pull/5503#discussion_r436192934



##########
File path: 
pinot-core/src/main/java/org/apache/pinot/core/io/writer/impl/v1/BaseChunkSingleValueWriter.java
##########
@@ -36,6 +37,9 @@
  * Base class for fixed and variable byte writer implementations.
  */
 public abstract class BaseChunkSingleValueWriter implements 
SingleColumnSingleValueWriter {
+  public static final int DEFAULT_VERSION = 2;

Review comment:
       Maybe add a TODO or create an issue to remove this before 0.5.0 and put 
a pointer here to the issue?

##########
File path: 
pinot-core/src/main/java/org/apache/pinot/core/io/writer/impl/v1/BaseChunkSingleValueWriter.java
##########
@@ -60,12 +64,13 @@
    * @param numDocsPerChunk Number of docs per data chunk
    * @param chunkSize Size of chunk
    * @param sizeOfEntry Size of entry (in bytes), max size for variable byte 
implementation.
-   * @param version Version of file
+   * @param version format version used to determine whether to use 8 or 4 
byte chunk offsets

Review comment:
       this comment may not hold in the long term, so just leave it as is

##########
File path: 
pinot-core/src/main/java/org/apache/pinot/core/io/writer/impl/v1/FixedByteChunkSingleValueWriter.java
##########
@@ -64,15 +62,15 @@
    * @param compressionType Type of compression to use.
    * @param totalDocs Total number of docs to write.
    * @param numDocsPerChunk Number of documents per chunk.
-   * @param sizeOfEntry Size of entry (in bytes).
+   * @param sizeOfEntry Size of entry (in bytes)
+   * @param writerVersion writer version used to determine whether to use 8 or 
4 byte chunk offsets

Review comment:
       same

##########
File path: 
pinot-core/src/main/java/org/apache/pinot/core/io/writer/impl/v1/BaseChunkSingleValueWriter.java
##########
@@ -36,6 +37,9 @@
  * Base class for fixed and variable byte writer implementations.
  */
 public abstract class BaseChunkSingleValueWriter implements 
SingleColumnSingleValueWriter {
+  public static final int DEFAULT_VERSION = 2;

Review comment:
       ```suggestion
     public static final int DEFAULT_WRITER_VERSION = 2;
   ```




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

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org

Reply via email to