You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2021/01/15 17:36:46 UTC
[GitHub] [spark] yaooqinn opened a new pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
yaooqinn opened a new pull request #31199:
URL: https://github.com/apache/spark/pull/31199
<!--
Thanks for sending a pull request! Here are some tips for you:
1. If this is your first time, please read our contributor guidelines: https://spark.apache.org/contributing.html
2. Ensure you have added or run the appropriate tests for your PR: https://spark.apache.org/developer-tools.html
3. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP][SPARK-XXXX] Your PR title ...'.
4. Be sure to keep the PR description updated to reflect all changes.
5. Please write your PR title to summarize what this PR proposes.
6. If possible, provide a concise example to reproduce the issue for a faster review.
7. If you want to add a new configuration, please read the guideline first for naming configurations in
'core/src/main/scala/org/apache/spark/internal/config/ConfigEntry.scala'.
-->
### What changes were proposed in this pull request?
<!--
Please clarify what changes you are proposing. The purpose of this section is to outline the changes and how this PR fixes the issue.
If possible, please consider writing useful notes for better and faster reviews in your PR. See the examples below.
1. If you refactor some codes with changing classes, showing the class hierarchy will help reviewers.
2. If you fix some SQL features, you can provide some references of other DBMSes.
3. If there is design documentation, please add the link.
4. If there is a discussion in the mailing list, please add the link.
-->
This could reduce the `generate.java` size to prevent codegen fallback which causes performance regression.
here is a case from tpcds that could be fixed by this improvement
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/133964/testReport/org.apache.spark.sql.execution/LogicalPlanTagInSparkPlanSuite/q41/
The original case generate 20K bytes, we are trying to reduce it to less than 8k
### Why are the changes needed?
<!--
Please clarify why the changes are needed. For instance,
1. If you propose a new API, clarify the use case for a new API.
2. If you fix a bug, you can clarify why it is a bug.
-->
performance improvement
### Does this PR introduce _any_ user-facing change?
<!--
Note that it means *any* user-facing change including all aspects such as the documentation fix.
If yes, please clarify the previous behavior and the change this PR proposes - provide the console output, description and/or an example to show the behavior difference if possible.
If possible, please also clarify if this is a user-facing change compared to the released Spark versions or within the unreleased branches such as master.
If no, write 'No'.
-->
no
### How was this patch tested?
<!--
If tests were added, say they were added here. Please make sure to add some test cases that check the changes thoroughly including negative and positive cases if possible.
If it was tested in a way different from regular unit tests, please clarify how you tested step by step, ideally copy and paste-able, so that other reviewers can test and check, and descendants can verify in the future.
If tests were not added, please describe why they were not added and/or why it was difficult to add.
-->
yes, it's a code reflect so the existing ut should be enough
cross-check with https://github.com/apache/spark/pull/31012 where the tpcds shall all pass
----------------------------------------------------------------
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.
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] AmplabJenkins commented on pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31199:
URL: https://github.com/apache/spark/pull/31199#issuecomment-762395835
----------------------------------------------------------------
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.
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] SparkQA removed a comment on pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #31199:
URL: https://github.com/apache/spark/pull/31199#issuecomment-762306413
**[Test build #134199 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134199/testReport)** for PR 31199 at commit [`1e19de4`](https://github.com/apache/spark/commit/1e19de44a803cf822f5d07bee82aaf982761a96b).
----------------------------------------------------------------
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.
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] AmplabJenkins commented on pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31199:
URL: https://github.com/apache/spark/pull/31199#issuecomment-762300201
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/38781/
----------------------------------------------------------------
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.
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 a change in pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31199:
URL: https://github.com/apache/spark/pull/31199#discussion_r559341457
##########
File path: sql/catalyst/src/main/java/org/apache/spark/sql/catalyst/util/CharVarcharCodegenUtils.java
##########
@@ -0,0 +1,55 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.catalyst.util;
+
+import org.apache.spark.unsafe.types.UTF8String;
+
+public class CharVarcharCodegenUtils {
+ private static final UTF8String SPACE = UTF8String.fromString(" ");
+
+ public static UTF8String lengthCheck(UTF8String inputStr, int limit) {
+ if (inputStr != null && inputStr.numChars() > limit) {
+ throw new RuntimeException("Exceeds varchar type length limitation: " + limit);
Review comment:
`varchar` -> `char/varchar`
----------------------------------------------------------------
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.
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] AmplabJenkins removed a comment on pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31199:
URL: https://github.com/apache/spark/pull/31199#issuecomment-762447194
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/134199/
----------------------------------------------------------------
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.
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] SparkQA commented on pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31199:
URL: https://github.com/apache/spark/pull/31199#issuecomment-761131467
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/38701/
----------------------------------------------------------------
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.
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] viirya commented on a change in pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #31199:
URL: https://github.com/apache/spark/pull/31199#discussion_r558809533
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/CharVarcharUtils.scala
##########
@@ -161,9 +162,20 @@ object CharVarcharUtils extends Logging {
private def paddingWithLengthCheck(expr: Expression, dt: DataType): Expression = dt match {
case CharType(length) =>
- StringRPad(stringLengthCheck(expr, dt, needTrim = false), Literal(length))
-
- case VarcharType(_) => stringLengthCheck(expr, dt, needTrim = false)
+ StaticInvoke(
Review comment:
Oh I see. Then we may need a benchmark to show the difference.
----------------------------------------------------------------
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.
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] SparkQA commented on pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31199:
URL: https://github.com/apache/spark/pull/31199#issuecomment-761119043
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/38705/
----------------------------------------------------------------
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.
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] AmplabJenkins removed a comment on pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31199:
URL: https://github.com/apache/spark/pull/31199#issuecomment-761684385
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/134143/
----------------------------------------------------------------
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.
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] AmplabJenkins commented on pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31199:
URL: https://github.com/apache/spark/pull/31199#issuecomment-762031207
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/134185/
----------------------------------------------------------------
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.
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] maropu commented on pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
maropu commented on pull request #31199:
URL: https://github.com/apache/spark/pull/31199#issuecomment-763275146
late lgtm. Nice fix and thanks, @yaooqinn
----------------------------------------------------------------
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.
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] SparkQA commented on pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31199:
URL: https://github.com/apache/spark/pull/31199#issuecomment-761683144
**[Test build #134143 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134143/testReport)** for PR 31199 at commit [`2010462`](https://github.com/apache/spark/commit/20104628fcaf33a89888e5d116a149ccce2370de).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
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.
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] AmplabJenkins removed a comment on pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31199:
URL: https://github.com/apache/spark/pull/31199#issuecomment-761614559
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/38726/
----------------------------------------------------------------
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.
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] SparkQA commented on pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31199:
URL: https://github.com/apache/spark/pull/31199#issuecomment-762031176
**[Test build #134185 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134185/testReport)** for PR 31199 at commit [`3d82a79`](https://github.com/apache/spark/commit/3d82a799c66410bed77cd637ec28c01f4275f015).
* This patch **fails to build**.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
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.
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] AmplabJenkins commented on pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31199:
URL: https://github.com/apache/spark/pull/31199#issuecomment-761622266
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/38727/
----------------------------------------------------------------
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.
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] SparkQA commented on pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31199:
URL: https://github.com/apache/spark/pull/31199#issuecomment-761597551
**[Test build #134143 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134143/testReport)** for PR 31199 at commit [`2010462`](https://github.com/apache/spark/commit/20104628fcaf33a89888e5d116a149ccce2370de).
----------------------------------------------------------------
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.
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] SparkQA commented on pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31199:
URL: https://github.com/apache/spark/pull/31199#issuecomment-761094615
**[Test build #134122 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134122/testReport)** for PR 31199 at commit [`1f8326e`](https://github.com/apache/spark/commit/1f8326e0215fb68d3708ec9dbd8492bf90c06ab6).
----------------------------------------------------------------
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.
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] SparkQA removed a comment on pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #31199:
URL: https://github.com/apache/spark/pull/31199#issuecomment-761597551
**[Test build #134143 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134143/testReport)** for PR 31199 at commit [`2010462`](https://github.com/apache/spark/commit/20104628fcaf33a89888e5d116a149ccce2370de).
----------------------------------------------------------------
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.
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 #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #31199:
URL: https://github.com/apache/spark/pull/31199#issuecomment-762701071
Yes, I think it's fine to port back. It's not very common case but we can think it's like a perf regression for varchar cases compared to Spark 3.1 in a way.
----------------------------------------------------------------
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.
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] SparkQA commented on pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31199:
URL: https://github.com/apache/spark/pull/31199#issuecomment-762028581
**[Test build #134185 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134185/testReport)** for PR 31199 at commit [`3d82a79`](https://github.com/apache/spark/commit/3d82a799c66410bed77cd637ec28c01f4275f015).
----------------------------------------------------------------
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.
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] SparkQA commented on pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31199:
URL: https://github.com/apache/spark/pull/31199#issuecomment-762373898
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/38787/
----------------------------------------------------------------
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.
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] AmplabJenkins commented on pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31199:
URL: https://github.com/apache/spark/pull/31199#issuecomment-761614559
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/38726/
----------------------------------------------------------------
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.
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] AmplabJenkins commented on pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31199:
URL: https://github.com/apache/spark/pull/31199#issuecomment-761116125
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/134122/
----------------------------------------------------------------
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.
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 #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #31199:
URL: https://github.com/apache/spark/pull/31199#issuecomment-762702242
thanks, merging to master/3.1!
----------------------------------------------------------------
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.
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] SparkQA commented on pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31199:
URL: https://github.com/apache/spark/pull/31199#issuecomment-761135322
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/38705/
----------------------------------------------------------------
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.
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] yaooqinn commented on a change in pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on a change in pull request #31199:
URL: https://github.com/apache/spark/pull/31199#discussion_r558799699
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/CharVarcharUtils.scala
##########
@@ -200,69 +212,60 @@ object CharVarcharUtils extends Logging {
*/
def stringLengthCheck(expr: Expression, targetAttr: Attribute): Expression = {
getRawType(targetAttr.metadata).map { rawType =>
- stringLengthCheck(expr, rawType, needTrim = true)
+ stringLengthCheck(expr, rawType)
}.getOrElse(expr)
}
private def raiseError(typeName: String, length: Int): Expression = {
val errMsg = UTF8String.fromString(s"Exceeds $typeName type length limitation: $length")
- RaiseError(Literal(errMsg, StringType), StringType)
+ RaiseError(Literal(errMsg, StringType), StringType)
}
- private def stringLengthCheck(expr: Expression, dt: DataType, needTrim: Boolean): Expression = {
+ private def stringLengthCheck(expr: Expression, dt: DataType): Expression = {
dt match {
case CharType(length) =>
- val trimmed = if (needTrim) StringTrimRight(expr) else expr
+ val trimmed = StringTrimRight(expr)
// Trailing spaces do not count in the length check. We don't need to retain the trailing
// spaces, as we will pad char type columns/fields at read time.
- If(
- GreaterThan(Length(trimmed), Literal(length)),
- raiseError("char", length),
- trimmed)
+ If(GreaterThan(Length(trimmed), Literal(length)), raiseError("char", length), trimmed)
case VarcharType(length) =>
- if (needTrim) {
- val trimmed = StringTrimRight(expr)
- // Trailing spaces do not count in the length check. We need to retain the trailing spaces
- // (truncate to length N), as there is no read-time padding for varchar type.
- // TODO: create a special TrimRight function that can trim to a certain length.
- If(
- LessThanOrEqual(Length(expr), Literal(length)),
- expr,
- If(
- GreaterThan(Length(trimmed), Literal(length)),
- raiseError("varchar", length),
- StringRPad(trimmed, Literal(length))))
- } else {
- If(GreaterThan(Length(expr), Literal(length)), raiseError("varchar", length), expr)
- }
+ val trimmed = StringTrimRight(expr)
Review comment:
the `needTrim` is determined by the callers, one is read-side(!needTrim) and the other is write-side. Now the read-side logic never goes here so it is removed
----------------------------------------------------------------
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.
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] srowen commented on a change in pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
srowen commented on a change in pull request #31199:
URL: https://github.com/apache/spark/pull/31199#discussion_r559234630
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/CharVarcharUtils.scala
##########
@@ -200,69 +212,60 @@ object CharVarcharUtils extends Logging {
*/
def stringLengthCheck(expr: Expression, targetAttr: Attribute): Expression = {
getRawType(targetAttr.metadata).map { rawType =>
- stringLengthCheck(expr, rawType, needTrim = true)
+ stringLengthCheck(expr, rawType)
}.getOrElse(expr)
}
private def raiseError(typeName: String, length: Int): Expression = {
val errMsg = UTF8String.fromString(s"Exceeds $typeName type length limitation: $length")
- RaiseError(Literal(errMsg, StringType), StringType)
+ RaiseError(Literal(errMsg, StringType), StringType)
}
- private def stringLengthCheck(expr: Expression, dt: DataType, needTrim: Boolean): Expression = {
+ private def stringLengthCheck(expr: Expression, dt: DataType): Expression = {
dt match {
case CharType(length) =>
- val trimmed = if (needTrim) StringTrimRight(expr) else expr
+ val trimmed = StringTrimRight(expr)
// Trailing spaces do not count in the length check. We don't need to retain the trailing
// spaces, as we will pad char type columns/fields at read time.
- If(
- GreaterThan(Length(trimmed), Literal(length)),
- raiseError("char", length),
- trimmed)
+ If(GreaterThan(Length(trimmed), Literal(length)), raiseError("char", length), trimmed)
case VarcharType(length) =>
- if (needTrim) {
- val trimmed = StringTrimRight(expr)
- // Trailing spaces do not count in the length check. We need to retain the trailing spaces
- // (truncate to length N), as there is no read-time padding for varchar type.
- // TODO: create a special TrimRight function that can trim to a certain length.
- If(
- LessThanOrEqual(Length(expr), Literal(length)),
- expr,
- If(
- GreaterThan(Length(trimmed), Literal(length)),
- raiseError("varchar", length),
- StringRPad(trimmed, Literal(length))))
- } else {
- If(GreaterThan(Length(expr), Literal(length)), raiseError("varchar", length), expr)
- }
+ val trimmed = StringTrimRight(expr)
Review comment:
I'm sure you're right, but just for more info, why does that change? this sounds like an optimization and I wasn't clear why that would mean read-side doesn't trim anymore.
----------------------------------------------------------------
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.
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] SparkQA commented on pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31199:
URL: https://github.com/apache/spark/pull/31199#issuecomment-762337689
**[Test build #134194 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134194/testReport)** for PR 31199 at commit [`79c2ecd`](https://github.com/apache/spark/commit/79c2ecde846bb8eccaa7dedb3f9df80f45a92cda).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
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.
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] yaooqinn commented on a change in pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on a change in pull request #31199:
URL: https://github.com/apache/spark/pull/31199#discussion_r558799699
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/CharVarcharUtils.scala
##########
@@ -200,69 +212,60 @@ object CharVarcharUtils extends Logging {
*/
def stringLengthCheck(expr: Expression, targetAttr: Attribute): Expression = {
getRawType(targetAttr.metadata).map { rawType =>
- stringLengthCheck(expr, rawType, needTrim = true)
+ stringLengthCheck(expr, rawType)
}.getOrElse(expr)
}
private def raiseError(typeName: String, length: Int): Expression = {
val errMsg = UTF8String.fromString(s"Exceeds $typeName type length limitation: $length")
- RaiseError(Literal(errMsg, StringType), StringType)
+ RaiseError(Literal(errMsg, StringType), StringType)
}
- private def stringLengthCheck(expr: Expression, dt: DataType, needTrim: Boolean): Expression = {
+ private def stringLengthCheck(expr: Expression, dt: DataType): Expression = {
dt match {
case CharType(length) =>
- val trimmed = if (needTrim) StringTrimRight(expr) else expr
+ val trimmed = StringTrimRight(expr)
// Trailing spaces do not count in the length check. We don't need to retain the trailing
// spaces, as we will pad char type columns/fields at read time.
- If(
- GreaterThan(Length(trimmed), Literal(length)),
- raiseError("char", length),
- trimmed)
+ If(GreaterThan(Length(trimmed), Literal(length)), raiseError("char", length), trimmed)
case VarcharType(length) =>
- if (needTrim) {
- val trimmed = StringTrimRight(expr)
- // Trailing spaces do not count in the length check. We need to retain the trailing spaces
- // (truncate to length N), as there is no read-time padding for varchar type.
- // TODO: create a special TrimRight function that can trim to a certain length.
- If(
- LessThanOrEqual(Length(expr), Literal(length)),
- expr,
- If(
- GreaterThan(Length(trimmed), Literal(length)),
- raiseError("varchar", length),
- StringRPad(trimmed, Literal(length))))
- } else {
- If(GreaterThan(Length(expr), Literal(length)), raiseError("varchar", length), expr)
- }
+ val trimmed = StringTrimRight(expr)
Review comment:
the `needTrim` is determined by the caller, one is read-side(!needTrim) and the other is write-side. Now the read-side logic never goes here so it is removed
----------------------------------------------------------------
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.
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] AmplabJenkins commented on pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31199:
URL: https://github.com/apache/spark/pull/31199#issuecomment-761235948
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/134118/
----------------------------------------------------------------
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.
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] yaooqinn commented on pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on pull request #31199:
URL: https://github.com/apache/spark/pull/31199#issuecomment-761597572
cc @cloud-fan @maropu
----------------------------------------------------------------
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.
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] SparkQA commented on pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31199:
URL: https://github.com/apache/spark/pull/31199#issuecomment-762239484
**[Test build #134196 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134196/testReport)** for PR 31199 at commit [`f665d33`](https://github.com/apache/spark/commit/f665d33cea1091ac7af66580d36418dd27ff6e1c).
----------------------------------------------------------------
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.
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] AmplabJenkins commented on pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31199:
URL: https://github.com/apache/spark/pull/31199#issuecomment-762466978
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/134202/
----------------------------------------------------------------
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.
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 #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #31199:
URL: https://github.com/apache/spark/pull/31199#issuecomment-763398983
It's unrelated to CBO. It's about the generated code is too big and stops whole-stage-codegen.
----------------------------------------------------------------
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.
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] AmplabJenkins removed a comment on pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31199:
URL: https://github.com/apache/spark/pull/31199#issuecomment-762466978
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/134202/
----------------------------------------------------------------
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.
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 #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #31199:
URL: https://github.com/apache/spark/pull/31199#issuecomment-762632317
Ideally, we shouldn't backport perf improvement, but not passing `TPCDSQuerySuite` with char/varchar in table schema concerns me (see https://github.com/apache/spark/pull/31012). Since it only touches the char/varchar code path, I feel it's safe and better to have this "fix" in branch-3.1 as well. I won't treat it as a blocker though (won't fail rc1).
@HyukjinKwon what do you think?
----------------------------------------------------------------
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.
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] AmplabJenkins removed a comment on pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31199:
URL: https://github.com/apache/spark/pull/31199#issuecomment-761116125
----------------------------------------------------------------
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.
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] AmplabJenkins commented on pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31199:
URL: https://github.com/apache/spark/pull/31199#issuecomment-762447194
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/134199/
----------------------------------------------------------------
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.
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] AmplabJenkins commented on pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31199:
URL: https://github.com/apache/spark/pull/31199#issuecomment-761692375
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/134144/
----------------------------------------------------------------
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.
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] AmplabJenkins commented on pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31199:
URL: https://github.com/apache/spark/pull/31199#issuecomment-762057320
----------------------------------------------------------------
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.
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] AmplabJenkins commented on pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31199:
URL: https://github.com/apache/spark/pull/31199#issuecomment-761684385
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/134143/
----------------------------------------------------------------
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.
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] SparkQA commented on pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31199:
URL: https://github.com/apache/spark/pull/31199#issuecomment-761613006
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/38727/
----------------------------------------------------------------
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.
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] yaooqinn commented on a change in pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on a change in pull request #31199:
URL: https://github.com/apache/spark/pull/31199#discussion_r558799350
##########
File path: sql/catalyst/src/main/java/org/apache/spark/sql/catalyst/util/CharVarcharCodegenUtils.java
##########
@@ -0,0 +1,40 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.catalyst.util;
+
+import org.apache.spark.unsafe.types.UTF8String;
+
+public class CharVarcharCodegenUtils {
+ private static final UTF8String SPACE = UTF8String.fromString(" ");
+
+ public static UTF8String lengthCheck(UTF8String inputStr, int limit) {
+ if (inputStr == null) return null;
+ if (inputStr.numChars() > limit) {
Review comment:
Ok, maybe I can create a benchmark test to verify this
----------------------------------------------------------------
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.
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] yaooqinn commented on a change in pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on a change in pull request #31199:
URL: https://github.com/apache/spark/pull/31199#discussion_r559350759
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/CharVarcharUtils.scala
##########
@@ -200,69 +212,59 @@ object CharVarcharUtils extends Logging {
*/
def stringLengthCheck(expr: Expression, targetAttr: Attribute): Expression = {
getRawType(targetAttr.metadata).map { rawType =>
- stringLengthCheck(expr, rawType, needTrim = true)
+ stringLengthCheck(expr, rawType)
}.getOrElse(expr)
}
private def raiseError(typeName: String, length: Int): Expression = {
val errMsg = UTF8String.fromString(s"Exceeds $typeName type length limitation: $length")
- RaiseError(Literal(errMsg, StringType), StringType)
+ RaiseError(Literal(errMsg, StringType), StringType)
}
- private def stringLengthCheck(expr: Expression, dt: DataType, needTrim: Boolean): Expression = {
+ private def stringLengthCheck(expr: Expression, dt: DataType): Expression = {
dt match {
case CharType(length) =>
- val trimmed = if (needTrim) StringTrimRight(expr) else expr
- // Trailing spaces do not count in the length check. We don't need to retain the trailing
- // spaces, as we will pad char type columns/fields at read time.
- If(
- GreaterThan(Length(trimmed), Literal(length)),
- raiseError("char", length),
- trimmed)
+ StaticInvoke(
+ classOf[CharVarcharCodegenUtils],
+ StringType,
+ "charTypeWriteCheck",
+ expr :: Literal(length) :: Nil,
+ propagateNull = false)
case VarcharType(length) =>
- if (needTrim) {
- val trimmed = StringTrimRight(expr)
- // Trailing spaces do not count in the length check. We need to retain the trailing spaces
- // (truncate to length N), as there is no read-time padding for varchar type.
- // TODO: create a special TrimRight function that can trim to a certain length.
- If(
- LessThanOrEqual(Length(expr), Literal(length)),
- expr,
- If(
- GreaterThan(Length(trimmed), Literal(length)),
- raiseError("varchar", length),
- StringRPad(trimmed, Literal(length))))
- } else {
- If(GreaterThan(Length(expr), Literal(length)), raiseError("varchar", length), expr)
- }
+ StaticInvoke(
+ classOf[CharVarcharCodegenUtils],
+ StringType,
+ "varcharTypeWriteSidePadAndCheck",
Review comment:
OK
----------------------------------------------------------------
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.
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] srowen commented on a change in pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
srowen commented on a change in pull request #31199:
URL: https://github.com/apache/spark/pull/31199#discussion_r559306487
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/CharVarcharUtils.scala
##########
@@ -200,69 +212,60 @@ object CharVarcharUtils extends Logging {
*/
def stringLengthCheck(expr: Expression, targetAttr: Attribute): Expression = {
getRawType(targetAttr.metadata).map { rawType =>
- stringLengthCheck(expr, rawType, needTrim = true)
+ stringLengthCheck(expr, rawType)
}.getOrElse(expr)
}
private def raiseError(typeName: String, length: Int): Expression = {
val errMsg = UTF8String.fromString(s"Exceeds $typeName type length limitation: $length")
- RaiseError(Literal(errMsg, StringType), StringType)
+ RaiseError(Literal(errMsg, StringType), StringType)
}
- private def stringLengthCheck(expr: Expression, dt: DataType, needTrim: Boolean): Expression = {
+ private def stringLengthCheck(expr: Expression, dt: DataType): Expression = {
dt match {
case CharType(length) =>
- val trimmed = if (needTrim) StringTrimRight(expr) else expr
+ val trimmed = StringTrimRight(expr)
// Trailing spaces do not count in the length check. We don't need to retain the trailing
// spaces, as we will pad char type columns/fields at read time.
- If(
- GreaterThan(Length(trimmed), Literal(length)),
- raiseError("char", length),
- trimmed)
+ If(GreaterThan(Length(trimmed), Literal(length)), raiseError("char", length), trimmed)
case VarcharType(length) =>
- if (needTrim) {
- val trimmed = StringTrimRight(expr)
- // Trailing spaces do not count in the length check. We need to retain the trailing spaces
- // (truncate to length N), as there is no read-time padding for varchar type.
- // TODO: create a special TrimRight function that can trim to a certain length.
- If(
- LessThanOrEqual(Length(expr), Literal(length)),
- expr,
- If(
- GreaterThan(Length(trimmed), Literal(length)),
- raiseError("varchar", length),
- StringRPad(trimmed, Literal(length))))
- } else {
- If(GreaterThan(Length(expr), Literal(length)), raiseError("varchar", length), expr)
- }
+ val trimmed = StringTrimRight(expr)
Review comment:
OK, I don't question it, thanks.
----------------------------------------------------------------
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.
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] SparkQA commented on pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31199:
URL: https://github.com/apache/spark/pull/31199#issuecomment-762389476
**[Test build #134196 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134196/testReport)** for PR 31199 at commit [`f665d33`](https://github.com/apache/spark/commit/f665d33cea1091ac7af66580d36418dd27ff6e1c).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
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.
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] SparkQA commented on pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31199:
URL: https://github.com/apache/spark/pull/31199#issuecomment-761228747
**[Test build #134118 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134118/testReport)** for PR 31199 at commit [`0caf4ed`](https://github.com/apache/spark/commit/0caf4ed6105612485f1695552f0ae6a1f27decb5).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
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.
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] SparkQA commented on pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31199:
URL: https://github.com/apache/spark/pull/31199#issuecomment-762359185
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/38787/
----------------------------------------------------------------
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.
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] SparkQA commented on pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31199:
URL: https://github.com/apache/spark/pull/31199#issuecomment-762454155
**[Test build #134202 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134202/testReport)** for PR 31199 at commit [`3f81d69`](https://github.com/apache/spark/commit/3f81d694be4817c7aebe1df7b168b22fa58fef60).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
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.
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] AmplabJenkins commented on pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31199:
URL: https://github.com/apache/spark/pull/31199#issuecomment-762338889
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/134194/
----------------------------------------------------------------
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.
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] SparkQA commented on pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31199:
URL: https://github.com/apache/spark/pull/31199#issuecomment-762306413
**[Test build #134199 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134199/testReport)** for PR 31199 at commit [`1e19de4`](https://github.com/apache/spark/commit/1e19de44a803cf822f5d07bee82aaf982761a96b).
----------------------------------------------------------------
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.
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 a change in pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31199:
URL: https://github.com/apache/spark/pull/31199#discussion_r559347494
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/CharVarcharUtils.scala
##########
@@ -200,69 +212,59 @@ object CharVarcharUtils extends Logging {
*/
def stringLengthCheck(expr: Expression, targetAttr: Attribute): Expression = {
getRawType(targetAttr.metadata).map { rawType =>
- stringLengthCheck(expr, rawType, needTrim = true)
+ stringLengthCheck(expr, rawType)
}.getOrElse(expr)
}
private def raiseError(typeName: String, length: Int): Expression = {
val errMsg = UTF8String.fromString(s"Exceeds $typeName type length limitation: $length")
- RaiseError(Literal(errMsg, StringType), StringType)
+ RaiseError(Literal(errMsg, StringType), StringType)
Review comment:
nit: identation is wrong
----------------------------------------------------------------
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.
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 a change in pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31199:
URL: https://github.com/apache/spark/pull/31199#discussion_r559347883
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/CharVarcharUtils.scala
##########
@@ -200,69 +212,59 @@ object CharVarcharUtils extends Logging {
*/
def stringLengthCheck(expr: Expression, targetAttr: Attribute): Expression = {
getRawType(targetAttr.metadata).map { rawType =>
- stringLengthCheck(expr, rawType, needTrim = true)
+ stringLengthCheck(expr, rawType)
}.getOrElse(expr)
}
private def raiseError(typeName: String, length: Int): Expression = {
val errMsg = UTF8String.fromString(s"Exceeds $typeName type length limitation: $length")
- RaiseError(Literal(errMsg, StringType), StringType)
+ RaiseError(Literal(errMsg, StringType), StringType)
}
- private def stringLengthCheck(expr: Expression, dt: DataType, needTrim: Boolean): Expression = {
+ private def stringLengthCheck(expr: Expression, dt: DataType): Expression = {
dt match {
case CharType(length) =>
- val trimmed = if (needTrim) StringTrimRight(expr) else expr
- // Trailing spaces do not count in the length check. We don't need to retain the trailing
- // spaces, as we will pad char type columns/fields at read time.
- If(
- GreaterThan(Length(trimmed), Literal(length)),
- raiseError("char", length),
- trimmed)
+ StaticInvoke(
+ classOf[CharVarcharCodegenUtils],
+ StringType,
+ "charTypeWriteCheck",
+ expr :: Literal(length) :: Nil,
+ propagateNull = false)
case VarcharType(length) =>
- if (needTrim) {
- val trimmed = StringTrimRight(expr)
- // Trailing spaces do not count in the length check. We need to retain the trailing spaces
- // (truncate to length N), as there is no read-time padding for varchar type.
- // TODO: create a special TrimRight function that can trim to a certain length.
- If(
- LessThanOrEqual(Length(expr), Literal(length)),
- expr,
- If(
- GreaterThan(Length(trimmed), Literal(length)),
- raiseError("varchar", length),
- StringRPad(trimmed, Literal(length))))
- } else {
- If(GreaterThan(Length(expr), Literal(length)), raiseError("varchar", length), expr)
- }
+ StaticInvoke(
+ classOf[CharVarcharCodegenUtils],
+ StringType,
+ "varcharTypeWriteSidePadAndCheck",
Review comment:
maybe just `varcharTypeWriteSideCheck` to show the general purpose. It also does trim but we didn't mention it in the name either.
----------------------------------------------------------------
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.
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] yaooqinn commented on pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on pull request #31199:
URL: https://github.com/apache/spark/pull/31199#issuecomment-762638710
+1 for @cloud-fan 's decision. As far as I'm concerned, query 41 from TPCDS is a quite normal case, 3.1 generates oversized codegen classes that let it fall back to the non-codegen mode and cause huge performance regession. Users can hit this regularly in real-world cases if they work on their pre-defined tables with char/varchar
----------------------------------------------------------------
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.
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] SparkQA removed a comment on pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #31199:
URL: https://github.com/apache/spark/pull/31199#issuecomment-762173675
**[Test build #134194 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134194/testReport)** for PR 31199 at commit [`79c2ecd`](https://github.com/apache/spark/commit/79c2ecde846bb8eccaa7dedb3f9df80f45a92cda).
----------------------------------------------------------------
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.
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] SparkQA commented on pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31199:
URL: https://github.com/apache/spark/pull/31199#issuecomment-761113412
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/38701/
----------------------------------------------------------------
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.
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] SparkQA removed a comment on pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #31199:
URL: https://github.com/apache/spark/pull/31199#issuecomment-762028581
**[Test build #134185 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134185/testReport)** for PR 31199 at commit [`3d82a79`](https://github.com/apache/spark/commit/3d82a799c66410bed77cd637ec28c01f4275f015).
----------------------------------------------------------------
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.
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] yaooqinn commented on a change in pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on a change in pull request #31199:
URL: https://github.com/apache/spark/pull/31199#discussion_r559286164
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/CharVarcharUtils.scala
##########
@@ -200,69 +212,60 @@ object CharVarcharUtils extends Logging {
*/
def stringLengthCheck(expr: Expression, targetAttr: Attribute): Expression = {
getRawType(targetAttr.metadata).map { rawType =>
- stringLengthCheck(expr, rawType, needTrim = true)
+ stringLengthCheck(expr, rawType)
}.getOrElse(expr)
}
private def raiseError(typeName: String, length: Int): Expression = {
val errMsg = UTF8String.fromString(s"Exceeds $typeName type length limitation: $length")
- RaiseError(Literal(errMsg, StringType), StringType)
+ RaiseError(Literal(errMsg, StringType), StringType)
}
- private def stringLengthCheck(expr: Expression, dt: DataType, needTrim: Boolean): Expression = {
+ private def stringLengthCheck(expr: Expression, dt: DataType): Expression = {
dt match {
case CharType(length) =>
- val trimmed = if (needTrim) StringTrimRight(expr) else expr
+ val trimmed = StringTrimRight(expr)
// Trailing spaces do not count in the length check. We don't need to retain the trailing
// spaces, as we will pad char type columns/fields at read time.
- If(
- GreaterThan(Length(trimmed), Literal(length)),
- raiseError("char", length),
- trimmed)
+ If(GreaterThan(Length(trimmed), Literal(length)), raiseError("char", length), trimmed)
case VarcharType(length) =>
- if (needTrim) {
- val trimmed = StringTrimRight(expr)
- // Trailing spaces do not count in the length check. We need to retain the trailing spaces
- // (truncate to length N), as there is no read-time padding for varchar type.
- // TODO: create a special TrimRight function that can trim to a certain length.
- If(
- LessThanOrEqual(Length(expr), Literal(length)),
- expr,
- If(
- GreaterThan(Length(trimmed), Literal(length)),
- raiseError("varchar", length),
- StringRPad(trimmed, Literal(length))))
- } else {
- If(GreaterThan(Length(expr), Literal(length)), raiseError("varchar", length), expr)
- }
+ val trimmed = StringTrimRight(expr)
Review comment:
> I wasn't clear why that would mean read-side doesn't trim anymore.
Looks like we have a misunderstanding here, we are not removing trim logic from the read-side in this PR, just the flag itself. The trim logic was removed in the previous PR https://github.com/apache/spark/pull/31181 as a bugfix.
----------------------------------------------------------------
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.
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] SparkQA removed a comment on pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #31199:
URL: https://github.com/apache/spark/pull/31199#issuecomment-761605815
**[Test build #134144 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134144/testReport)** for PR 31199 at commit [`54f30ee`](https://github.com/apache/spark/commit/54f30ee9443cc263581b58c3fc419d17d3158c65).
----------------------------------------------------------------
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.
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] AmplabJenkins removed a comment on pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31199:
URL: https://github.com/apache/spark/pull/31199#issuecomment-762300201
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/38781/
----------------------------------------------------------------
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.
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] AmplabJenkins commented on pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31199:
URL: https://github.com/apache/spark/pull/31199#issuecomment-761160836
----------------------------------------------------------------
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.
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] yaooqinn edited a comment on pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
yaooqinn edited a comment on pull request #31199:
URL: https://github.com/apache/spark/pull/31199#issuecomment-761732525
# Codegen vs NonCodegen reslut from master
the performance itself does not change much, because the purpose here is to prevent queries from fallback to non-codegen
```logtalk
================================================================================================
Char Varchar Read Side Perf
================================================================================================
Java HotSpot(TM) 64-Bit Server VM 1.8.0_251-b08 on Mac OS X 10.16
Intel(R) Core(TM) i9-9980HK CPU @ 2.40GHz
read char with length 20: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative
------------------------------------------------------------------------------------------------------------------------
read char with length 20 wholestage off 4830 5393 796 20.7 48.3 1.0X
read char with length 20 wholestage on 1693 1755 42 59.1 16.9 2.9X
Java HotSpot(TM) 64-Bit Server VM 1.8.0_251-b08 on Mac OS X 10.16
Intel(R) Core(TM) i9-9980HK CPU @ 2.40GHz
read varchar with length 20: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative
--------------------------------------------------------------------------------------------------------------------------
read varchar with length 20 wholestage off 4763 4884 171 21.0 47.6 1.0X
read varchar with length 20 wholestage on 1802 1888 59 55.5 18.0 2.6X
Java HotSpot(TM) 64-Bit Server VM 1.8.0_251-b08 on Mac OS X 10.16
Intel(R) Core(TM) i9-9980HK CPU @ 2.40GHz
read char with length 40: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative
------------------------------------------------------------------------------------------------------------------------
read char with length 40 wholestage off 4685 4718 46 21.3 46.9 1.0X
read char with length 40 wholestage on 1858 1904 39 53.8 18.6 2.5X
Java HotSpot(TM) 64-Bit Server VM 1.8.0_251-b08 on Mac OS X 10.16
Intel(R) Core(TM) i9-9980HK CPU @ 2.40GHz
read varchar with length 40: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative
--------------------------------------------------------------------------------------------------------------------------
read varchar with length 40 wholestage off 4730 4749 27 21.1 47.3 1.0X
read varchar with length 40 wholestage on 1738 1831 99 57.5 17.4 2.7X
Java HotSpot(TM) 64-Bit Server VM 1.8.0_251-b08 on Mac OS X 10.16
Intel(R) Core(TM) i9-9980HK CPU @ 2.40GHz
read char with length 60: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative
------------------------------------------------------------------------------------------------------------------------
read char with length 60 wholestage off 4936 4984 68 20.3 49.4 1.0X
read char with length 60 wholestage on 1787 1835 46 55.9 17.9 2.8X
Java HotSpot(TM) 64-Bit Server VM 1.8.0_251-b08 on Mac OS X 10.16
Intel(R) Core(TM) i9-9980HK CPU @ 2.40GHz
read varchar with length 60: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative
--------------------------------------------------------------------------------------------------------------------------
read varchar with length 60 wholestage off 4642 4740 138 21.5 46.4 1.0X
read varchar with length 60 wholestage on 1694 1755 48 59.0 16.9 2.7X
Java HotSpot(TM) 64-Bit Server VM 1.8.0_251-b08 on Mac OS X 10.16
Intel(R) Core(TM) i9-9980HK CPU @ 2.40GHz
read char with length 80: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative
------------------------------------------------------------------------------------------------------------------------
read char with length 80 wholestage off 4709 4736 38 21.2 47.1 1.0X
read char with length 80 wholestage on 1787 1869 76 56.0 17.9 2.6X
Java HotSpot(TM) 64-Bit Server VM 1.8.0_251-b08 on Mac OS X 10.16
Intel(R) Core(TM) i9-9980HK CPU @ 2.40GHz
read varchar with length 80: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative
--------------------------------------------------------------------------------------------------------------------------
read varchar with length 80 wholestage off 4613 4721 153 21.7 46.1 1.0X
read varchar with length 80 wholestage on 1757 1814 59 56.9 17.6 2.6X
================================================================================================
Char Varchar Write Side Perf
================================================================================================
Java HotSpot(TM) 64-Bit Server VM 1.8.0_251-b08 on Mac OS X 10.16
Intel(R) Core(TM) i9-9980HK CPU @ 2.40GHz
write char with length 20: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative
------------------------------------------------------------------------------------------------------------------------
write char with length 20 wholestage off 7312 7439 180 1.4 731.2 1.0X
write char with length 20 wholestage on 5957 6332 355 1.7 595.7 1.2X
Java HotSpot(TM) 64-Bit Server VM 1.8.0_251-b08 on Mac OS X 10.16
Intel(R) Core(TM) i9-9980HK CPU @ 2.40GHz
write varchar with length 20: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative
---------------------------------------------------------------------------------------------------------------------------
write varchar with length 20 wholestage off 9318 10265 1339 1.1 931.8 1.0X
write varchar with length 20 wholestage on 8722 9268 512 1.1 872.2 1.1X
Java HotSpot(TM) 64-Bit Server VM 1.8.0_251-b08 on Mac OS X 10.16
Intel(R) Core(TM) i9-9980HK CPU @ 2.40GHz
write char with length 40: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative
------------------------------------------------------------------------------------------------------------------------
write char with length 40 wholestage off 10051 10257 292 1.0 1005.1 1.0X
write char with length 40 wholestage on 8534 8917 316 1.2 853.4 1.2X
Java HotSpot(TM) 64-Bit Server VM 1.8.0_251-b08 on Mac OS X 10.16
Intel(R) Core(TM) i9-9980HK CPU @ 2.40GHz
write varchar with length 40: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative
---------------------------------------------------------------------------------------------------------------------------
write varchar with length 40 wholestage off 13713 14250 759 0.7 1371.3 1.0X
write varchar with length 40 wholestage on 12070 12609 516 0.8 1207.0 1.1X
Java HotSpot(TM) 64-Bit Server VM 1.8.0_251-b08 on Mac OS X 10.16
Intel(R) Core(TM) i9-9980HK CPU @ 2.40GHz
write char with length 60: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative
------------------------------------------------------------------------------------------------------------------------
write char with length 60 wholestage off 11728 12975 1763 0.9 1172.8 1.0X
write char with length 60 wholestage on 10211 10515 256 1.0 1021.1 1.1X
Java HotSpot(TM) 64-Bit Server VM 1.8.0_251-b08 on Mac OS X 10.16
Intel(R) Core(TM) i9-9980HK CPU @ 2.40GHz
write varchar with length 60: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative
---------------------------------------------------------------------------------------------------------------------------
write varchar with length 60 wholestage off 17845 17924 113 0.6 1784.5 1.0X
write varchar with length 60 wholestage on 16338 17353 1330 0.6 1633.8 1.1X
Java HotSpot(TM) 64-Bit Server VM 1.8.0_251-b08 on Mac OS X 10.16
Intel(R) Core(TM) i9-9980HK CPU @ 2.40GHz
write char with length 80: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative
------------------------------------------------------------------------------------------------------------------------
write char with length 80 wholestage off 14381 14656 390 0.7 1438.1 1.0X
write char with length 80 wholestage on 13991 14556 455 0.7 1399.1 1.0X
Java HotSpot(TM) 64-Bit Server VM 1.8.0_251-b08 on Mac OS X 10.16
Intel(R) Core(TM) i9-9980HK CPU @ 2.40GHz
write varchar with length 80: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative
---------------------------------------------------------------------------------------------------------------------------
write varchar with length 80 wholestage off 21476 22547 1515 0.5 2147.6 1.0X
write varchar with length 80 wholestage on 21092 22393 899 0.5 2109.2 1.0X
```
----------------------------------------------------------------
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.
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] SparkQA commented on pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31199:
URL: https://github.com/apache/spark/pull/31199#issuecomment-762173675
**[Test build #134194 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134194/testReport)** for PR 31199 at commit [`79c2ecd`](https://github.com/apache/spark/commit/79c2ecde846bb8eccaa7dedb3f9df80f45a92cda).
----------------------------------------------------------------
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.
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] AmplabJenkins commented on pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31199:
URL: https://github.com/apache/spark/pull/31199#issuecomment-762232998
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/38779/
----------------------------------------------------------------
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.
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] AmplabJenkins removed a comment on pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31199:
URL: https://github.com/apache/spark/pull/31199#issuecomment-762338889
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/134194/
----------------------------------------------------------------
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.
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] SparkQA commented on pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31199:
URL: https://github.com/apache/spark/pull/31199#issuecomment-761605815
**[Test build #134144 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134144/testReport)** for PR 31199 at commit [`54f30ee`](https://github.com/apache/spark/commit/54f30ee9443cc263581b58c3fc419d17d3158c65).
----------------------------------------------------------------
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.
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] AmplabJenkins removed a comment on pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31199:
URL: https://github.com/apache/spark/pull/31199#issuecomment-761235948
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/134118/
----------------------------------------------------------------
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.
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] dongjoon-hyun commented on pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #31199:
URL: https://github.com/apache/spark/pull/31199#issuecomment-763367862
So, does it happen only with CBO and new CHAR/VARCHAR combination, @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.
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] SparkQA commented on pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31199:
URL: https://github.com/apache/spark/pull/31199#issuecomment-762337113
**[Test build #134202 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134202/testReport)** for PR 31199 at commit [`3f81d69`](https://github.com/apache/spark/commit/3f81d694be4817c7aebe1df7b168b22fa58fef60).
----------------------------------------------------------------
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.
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] SparkQA commented on pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31199:
URL: https://github.com/apache/spark/pull/31199#issuecomment-761617023
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/38727/
----------------------------------------------------------------
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.
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] SparkQA commented on pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31199:
URL: https://github.com/apache/spark/pull/31199#issuecomment-762352240
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/38784/
----------------------------------------------------------------
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.
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] viirya commented on a change in pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #31199:
URL: https://github.com/apache/spark/pull/31199#discussion_r558595669
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/CharVarcharUtils.scala
##########
@@ -161,9 +162,20 @@ object CharVarcharUtils extends Logging {
private def paddingWithLengthCheck(expr: Expression, dt: DataType): Expression = dt match {
case CharType(length) =>
- StringRPad(stringLengthCheck(expr, dt, needTrim = false), Literal(length))
-
- case VarcharType(_) => stringLengthCheck(expr, dt, needTrim = false)
+ StaticInvoke(
Review comment:
`StringRPad`'s gen-ed code is pretty short, as `s"$str.rpad($len, $pad)"`. Does `StaticInvoke` help reduce gen-ed code?
----------------------------------------------------------------
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.
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] SparkQA removed a comment on pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #31199:
URL: https://github.com/apache/spark/pull/31199#issuecomment-761084962
**[Test build #134118 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134118/testReport)** for PR 31199 at commit [`0caf4ed`](https://github.com/apache/spark/commit/0caf4ed6105612485f1695552f0ae6a1f27decb5).
----------------------------------------------------------------
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.
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] SparkQA commented on pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31199:
URL: https://github.com/apache/spark/pull/31199#issuecomment-761689574
**[Test build #134144 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134144/testReport)** for PR 31199 at commit [`54f30ee`](https://github.com/apache/spark/commit/54f30ee9443cc263581b58c3fc419d17d3158c65).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
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.
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] AmplabJenkins removed a comment on pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31199:
URL: https://github.com/apache/spark/pull/31199#issuecomment-762031207
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/134185/
----------------------------------------------------------------
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.
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] SparkQA commented on pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31199:
URL: https://github.com/apache/spark/pull/31199#issuecomment-762446482
**[Test build #134199 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134199/testReport)** for PR 31199 at commit [`1e19de4`](https://github.com/apache/spark/commit/1e19de44a803cf822f5d07bee82aaf982761a96b).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
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.
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] yaooqinn commented on pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on pull request #31199:
URL: https://github.com/apache/spark/pull/31199#issuecomment-761732525
# reslut from master
the performance itself does not change much, because the purpose here is to prevent queries from fallback to non-codegen
```logtalk
================================================================================================
Char Varchar Read Side Perf
================================================================================================
Java HotSpot(TM) 64-Bit Server VM 1.8.0_251-b08 on Mac OS X 10.16
Intel(R) Core(TM) i9-9980HK CPU @ 2.40GHz
read char with length 20: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative
------------------------------------------------------------------------------------------------------------------------
read char with length 20 wholestage off 4830 5393 796 20.7 48.3 1.0X
read char with length 20 wholestage on 1693 1755 42 59.1 16.9 2.9X
Java HotSpot(TM) 64-Bit Server VM 1.8.0_251-b08 on Mac OS X 10.16
Intel(R) Core(TM) i9-9980HK CPU @ 2.40GHz
read varchar with length 20: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative
--------------------------------------------------------------------------------------------------------------------------
read varchar with length 20 wholestage off 4763 4884 171 21.0 47.6 1.0X
read varchar with length 20 wholestage on 1802 1888 59 55.5 18.0 2.6X
Java HotSpot(TM) 64-Bit Server VM 1.8.0_251-b08 on Mac OS X 10.16
Intel(R) Core(TM) i9-9980HK CPU @ 2.40GHz
read char with length 40: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative
------------------------------------------------------------------------------------------------------------------------
read char with length 40 wholestage off 4685 4718 46 21.3 46.9 1.0X
read char with length 40 wholestage on 1858 1904 39 53.8 18.6 2.5X
Java HotSpot(TM) 64-Bit Server VM 1.8.0_251-b08 on Mac OS X 10.16
Intel(R) Core(TM) i9-9980HK CPU @ 2.40GHz
read varchar with length 40: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative
--------------------------------------------------------------------------------------------------------------------------
read varchar with length 40 wholestage off 4730 4749 27 21.1 47.3 1.0X
read varchar with length 40 wholestage on 1738 1831 99 57.5 17.4 2.7X
Java HotSpot(TM) 64-Bit Server VM 1.8.0_251-b08 on Mac OS X 10.16
Intel(R) Core(TM) i9-9980HK CPU @ 2.40GHz
read char with length 60: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative
------------------------------------------------------------------------------------------------------------------------
read char with length 60 wholestage off 4936 4984 68 20.3 49.4 1.0X
read char with length 60 wholestage on 1787 1835 46 55.9 17.9 2.8X
Java HotSpot(TM) 64-Bit Server VM 1.8.0_251-b08 on Mac OS X 10.16
Intel(R) Core(TM) i9-9980HK CPU @ 2.40GHz
read varchar with length 60: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative
--------------------------------------------------------------------------------------------------------------------------
read varchar with length 60 wholestage off 4642 4740 138 21.5 46.4 1.0X
read varchar with length 60 wholestage on 1694 1755 48 59.0 16.9 2.7X
Java HotSpot(TM) 64-Bit Server VM 1.8.0_251-b08 on Mac OS X 10.16
Intel(R) Core(TM) i9-9980HK CPU @ 2.40GHz
read char with length 80: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative
------------------------------------------------------------------------------------------------------------------------
read char with length 80 wholestage off 4709 4736 38 21.2 47.1 1.0X
read char with length 80 wholestage on 1787 1869 76 56.0 17.9 2.6X
Java HotSpot(TM) 64-Bit Server VM 1.8.0_251-b08 on Mac OS X 10.16
Intel(R) Core(TM) i9-9980HK CPU @ 2.40GHz
read varchar with length 80: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative
--------------------------------------------------------------------------------------------------------------------------
read varchar with length 80 wholestage off 4613 4721 153 21.7 46.1 1.0X
read varchar with length 80 wholestage on 1757 1814 59 56.9 17.6 2.6X
================================================================================================
Char Varchar Write Side Perf
================================================================================================
Java HotSpot(TM) 64-Bit Server VM 1.8.0_251-b08 on Mac OS X 10.16
Intel(R) Core(TM) i9-9980HK CPU @ 2.40GHz
write char with length 20: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative
------------------------------------------------------------------------------------------------------------------------
write char with length 20 wholestage off 7312 7439 180 1.4 731.2 1.0X
write char with length 20 wholestage on 5957 6332 355 1.7 595.7 1.2X
Java HotSpot(TM) 64-Bit Server VM 1.8.0_251-b08 on Mac OS X 10.16
Intel(R) Core(TM) i9-9980HK CPU @ 2.40GHz
write varchar with length 20: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative
---------------------------------------------------------------------------------------------------------------------------
write varchar with length 20 wholestage off 9318 10265 1339 1.1 931.8 1.0X
write varchar with length 20 wholestage on 8722 9268 512 1.1 872.2 1.1X
Java HotSpot(TM) 64-Bit Server VM 1.8.0_251-b08 on Mac OS X 10.16
Intel(R) Core(TM) i9-9980HK CPU @ 2.40GHz
write char with length 40: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative
------------------------------------------------------------------------------------------------------------------------
write char with length 40 wholestage off 10051 10257 292 1.0 1005.1 1.0X
write char with length 40 wholestage on 8534 8917 316 1.2 853.4 1.2X
Java HotSpot(TM) 64-Bit Server VM 1.8.0_251-b08 on Mac OS X 10.16
Intel(R) Core(TM) i9-9980HK CPU @ 2.40GHz
write varchar with length 40: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative
---------------------------------------------------------------------------------------------------------------------------
write varchar with length 40 wholestage off 13713 14250 759 0.7 1371.3 1.0X
write varchar with length 40 wholestage on 12070 12609 516 0.8 1207.0 1.1X
Java HotSpot(TM) 64-Bit Server VM 1.8.0_251-b08 on Mac OS X 10.16
Intel(R) Core(TM) i9-9980HK CPU @ 2.40GHz
write char with length 60: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative
------------------------------------------------------------------------------------------------------------------------
write char with length 60 wholestage off 11728 12975 1763 0.9 1172.8 1.0X
write char with length 60 wholestage on 10211 10515 256 1.0 1021.1 1.1X
Java HotSpot(TM) 64-Bit Server VM 1.8.0_251-b08 on Mac OS X 10.16
Intel(R) Core(TM) i9-9980HK CPU @ 2.40GHz
write varchar with length 60: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative
---------------------------------------------------------------------------------------------------------------------------
write varchar with length 60 wholestage off 17845 17924 113 0.6 1784.5 1.0X
write varchar with length 60 wholestage on 16338 17353 1330 0.6 1633.8 1.1X
Java HotSpot(TM) 64-Bit Server VM 1.8.0_251-b08 on Mac OS X 10.16
Intel(R) Core(TM) i9-9980HK CPU @ 2.40GHz
write char with length 80: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative
------------------------------------------------------------------------------------------------------------------------
write char with length 80 wholestage off 14381 14656 390 0.7 1438.1 1.0X
write char with length 80 wholestage on 13991 14556 455 0.7 1399.1 1.0X
Java HotSpot(TM) 64-Bit Server VM 1.8.0_251-b08 on Mac OS X 10.16
Intel(R) Core(TM) i9-9980HK CPU @ 2.40GHz
write varchar with length 80: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative
---------------------------------------------------------------------------------------------------------------------------
write varchar with length 80 wholestage off 21476 22547 1515 0.5 2147.6 1.0X
write varchar with length 80 wholestage on 21092 22393 899 0.5 2109.2 1.0X
```
----------------------------------------------------------------
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.
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 a change in pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31199:
URL: https://github.com/apache/spark/pull/31199#discussion_r559345830
##########
File path: sql/catalyst/src/main/java/org/apache/spark/sql/catalyst/util/CharVarcharCodegenUtils.java
##########
@@ -0,0 +1,72 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.catalyst.util;
+
+import org.apache.spark.unsafe.types.UTF8String;
+
+public class CharVarcharCodegenUtils {
+ private static final UTF8String SPACE = UTF8String.fromString(" ");
+
+ public static UTF8String varcharTypeReadSideCheck(UTF8String inputStr, int limit) {
+ if (inputStr != null && inputStr.numChars() > limit) {
+ throw new RuntimeException("Exceeds varchar type length limitation: " + limit);
+ }
+ return inputStr;
+ }
+
+ /**
+ * Trailing spaces do not count in the length check. We don't need to retain the trailing
+ * spaces, as we will pad char type columns/fields at read time.
+ */
+ public static UTF8String charTypeWriteCheck(UTF8String inputStr, int limit) {
Review comment:
`charTypeWriteSideCheck` to be consistent.
----------------------------------------------------------------
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.
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] SparkQA removed a comment on pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #31199:
URL: https://github.com/apache/spark/pull/31199#issuecomment-762337113
**[Test build #134202 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134202/testReport)** for PR 31199 at commit [`3f81d69`](https://github.com/apache/spark/commit/3f81d694be4817c7aebe1df7b168b22fa58fef60).
----------------------------------------------------------------
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.
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] AmplabJenkins removed a comment on pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31199:
URL: https://github.com/apache/spark/pull/31199#issuecomment-761622266
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/38727/
----------------------------------------------------------------
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.
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] srowen commented on a change in pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
srowen commented on a change in pull request #31199:
URL: https://github.com/apache/spark/pull/31199#discussion_r558582512
##########
File path: sql/catalyst/src/main/java/org/apache/spark/sql/catalyst/util/CharVarcharCodegenUtils.java
##########
@@ -0,0 +1,40 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.catalyst.util;
+
+import org.apache.spark.unsafe.types.UTF8String;
+
+public class CharVarcharCodegenUtils {
+ private static final UTF8String SPACE = UTF8String.fromString(" ");
+
+ public static UTF8String lengthCheck(UTF8String inputStr, int limit) {
+ if (inputStr == null) return null;
+ if (inputStr.numChars() > limit) {
Review comment:
Not sure if it helps code size or speed, but you can skip the if/return above and add "inputStr != null &&" here for the same result.
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/CharVarcharUtils.scala
##########
@@ -200,69 +212,60 @@ object CharVarcharUtils extends Logging {
*/
def stringLengthCheck(expr: Expression, targetAttr: Attribute): Expression = {
getRawType(targetAttr.metadata).map { rawType =>
- stringLengthCheck(expr, rawType, needTrim = true)
+ stringLengthCheck(expr, rawType)
}.getOrElse(expr)
}
private def raiseError(typeName: String, length: Int): Expression = {
val errMsg = UTF8String.fromString(s"Exceeds $typeName type length limitation: $length")
- RaiseError(Literal(errMsg, StringType), StringType)
+ RaiseError(Literal(errMsg, StringType), StringType)
}
- private def stringLengthCheck(expr: Expression, dt: DataType, needTrim: Boolean): Expression = {
+ private def stringLengthCheck(expr: Expression, dt: DataType): Expression = {
dt match {
case CharType(length) =>
- val trimmed = if (needTrim) StringTrimRight(expr) else expr
+ val trimmed = StringTrimRight(expr)
// Trailing spaces do not count in the length check. We don't need to retain the trailing
// spaces, as we will pad char type columns/fields at read time.
- If(
- GreaterThan(Length(trimmed), Literal(length)),
- raiseError("char", length),
- trimmed)
+ If(GreaterThan(Length(trimmed), Literal(length)), raiseError("char", length), trimmed)
case VarcharType(length) =>
- if (needTrim) {
- val trimmed = StringTrimRight(expr)
- // Trailing spaces do not count in the length check. We need to retain the trailing spaces
- // (truncate to length N), as there is no read-time padding for varchar type.
- // TODO: create a special TrimRight function that can trim to a certain length.
- If(
- LessThanOrEqual(Length(expr), Literal(length)),
- expr,
- If(
- GreaterThan(Length(trimmed), Literal(length)),
- raiseError("varchar", length),
- StringRPad(trimmed, Literal(length))))
- } else {
- If(GreaterThan(Length(expr), Literal(length)), raiseError("varchar", length), expr)
- }
+ val trimmed = StringTrimRight(expr)
Review comment:
Why does the needTrim check go away here?
----------------------------------------------------------------
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.
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] yaooqinn commented on a change in pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on a change in pull request #31199:
URL: https://github.com/apache/spark/pull/31199#discussion_r559350675
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/CharVarcharUtils.scala
##########
@@ -200,69 +212,59 @@ object CharVarcharUtils extends Logging {
*/
def stringLengthCheck(expr: Expression, targetAttr: Attribute): Expression = {
getRawType(targetAttr.metadata).map { rawType =>
- stringLengthCheck(expr, rawType, needTrim = true)
+ stringLengthCheck(expr, rawType)
}.getOrElse(expr)
}
private def raiseError(typeName: String, length: Int): Expression = {
val errMsg = UTF8String.fromString(s"Exceeds $typeName type length limitation: $length")
- RaiseError(Literal(errMsg, StringType), StringType)
+ RaiseError(Literal(errMsg, StringType), StringType)
Review comment:
removed it
----------------------------------------------------------------
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.
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] AmplabJenkins removed a comment on pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31199:
URL: https://github.com/apache/spark/pull/31199#issuecomment-762057320
----------------------------------------------------------------
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.
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] AmplabJenkins removed a comment on pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31199:
URL: https://github.com/apache/spark/pull/31199#issuecomment-762395833
----------------------------------------------------------------
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.
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] SparkQA commented on pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31199:
URL: https://github.com/apache/spark/pull/31199#issuecomment-762332411
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/38784/
----------------------------------------------------------------
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.
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] AmplabJenkins removed a comment on pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31199:
URL: https://github.com/apache/spark/pull/31199#issuecomment-762232998
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/38779/
----------------------------------------------------------------
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.
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] AmplabJenkins removed a comment on pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31199:
URL: https://github.com/apache/spark/pull/31199#issuecomment-762367847
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/38784/
----------------------------------------------------------------
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.
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] yaooqinn commented on a change in pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on a change in pull request #31199:
URL: https://github.com/apache/spark/pull/31199#discussion_r558799511
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/CharVarcharUtils.scala
##########
@@ -161,9 +162,20 @@ object CharVarcharUtils extends Logging {
private def paddingWithLengthCheck(expr: Expression, dt: DataType): Expression = dt match {
case CharType(length) =>
- StringRPad(stringLengthCheck(expr, dt, needTrim = false), Literal(length))
-
- case VarcharType(_) => stringLengthCheck(expr, dt, needTrim = false)
+ StaticInvoke(
Review comment:
It not just a single rad but,
StringRPad(If(GreaterThan(Length(StringTrimRight(expr)), Literal(length)), raiseError("char", length),StringTrimRight(expr))))
----------------------------------------------------------------
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.
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] SparkQA commented on pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31199:
URL: https://github.com/apache/spark/pull/31199#issuecomment-761084962
**[Test build #134118 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134118/testReport)** for PR 31199 at commit [`0caf4ed`](https://github.com/apache/spark/commit/0caf4ed6105612485f1695552f0ae6a1f27decb5).
----------------------------------------------------------------
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.
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] AmplabJenkins removed a comment on pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31199:
URL: https://github.com/apache/spark/pull/31199#issuecomment-761692375
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/134144/
----------------------------------------------------------------
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.
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] AmplabJenkins commented on pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31199:
URL: https://github.com/apache/spark/pull/31199#issuecomment-762367847
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/38784/
----------------------------------------------------------------
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.
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 closed pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
cloud-fan closed pull request #31199:
URL: https://github.com/apache/spark/pull/31199
----------------------------------------------------------------
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.
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] yaooqinn commented on pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on pull request #31199:
URL: https://github.com/apache/spark/pull/31199#issuecomment-762706758
thanks for merging
----------------------------------------------------------------
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.
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] SparkQA removed a comment on pull request #31199: [SPARK-34130][SQL] Impove preformace for char varchar padding and length check with StaticInvoke
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #31199:
URL: https://github.com/apache/spark/pull/31199#issuecomment-762239484
**[Test build #134196 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134196/testReport)** for PR 31199 at commit [`f665d33`](https://github.com/apache/spark/commit/f665d33cea1091ac7af66580d36418dd27ff6e1c).
----------------------------------------------------------------
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.
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