sunchao commented on code in PR #5560:
URL: https://github.com/apache/datafusion-comet/pull/5560#discussion_r3991503568


##########
spark/src/main/spark-4.x/org/apache/spark/sql/execution/python/CometArrowPythonRunnerBase.scala:
##########
@@ -338,6 +348,170 @@ private[python] trait CometArrowPythonRunnerBase
 
 private[python] object CometArrowPythonRunnerBase {
 
+  // A regular Arrow variable-width data buffer uses signed 32-bit offsets. 
The Spark setting is
+  // already restricted to this range, but cap it here as a final guard for 
direct test callers.
+  private val MaxDecodedBatchBytes = Int.MaxValue.toLong
+
+  private def dictionaryVector(column: CometDictionaryVector): FieldVector = {
+    val indices = column.getValueVector
+    val encoding = indices.getField.getDictionary
+    column.getDictionaryProvider.lookup(encoding.getId).getVector
+  }
+
+  private def initialDecodedBytes(values: FieldVector): Long =
+    values match {
+      case _: BaseVariableWidthVector => BaseVariableWidthVector.OFFSET_WIDTH
+      case _: BaseLargeVariableWidthVector => 
BaseLargeVariableWidthVector.OFFSET_WIDTH
+      case _ => 0L
+    }
+
+  /** Conservative logical bytes added by one decoded dictionary value. */
+  private def decodedValueBytes(
+      column: CometDictionaryVector,
+      values: FieldVector,
+      row: Int,
+      batchRow: Int): Long = {
+    val dictionaryIndex = if (column.isNullAt(row)) -1 else 
column.indices.getInt(row)
+    val validityBytes = if ((batchRow & 7) == 0) 1L else 0L
+    values match {
+      case vector: BaseVariableWidthVector =>
+        val valueBytes = if (dictionaryIndex < 0) 0L else 
vector.getValueLength(dictionaryIndex)
+        valueBytes + BaseVariableWidthVector.OFFSET_WIDTH + validityBytes
+      case vector: BaseLargeVariableWidthVector =>
+        val valueBytes = if (dictionaryIndex < 0) 0L else 
vector.getValueLength(dictionaryIndex)
+        valueBytes + BaseLargeVariableWidthVector.OFFSET_WIDTH + validityBytes
+      case vector: BaseFixedWidthVector =>
+        vector.getBufferSizeFor(batchRow + 1).toLong -
+          vector.getBufferSizeFor(batchRow).toLong
+      case _: NullVector => 0L
+      case vector =>
+        // Comet's JVM shuffle currently dictionary-encodes only strings and 
binary values.
+        // If another Arrow type reaches this path, the complete dictionary is 
a safe upper
+        // bound for any one selected value and favors smaller batches over a 
large allocation.
+        math.max(1L, vector.getBufferSize.toLong)
+    }
+  }
+
+  private def saturatedAdd(left: Long, right: Long): Long =
+    if (right >= Long.MaxValue - left) Long.MaxValue else left + right
+
+  /**
+   * Split a compact dictionary batch before decoding it.
+   *
+   * The byte estimate covers the temporary logical dictionary vectors. Plain 
input vectors are
+   * already allocated and remain zero-copy when no dictionary column is 
present. Every returned
+   * range is applied to all columns so rows stay aligned. A single oversized 
row is allowed,
+   * matching Spark's Arrow batching contract.
+   */
+  private[python] def inputBatchRanges(

Review Comment:
   Added the dictionary-entry maximum bound, which returns the whole batch when 
it provably fits both limits without reading row indices. The fallback now uses 
while loops over parallel arrays and computes each selected value length once; 
a split only adjusts bitmap accounting.
   
   [`a conservative dictionary bound accepts small batches without reading row 
indices`](https://github.com/apache/datafusion-comet/blob/e75103f8b51bab3ec440bd9ee0fcb16b0ec4e069/spark/src/test/spark-4.x/org/apache/spark/sql/execution/python/CometArrowPythonRunnerSuite.scala#L947)
 passes locally on Spark 4.1: it asserts zero index reads for the accepted 
batch and exactly one per row when forced to split. The seeded range-oracle 
test also passes.



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

Reply via email to