spark-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From kiszk <...@git.apache.org>
Subject [GitHub] spark pull request #20850: [SPARK-23713][SQL] Cleanup UnsafeWriter and Buffe...
Date Mon, 19 Mar 2018 17:43:01 GMT
Github user kiszk commented on a diff in the pull request:

    https://github.com/apache/spark/pull/20850#discussion_r175525056
  
    --- Diff: sql/catalyst/src/main/java/org/apache/spark/sql/catalyst/expressions/codegen/UnsafeRowWriter.java
---
    @@ -40,29 +37,45 @@
      */
     public final class UnsafeRowWriter extends UnsafeWriter {
     
    -  private final BufferHolder holder;
    -  // The offset of the global buffer where we start to write this row.
    -  private int startingOffset;
    +  private final UnsafeRow row;
    +
       private final int nullBitsSize;
       private final int fixedSize;
     
    -  public UnsafeRowWriter(BufferHolder holder, int numFields) {
    -    this.holder = holder;
    +  public UnsafeRowWriter(UnsafeRow row, int initialBufferSize) {
    +    this(row, new BufferHolder(row, initialBufferSize), row.numFields());
    +  }
    +
    +  public UnsafeRowWriter(UnsafeRow row) {
    +    this(row, new BufferHolder(row), row.numFields());
    +  }
    +
    +  public UnsafeRowWriter(UnsafeWriter writer, int numFields) {
    +    this(null, writer.getBufferHolder(), numFields);
    +  }
    +
    +  private UnsafeRowWriter(UnsafeRow row, BufferHolder holder, int numFields) {
    +    super(holder);
    +    this.row = row;
         this.nullBitsSize = UnsafeRow.calculateBitSetWidthInBytes(numFields);
         this.fixedSize = nullBitsSize + 8 * numFields;
    -    this.startingOffset = holder.cursor;
    +    this.startingOffset = cursor();
    +  }
    +
    +  public void setTotalSize() {
    --- End diff --
    
    It could be. Beyond that, the current approach using `reset` and `setTotalSize()` looks
easy to read the generated code.
    It is clear to understand the beginning and end of the region. If it is critical to remove
the `UnsafeWriter.reset` method, I agree with renaming to `flip`.
    WDYT?
    



---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Mime
View raw message