[GitHub] spark issue #20419: [SPARK-23032][SQL][FOLLOW-UP]Add codegenStageId in comme...

Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/20419 Merged build finished. Test FAILed. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional

[GitHub] spark issue #20419: [SPARK-23032][SQL][FOLLOW-UP]Add codegenStageId in comme...

Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/20419 Test FAILed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/87405/ Test FAILed. ---

[GitHub] spark issue #20419: [SPARK-23032][SQL][FOLLOW-UP]Add codegenStageId in comme...

Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/20419 **[Test build #87405 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/87405/testReport)** for PR 20419 at commit

[GitHub] spark issue #20419: [SPARK-23032][SQL][FOLLOW-UP]Add codegenStageId in comme...

Github user kiszk commented on the issue: https://github.com/apache/spark/pull/20419 I see. I will open a new JIRA tomorrow. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional

[GitHub] spark issue #20419: [SPARK-23032][SQL][FOLLOW-UP]Add codegenStageId in comme...

Github user gatorsmile commented on the issue: https://github.com/apache/spark/pull/20419 @kiszk This is unable to merge to 2.3. Could you open a new JIRA for this? --- - To unsubscribe, e-mail:

[GitHub] spark issue #20419: [SPARK-23032][SQL][FOLLOW-UP]Add codegenStageId in comme...

Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/20419 Merged build finished. Test PASSed. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional

[GitHub] spark issue #20419: [SPARK-23032][SQL][FOLLOW-UP]Add codegenStageId in comme...

Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/20419 Test PASSed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution/863/

[GitHub] spark issue #20419: [SPARK-23032][SQL][FOLLOW-UP]Add codegenStageId in comme...

Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/20419 **[Test build #87405 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/87405/testReport)** for PR 20419 at commit

[GitHub] spark issue #20419: [SPARK-23032][SQL][FOLLOW-UP]Add codegenStageId in comme...

Github user kiszk commented on the issue: https://github.com/apache/spark/pull/20419 ping @rednaxelafx --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail:

[GitHub] spark issue #20419: [SPARK-23032][SQL][FOLLOW-UP]Add codegenStageId in comme...

Github user kiszk commented on the issue: https://github.com/apache/spark/pull/20419 ping @rednaxelafx --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail:

[GitHub] spark issue #20419: [SPARK-23032][SQL][FOLLOW-UP]Add codegenStageId in comme...

Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/20419 Test PASSed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/87084/ Test PASSed. ---

[GitHub] spark issue #20419: [SPARK-23032][SQL][FOLLOW-UP]Add codegenStageId in comme...

Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/20419 Merged build finished. Test PASSed. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional

[GitHub] spark issue #20419: [SPARK-23032][SQL][FOLLOW-UP]Add codegenStageId in comme...

Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/20419 **[Test build #87084 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/87084/testReport)** for PR 20419 at commit

[GitHub] spark issue #20419: [SPARK-23032][SQL][FOLLOW-UP]Add codegenStageId in comme...

Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/20419 **[Test build #87084 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/87084/testReport)** for PR 20419 at commit

[GitHub] spark issue #20419: [SPARK-23032][SQL][FOLLOW-UP]Add codegenStageId in comme...

Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/20419 Test PASSed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution/605/

[GitHub] spark issue #20419: [SPARK-23032][SQL][FOLLOW-UP]Add codegenStageId in comme...

Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/20419 Merged build finished. Test PASSed. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional

[GitHub] spark issue #20419: [SPARK-23032][SQL][FOLLOW-UP]Add codegenStageId in comme...

Github user kiszk commented on the issue: https://github.com/apache/spark/pull/20419 Retest this please --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail:

[GitHub] spark issue #20419: [SPARK-23032][SQL][FOLLOW-UP]Add codegenStageId in comme...

Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/20419 Merged build finished. Test FAILed. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional

[GitHub] spark issue #20419: [SPARK-23032][SQL][FOLLOW-UP]Add codegenStageId in comme...

Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/20419 Test FAILed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/87076/ Test FAILed. ---

[GitHub] spark issue #20419: [SPARK-23032][SQL][FOLLOW-UP]Add codegenStageId in comme...

Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/20419 **[Test build #87076 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/87076/testReport)** for PR 20419 at commit

[GitHub] spark issue #20419: [SPARK-23032][SQL][FOLLOW-UP]Add codegenStageId in comme...

Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/20419 **[Test build #87076 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/87076/testReport)** for PR 20419 at commit

[GitHub] spark issue #20419: [SPARK-23032][SQL][FOLLOW-UP]Add codegenStageId in comme...

Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/20419 Test PASSed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution/598/

[GitHub] spark issue #20419: [SPARK-23032][SQL][FOLLOW-UP]Add codegenStageId in comme...

Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/20419 Merged build finished. Test PASSed. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional

[GitHub] spark issue #20419: [SPARK-23032][SQL][FOLLOW-UP]Add codegenStageId in comme...

Github user rednaxelafx commented on the issue: https://github.com/apache/spark/pull/20419 @kiszk For this specific kind of usage, I don't think using a hardcoded stable ID will be a problem. The comment we're talking about is the kind the can only appear once in a single

[GitHub] spark issue #20419: [SPARK-23032][SQL][FOLLOW-UP]Add codegenStageId in comme...

Github user kiszk commented on the issue: https://github.com/apache/spark/pull/20419 @rednaxelafx I understand your concern when `ctx.registerComment()` is conditionally called. If we call `ctx.registerComment()` with the specific identified multiple times, how do we handle?

[GitHub] spark issue #20419: [SPARK-23032][SQL][FOLLOW-UP]Add codegenStageId in comme...

Github user rednaxelafx commented on the issue: https://github.com/apache/spark/pull/20419 I gave it a bit more thought. Here's an alternative proposal: instead of using a "force comment" mechanism in the current form (which still gets a `ctx.freshName("c")` that the caller has no

[GitHub] spark issue #20419: [SPARK-23032][SQL][FOLLOW-UP]Add codegenStageId in comme...

Github user rednaxelafx commented on the issue: https://github.com/apache/spark/pull/20419 I did the experiment that I asked about at https://github.com/apache/spark/pull/20419#issuecomment-361359825 , and verified that under the current implementation, this PR will not affect the

[GitHub] spark issue #20419: [SPARK-23032][SQL][FOLLOW-UP]Add codegenStageId in comme...

Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/20419 Test PASSed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/86882/ Test PASSed. ---

[GitHub] spark issue #20419: [SPARK-23032][SQL][FOLLOW-UP]Add codegenStageId in comme...

Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/20419 Merged build finished. Test PASSed. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional

[GitHub] spark issue #20419: [SPARK-23032][SQL][FOLLOW-UP]Add codegenStageId in comme...

Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/20419 **[Test build #86882 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/86882/testReport)** for PR 20419 at commit

[GitHub] spark issue #20419: [SPARK-23032][SQL][FOLLOW-UP]Add codegenStageId in comme...

Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/20419 Test PASSed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution/436/

[GitHub] spark issue #20419: [SPARK-23032][SQL][FOLLOW-UP]Add codegenStageId in comme...

Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/20419 Merged build finished. Test PASSed. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional

[GitHub] spark issue #20419: [SPARK-23032][SQL][FOLLOW-UP]Add codegenStageId in comme...

Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/20419 **[Test build #86882 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/86882/testReport)** for PR 20419 at commit

[GitHub] spark issue #20419: [SPARK-23032][SQL][FOLLOW-UP]Add codegenStageId in comme...

Github user gatorsmile commented on the issue: https://github.com/apache/spark/pull/20419 Then, we should add a test case to ensure it will not be broken. --- - To unsubscribe, e-mail:

[GitHub] spark issue #20419: [SPARK-23032][SQL][FOLLOW-UP]Add codegenStageId in comme...

Github user rednaxelafx commented on the issue: https://github.com/apache/spark/pull/20419 @gatorsmile Not directly. The `CodeAndComment` case class is just a "container", it doesn't handle what gets into the `body` field. When we force embed a comment, it'll leave a comment as a

[GitHub] spark issue #20419: [SPARK-23032][SQL][FOLLOW-UP]Add codegenStageId in comme...

Github user gatorsmile commented on the issue: https://github.com/apache/spark/pull/20419 @rednaxelafx Does the following [codes](https://github.com/apache/spark/blob/master/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeGenerator.scala#L1286-L1294)

[GitHub] spark issue #20419: [SPARK-23032][SQL][FOLLOW-UP]Add codegenStageId in comme...

Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/20419 Merged build finished. Test PASSed. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional

[GitHub] spark issue #20419: [SPARK-23032][SQL][FOLLOW-UP]Add codegenStageId in comme...

Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/20419 Test PASSed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/86774/ Test PASSed. ---

[GitHub] spark issue #20419: [SPARK-23032][SQL][FOLLOW-UP]Add codegenStageId in comme...

Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/20419 **[Test build #86774 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/86774/testReport)** for PR 20419 at commit

[GitHub] spark issue #20419: [SPARK-23032][SQL][FOLLOW-UP]Add codegenStageId in comme...

Github user rednaxelafx commented on the issue: https://github.com/apache/spark/pull/20419 @kiszk SGTM and LGTM. Let's ship it! One more question on the side: with the `forceComment = true`, are we fully sure that won't affect the equality of `CodeAndComment`? The whole

[GitHub] spark issue #20419: [SPARK-23032][SQL][FOLLOW-UP]Add codegenStageId in comme...

Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/20419 **[Test build #86774 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/86774/testReport)** for PR 20419 at commit

[GitHub] spark issue #20419: [SPARK-23032][SQL][FOLLOW-UP]Add codegenStageId in comme...

Github user kiszk commented on the issue: https://github.com/apache/spark/pull/20419 I always leave the comment regardless of the `spark.sql.codegen.useIdInClassName` for a unified way to access the ID from the comment. ---

[GitHub] spark issue #20419: [SPARK-23032][SQL][FOLLOW-UP]Add codegenStageId in comme...

Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/20419 Test PASSed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution/341/

[GitHub] spark issue #20419: [SPARK-23032][SQL][FOLLOW-UP]Add codegenStageId in comme...

Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/20419 Merged build finished. Test PASSed. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional

[GitHub] spark issue #20419: [SPARK-23032][SQL][FOLLOW-UP]Add codegenStageId in comme...

Github user rednaxelafx commented on the issue: https://github.com/apache/spark/pull/20419 LGTM, and +1 on @viirya 's idea. I like it better for the comment to be on top of the class declaration instead of inside it; but I'm okay either way if others have strong opinion otherwise. As

[GitHub] spark issue #20419: [SPARK-23032][SQL][FOLLOW-UP]Add codegenStageId in comme...

Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/20419 Test PASSed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/86745/ Test PASSed. ---

[GitHub] spark issue #20419: [SPARK-23032][SQL][FOLLOW-UP]Add codegenStageId in comme...

Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/20419 Merged build finished. Test PASSed. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional

[GitHub] spark issue #20419: [SPARK-23032][SQL][FOLLOW-UP]Add codegenStageId in comme...

Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/20419 **[Test build #86745 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/86745/testReport)** for PR 20419 at commit

[GitHub] spark issue #20419: [SPARK-23032][SQL][FOLLOW-UP]Add codegenStageId in comme...

Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/20419 **[Test build #86745 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/86745/testReport)** for PR 20419 at commit

[GitHub] spark issue #20419: [SPARK-23032][SQL][FOLLOW-UP]Add codegenStageId in comme...

Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/20419 Test PASSed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution/316/

[GitHub] spark issue #20419: [SPARK-23032][SQL][FOLLOW-UP]Add codegenStageId in comme...

Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/20419 Merged build finished. Test PASSed. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional

[GitHub] spark issue #20419: [SPARK-23032][SQL][FOLLOW-UP]Add codegenStageId in comme...

Github user kiszk commented on the issue: https://github.com/apache/spark/pull/20419 ping @rednaxelafx --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: