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-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
