viirya commented on PR #59581:
URL: https://github.com/apache/spark/pull/59581#issuecomment-6276911959
Thanks for raising this. Nothing in this PR branches on the marker; it
matters for #59582, where the Arrow columnar path decides which input columns
it may hold.
For #59582, we could also treat a vector as reused when its class overrides
`closeIfFreeable`, which is already the public signal: the doc of
`ColumnVector.closeIfFreeable` says that writable and constant vectors, which
are reused across batches, override it to do nothing. It can be checked once
per class, e.g. `cls.getMethod("closeIfFreeable").getDeclaringClass !=
classOf[ColumnVector]`. The check cannot tell whether the override is a no-op,
but it errs on the safe side: a vector wrongly treated as reused only takes the
slower row queue path. A third-party Arrow vector that follows the public doc
would then not be held, and we would document the hold/copy protocol on
`ColumnVector.closeIfFreeable`.
Taken further, the internal marker could go away. The in-tree vectors
carrying it (`WritableColumnVector`, `ConstantColumnVector` and
`ReusedArrowColumnVector`) are exactly those whose `closeIfFreeable` is
overridden as a no-op, so the override alone could be the protocol, with a
single public rule. The marker states the intent explicitly and is a plain type
check, while the override check relies on reflection. Which would you prefer:
keeping the marker and also honoring the override, or only the override?
A source that wraps a reused root in a plain `ArrowColumnVector` without
overriding `closeIfFreeable` would still be held. Its vectors are already
closed by the generated code of `ColumnarToRowExec` since SPARK-50235, though,
so it is broken today regardless of this path.
--
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]
