You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "tedyu (via GitHub)" <gi...@apache.org> on 2023/07/08 14:24:02 UTC
[GitHub] [spark] tedyu opened a new pull request, #41901: Reuse evaluator in ColumnarToRowExec#doExecute
tedyu opened a new pull request, #41901:
URL: https://github.com/apache/spark/pull/41901
### What changes were proposed in this pull request?
In `ColumnarToRowExec#doExecute`, we can instantiate the evaluator and use it in `mapPartitionsInternal`.
### Why are the changes needed?
We don't need to instantiate the evaluator for every iteration.
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
Existing test suite.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] datavisortedyu commented on pull request #41901: [SPARK-44287][SQL] Reuse evaluator in ColumnarToRowExec#doExecute
Posted by "datavisortedyu (via GitHub)" <gi...@apache.org>.
datavisortedyu commented on PR #41901:
URL: https://github.com/apache/spark/pull/41901#issuecomment-1631735223
@cloud-fan
I took a look last night. But I haven't figured out how to make the evaluator factory serializable.
It would be prudent to temporarily revert the ColumnarToRowExec change until we figure out the serialization part.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on pull request #41901: Reuse evaluator in ColumnarToRowExec#doExecute
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on PR #41901:
URL: https://github.com/apache/spark/pull/41901#issuecomment-1629943430
LGTM. Can we add the JITA ID to the PR title?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] tedyu commented on pull request #41901: Reuse evaluator in ColumnarToRowExec#doExecute
Posted by "tedyu (via GitHub)" <gi...@apache.org>.
tedyu commented on PR #41901:
URL: https://github.com/apache/spark/pull/41901#issuecomment-1627339532
cc @vinodkc @cloud-fan
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] tedyu commented on pull request #41901: [SPARK-44287] Reuse evaluator in ColumnarToRowExec#doExecute
Posted by "tedyu (via GitHub)" <gi...@apache.org>.
tedyu commented on PR #41901:
URL: https://github.com/apache/spark/pull/41901#issuecomment-1629996387
@cloud-fan
I have added the JIRA to title.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on pull request #41901: [SPARK-44287][SQL] Reuse evaluator in ColumnarToRowExec#doExecute
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on PR #41901:
URL: https://github.com/apache/spark/pull/41901#issuecomment-1631720401
@tedyu can you look into it? It seems we should revert the `ColumnarToRowExec` change if we can't serialize the partition evaluator factory.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] tedyu commented on pull request #41901: [SPARK-44287][SQL] Reuse evaluator in ColumnarToRowExec#doExecute
Posted by "tedyu (via GitHub)" <gi...@apache.org>.
tedyu commented on PR #41901:
URL: https://github.com/apache/spark/pull/41901#issuecomment-1630106640
Should have looked closer.
The evaluator has serialization error.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] datavisortedyu commented on pull request #41901: [SPARK-44287][SQL] Reuse evaluator in ColumnarToRowExec#doExecute
Posted by "datavisortedyu (via GitHub)" <gi...@apache.org>.
datavisortedyu commented on PR #41901:
URL: https://github.com/apache/spark/pull/41901#issuecomment-1630083650
I was looking at https://github.com/tedyu/spark/actions/runs/5496387824/jobs/10016389918
```
This step has been truncated due to its large size. Download the full logs from the menu once the workflow run has completed.
```
Still searching for the failed test(s).
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on pull request #41901: [SPARK-44287][SQL] Reuse evaluator in ColumnarToRowExec#doExecute
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on PR #41901:
URL: https://github.com/apache/spark/pull/41901#issuecomment-1631738254
ah sorry I misread your message. partition evaluator factory is serializable but partition evaluator is not. So the current code is correct.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] datavisortedyu commented on pull request #41901: [SPARK-44287][SQL] Reuse evaluator in ColumnarToRowExec#doExecute
Posted by "datavisortedyu (via GitHub)" <gi...@apache.org>.
datavisortedyu commented on PR #41901:
URL: https://github.com/apache/spark/pull/41901#issuecomment-1630086871
From https://github.com/tedyu/spark/actions/runs/5496387824/jobs/10016389977:
```
[error] /home/runner/work/spark/spark/mllib/target/java/org/apache/spark/mllib/util/MLlibTestSparkContext.java:10:1: error: illegal combination of modifiers: public and protected
[error] protected class testImplicits {
[error] ^
[error] /home/runner/work/spark/spark/sql/core/target/java/org/apache/spark/sql/UDFSuite.java:4:1: error: modifier static not allowed here
[error] static public class MalformedNonPrimitiveFunction implements scala.Function1<java.lang.String, java.lang.Object>, scala.Serializable {
[error] ^
[error] /home/runner/work/spark/spark/sql/core/target/java/org/apache/spark/sql/test/SQLTestUtilsBase.java:19:1: error: illegal combination of modifiers: public and protected
[error] protected class testImplicits {
[error] ^
[error] /home/runner/work/spark/spark/common/sketch/target/java/org/apache/spark/util/sketch/BitArraySuite.java:2:1: error: package org.scalatest.funsuite does not exist
[error] public class BitArraySuite extends org.scalatest.funsuite.AnyFunSuite {
[error] ^
```
I don't think the above was caused by my PR.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] tedyu closed pull request #41901: [SPARK-44287][SQL] Reuse evaluator in ColumnarToRowExec#doExecute
Posted by "tedyu (via GitHub)" <gi...@apache.org>.
tedyu closed pull request #41901: [SPARK-44287][SQL] Reuse evaluator in ColumnarToRowExec#doExecute
URL: https://github.com/apache/spark/pull/41901
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on pull request #41901: [SPARK-44287][SQL] Reuse evaluator in ColumnarToRowExec#doExecute
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on PR #41901:
URL: https://github.com/apache/spark/pull/41901#issuecomment-1630072004
I think the test failures are relevant though ..
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org