andygrove commented on PR #6130:
URL:
https://github.com/apache/datafusion-comet/pull/6130#issuecomment-5876781464
This is a light fully automated review since there are so many PRs open.
Unlike its sibling operators, `CometArrowEvalPythonExec`
(`spark/src/main/spark-4.1+/org/apache/spark/sql/comet/CometArrowEvalPythonExec.scala:152`)
doesn't override `stringArgs`, `equals` or `hashCode`. Spark's default
`stringArgs` is `productIterator`, so `argString` prints `nativeOp` through
protobuf's `toString`, which dumps the whole native subtree. For
`spark.read.parquet("s3a://...").select(f("x"))` that includes the pickled
command bytes and the child scan's `object_store_options`, which
`NativeConfig.extractObjectStoreOptions` fills with every `fs.s3a.*` key,
`fs.s3a.secret.key` included. That text lands in `explain()`, the SQL UI and
the event log, although `CometNativeScanExec` itself prints only `output`. The
default `equals` and `hashCode` also compare `nativeOp`, whose `plan_id`
differs between two otherwise identical subtrees, so an exchange above a native
UDF can't be reused and a self-join runs the Python function once per side.
Could this override the three metho
ds the way `CometProjectExec` does, keeping the UDFs in the identity so two
functions with the same result type still differ? A test asserting the plan
string stays free of the command would guard against a regression.
`docs/source/user-guide/latest/pyarrow-udfs.md:153` says PyArrow allocations
are outside Comet's memory pool. They are also missing from the `allocated`
figure in the executor's memory usage log, which counts only Rust's global
allocator, even though the UDF output columns then flow through native
operators that may reserve them. Sizing `spark.executor.memoryOverhead` from
`allocated - reserved`, as the tuning guide describes, would come up short by
whatever the interpreter and PyArrow hold. Could the guide say to add that on
top, and could the non-Rust allocations list at
`docs/source/contributor-guide/memory_management.md:364` mention the embedded
interpreter and PyArrow's allocator?
--
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]