dongjoon-hyun commented on code in PR #58228:
URL: https://github.com/apache/spark/pull/58228#discussion_r3839487585
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/columnar/compression/CompressionScheme.scala:
##########
@@ -45,6 +45,13 @@ private[columnar] trait Decoder[T <: PhysicalDataType] {
def hasNext: Boolean
def decompress(columnVector: WritableColumnVector, capacity: Int): Unit
+
+ /** Duplicates `buffer` in native byte order and rewinds it, for reading
null state. */
+ protected def createNullsBuffer(buffer: ByteBuffer): ByteBuffer = {
Review Comment:
Could we put this on `object CompressionScheme` instead of the `Decoder`
trait?
The helper never references `this` — it's a standalone `ByteBuffer` utility.
Making it a `protected` member of `Decoder` adds it to the decoder contract, so
every current and future implementation inherits it, which seems like more than
this needs. `object CompressionScheme` in this same file already hosts a
comparable helper (`columnHeaderSize`, which uses the same
`duplicate().order(nativeOrder)` idiom), so it looks like the more natural home:
```scala
private[columnar] object CompressionScheme {
...
def createNullsBuffer(buffer: ByteBuffer): ByteBuffer = {
val nullsBuffer = buffer.duplicate().order(ByteOrder.nativeOrder())
nullsBuffer.rewind()
nullsBuffer
}
}
```
The call sites would then read `CompressionScheme.createNullsBuffer(buffer)`.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
