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 2020/11/18 16:26:46 UTC
[GitHub] [spark] cloud-fan opened a new pull request #30412: [SPARK-33480][SQL] support char/varchar type
cloud-fan opened a new pull request #30412:
URL: https://github.com/apache/spark/pull/30412
<!--
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 PR adds the char/varchar type which is kind of a variant of string type:
1. Char type is fixed-length string. When comparing char type values, we need to pad the shorter one to the longer length.
2. Varchar type is string with a length limitation.
To implement the char/varchar semantic, this PR:
1. Do string length check when writing to char/varchar type columns.
2. Do string padding when reading char type columns. We don't do it at the writing side to save storage space.
3. Do string padding when comparing char type column with string literal or another char type column. (string literal is fixed length so should be treated as char type as well)
To simplify the implementation, this PR doesn't propagate char/varchar type info through functions/operators(e.g. `substring`). That said, a column can only be char/varchar type if it's a table column, not a derived column like `SELECT substring(col)`.
To be safe, this PR doesn't add char/varchar type to the core type system. We will replace char/varchar type by string type with metadata (`Attribute.metadata` or `StructField.metadata`) that includes the original type string before it goes into the query engine. That said, the existing code will not see char/varchar type but only string type.
char/varchar type may come from several places:
1. v1 table from hive catalog.
2. v2 table from v2 catalog.
3. user-specified schema in `spark.read.schema` and `spark.readStream.schema`
4. `Column.cast`
This PR covers all the above cases, implements the length check and padding feature by looking at string type with special metadata.
### 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.
-->
char and varchar are standard SQL types. varchar is widely used in other databases instead of string type.
### 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'.
-->
For hive tables: now the table insertion fails if the value exceeds char/varchar length. Previously we truncate the value silently.
For other tables:
1. now char type is allowed.
2. now we have length check when inserting to varchar columns. Previously we write the value as it is.
### 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.
-->
new tests
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-730545928
**[Test build #131365 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131365/testReport)** for PR 30412 at commit [`c239834`](https://github.com/apache/spark/commit/c239834ef7ce730910d2d3a64f858c2c53dbc6a5).
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-732853983
**[Test build #131654 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131654/testReport)** for PR 30412 at commit [`e5fb41a`](https://github.com/apache/spark/commit/e5fb41aa2aa220b0750c6d6812e34075a9c118b2).
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-732320855
**[Test build #131561 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131561/testReport)** for PR 30412 at commit [`b21157d`](https://github.com/apache/spark/commit/b21157db8596342a406dd14832ae18e9632f21fb).
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-733510491
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-733811335
**[Test build #131786 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131786/testReport)** for PR 30412 at commit [`8789494`](https://github.com/apache/spark/commit/8789494b87a17d5371b61731bbcee2386caf9074).
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-732676986
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-733089649
**[Test build #131674 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131674/testReport)** for PR 30412 at commit [`38999b5`](https://github.com/apache/spark/commit/38999b535e78817d2647d186605618438f438220).
* This patch **fails Spark unit 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] dongjoon-hyun commented on a change in pull request #30412: [SPARK-33480][SQL] support char/varchar type
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #30412:
URL: https://github.com/apache/spark/pull/30412#discussion_r526249805
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/types/CharType.scala
##########
@@ -0,0 +1,36 @@
+/*
+ * 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.types
+
+import scala.math.Ordering
+import scala.reflect.runtime.universe.typeTag
+
+import org.apache.spark.annotation.Experimental
+import org.apache.spark.unsafe.types.UTF8String
+
+@Experimental
+case class CharType(length: Int) extends AtomicType {
Review comment:
Shall we add `SPARK-6412 (Add Char support in dataTypes)` into the PR title as a secondary JIRA?
- SPARK-6412 was resolved as `Later`.
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-732367902
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-733510497
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-734858011
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-732360181
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/36163/
Test FAILed.
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-732844437
retest this please
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-733941137
**[Test build #131786 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131786/testReport)** for PR 30412 at commit [`8789494`](https://github.com/apache/spark/commit/8789494b87a17d5371b61731bbcee2386caf9074).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `case class SerdeInfo(`
* `case class FormatClasses(input: String, output: String) `
* `case class ShowColumns(`
* `case class TruncateTable(`
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] support char/varchar type
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-729817585
cc @wangyum and @MaxGekk , too.
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] support char/varchar type
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-729962424
**[Test build #131291 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131291/testReport)** for PR 30412 at commit [`a7f0730`](https://github.com/apache/spark/commit/a7f0730a48850c237313e9471fe63cbf2eef9a95).
* 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 #30412: [SPARK-33480][SQL] support char/varchar type
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-729844978
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-732991018
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-730588001
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/35969/
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30412:
URL: https://github.com/apache/spark/pull/30412#discussion_r531406576
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/Column.scala
##########
@@ -1181,7 +1181,9 @@ class Column(val expr: Expression) extends Logging {
* @group expr_ops
* @since 1.3.0
*/
- def cast(to: DataType): Column = withExpr { Cast(expr, to) }
+ def cast(to: DataType): Column = withExpr {
+ Cast(expr, CharVarcharUtils.replaceCharVarcharWithString(to))
+ }
Review comment:
If you do `col.cast("char(5)")` before this PR, Spark already silently treats it as cast to string type. I don't want to change this behavior here. We can make it better later, by failing explicitly and saying that cast to char type is not supported.
----------------------------------------------------------------
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 a change in pull request #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
maropu commented on a change in pull request #30412:
URL: https://github.com/apache/spark/pull/30412#discussion_r527403578
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/CharVarcharTestSuite.scala
##########
@@ -0,0 +1,374 @@
+/*
+ * 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
+
+import org.apache.spark.{SparkConf, SparkException}
+import org.apache.spark.sql.connector.InMemoryTableCatalog
+import org.apache.spark.sql.internal.SQLConf
+import org.apache.spark.sql.test.{SharedSparkSession, SQLTestUtils}
+
+trait CharVarcharTestSuite extends QueryTest with SQLTestUtils {
+
+ def format: String
+
+ test("char type values should be padded: top-level columns") {
+ withTable("t") {
+ sql(s"CREATE TABLE t(i STRING, c CHAR(5)) USING $format")
+ sql("INSERT INTO t VALUES ('1', 'a')")
+ checkAnswer(spark.table("t"), Row("1", "a" + " " * 4))
Review comment:
How about checking an output schema, too, in these tests?
```
scala> sql("CREATE TABLE t(i STRING, c CHAR(5)) USING parquet PARTITIONED BY (c)")
scala> spark.table("t").printSchema
root
|-- i: string (nullable = true)
|-- c: string (nullable = true) <---- this check
```
btw, how do users check a char length after defining a table? In pg, users can check a char length via some commands, e.g., `\d`;
```
postgres=# create table t (c char(5));
CREATE TABLE
postgres=# \d t
Table "public.t"
Column | Type | Collation | Nullable | Default
--------+--------------+-----------+----------+---------
c | character(5) | | |
```
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CheckAnalysis.scala
##########
@@ -94,6 +94,10 @@ trait CheckAnalysis extends PredicateHelper {
case p if p.analyzed => // Skip already analyzed sub-plans
+ case p if p.output.map(_.dataType).exists(CharVarcharUtils.hasCharVarchar) =>
+ throw new IllegalStateException(
+ "[BUG] logical plan should not have output of char/varchar type: " + p)
Review comment:
In the case below, could we use `AnalysisException` instead?
```
scala> sql("""SELECT from_json("{'a': 'aaa'}", "a char(3)")""").printSchema()
java.lang.IllegalStateException: [BUG] logical plan should not have output of char/varchar type: Project [from_json(StructField(a,CharType(3),true), {'a': 'aaa'}, Some(Asia/Tokyo)) AS from_json({'a': 'aaa'})#37]
+- OneRowRelation
```
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/CharVarcharUtils.scala
##########
@@ -0,0 +1,277 @@
+/*
+ * 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 scala.collection.mutable
+
+import org.apache.spark.sql.catalyst.expressions._
+import org.apache.spark.sql.catalyst.parser.CatalystSqlParser
+import org.apache.spark.sql.types._
+
+object CharVarcharUtils {
+
+ private val CHAR_VARCHAR_TYPE_STRING_METADATA_KEY = "__CHAR_VARCHAR_TYPE_STRING"
+
+ /**
+ * Replaces CharType/VarcharType with StringType recursively in the given struct type. If a
+ * top-level StructField's data type is CharType/VarcharType or has nested CharType/VarcharType,
+ * this method will add the original type string to the StructField's metadata, so that we can
+ * re-construct the original data type with CharType/VarcharType later when needed.
+ */
+ def replaceCharVarcharWithStringInSchema(st: StructType): StructType = {
+ StructType(st.map { field =>
+ if (hasCharVarchar(field.dataType)) {
+ val metadata = new MetadataBuilder().withMetadata(field.metadata)
+ .putString(CHAR_VARCHAR_TYPE_STRING_METADATA_KEY, field.dataType.sql).build()
+ field.copy(dataType = replaceCharVarcharWithString(field.dataType), metadata = metadata)
+ } else {
+ field
+ }
+ })
+ }
+
+ /**
+ * Returns true if the given data type is CharType/VarcharType or has nested CharType/VarcharType.
+ */
+ def hasCharVarchar(dt: DataType): Boolean = {
+ dt.existsRecursively(f => f.isInstanceOf[CharType] || f.isInstanceOf[VarcharType])
+ }
+
+ /**
+ * Replaces CharType/VarcharType with StringType recursively in the given data type.
+ */
+ def replaceCharVarcharWithString(dt: DataType): DataType = dt match {
+ case ArrayType(et, nullable) =>
+ ArrayType(replaceCharVarcharWithString(et), nullable)
+ case MapType(kt, vt, nullable) =>
+ MapType(replaceCharVarcharWithString(kt), replaceCharVarcharWithString(vt), nullable)
+ case StructType(fields) =>
+ StructType(fields.map { field =>
+ field.copy(dataType = replaceCharVarcharWithString(field.dataType))
+ })
+ case _: CharType => StringType
+ case _: VarcharType => StringType
+ case _ => dt
+ }
+
+ /**
+ * Removes the metadata entry that contains the original type string of CharType/VarcharType from
+ * the given attribute's metadata.
+ */
+ def cleanAttrMetadata(attr: AttributeReference): AttributeReference = {
+ val cleaned = new MetadataBuilder().withMetadata(attr.metadata)
+ .remove(CHAR_VARCHAR_TYPE_STRING_METADATA_KEY).build()
+ attr.withMetadata(cleaned)
+ }
+
+ /**
+ * Re-construct the original data type from the type string in the given metadata.
+ * This is needed when dealing with char/varchar columns/fields.
+ */
+ def getRawType(metadata: Metadata): Option[DataType] = {
+ if (metadata.contains(CHAR_VARCHAR_TYPE_STRING_METADATA_KEY)) {
+ Some(CatalystSqlParser.parseRawDataType(
+ metadata.getString(CHAR_VARCHAR_TYPE_STRING_METADATA_KEY)))
+ } else {
+ None
+ }
+ }
+
+ /**
+ * Re-construct the original StructType from the type strings in the metadata of StructFields.
+ * This is needed when dealing with char/varchar columns/fields.
+ */
+ def getRawSchema(schema: StructType): StructType = {
Review comment:
not used now?
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/CharVarcharTestSuite.scala
##########
@@ -0,0 +1,374 @@
+/*
+ * 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
+
+import org.apache.spark.{SparkConf, SparkException}
+import org.apache.spark.sql.connector.InMemoryTableCatalog
+import org.apache.spark.sql.internal.SQLConf
+import org.apache.spark.sql.test.{SharedSparkSession, SQLTestUtils}
+
+trait CharVarcharTestSuite extends QueryTest with SQLTestUtils {
+
+ def format: String
+
+ test("char type values should be padded: top-level columns") {
+ withTable("t") {
+ sql(s"CREATE TABLE t(i STRING, c CHAR(5)) USING $format")
+ sql("INSERT INTO t VALUES ('1', 'a')")
+ checkAnswer(spark.table("t"), Row("1", "a" + " " * 4))
+ }
+ }
+
+ test("char type values should be padded: partitioned columns") {
+ // DS V2 doesn't support partitioned table.
+ if (!conf.contains(SQLConf.DEFAULT_CATALOG.key)) {
+ withTable("t") {
+ sql(s"CREATE TABLE t(i STRING, c CHAR(5)) USING $format PARTITIONED BY (c)")
+ sql("INSERT INTO t VALUES ('1', 'a')")
+ checkAnswer(spark.table("t"), Row("1", "a" + " " * 4))
+ }
+ }
+ }
+
+ test("char type values should be padded: nested in struct") {
+ withTable("t") {
+ sql(s"CREATE TABLE t(i STRING, c STRUCT<c: CHAR(5)>) USING $format")
+ sql("INSERT INTO t VALUES ('1', struct('a'))")
+ checkAnswer(spark.table("t"), Row("1", Row("a" + " " * 4)))
+ }
+ }
+
+ test("char type values should be padded: nested in array") {
+ withTable("t") {
+ sql(s"CREATE TABLE t(i STRING, c ARRAY<CHAR(5)>) USING $format")
+ sql("INSERT INTO t VALUES ('1', array('a', 'ab'))")
+ checkAnswer(spark.table("t"), Row("1", Seq("a" + " " * 4, "ab" + " " * 3)))
+ }
+ }
+
+ test("char type values should be padded: nested in map key") {
+ withTable("t") {
+ sql(s"CREATE TABLE t(i STRING, c MAP<CHAR(5), STRING>) USING $format")
+ sql("INSERT INTO t VALUES ('1', map('a', 'ab'))")
+ checkAnswer(spark.table("t"), Row("1", Map(("a" + " " * 4, "ab"))))
+ }
+ }
+
+ test("char type values should be padded: nested in map value") {
+ withTable("t") {
+ sql(s"CREATE TABLE t(i STRING, c MAP<STRING, CHAR(5)>) USING $format")
+ sql("INSERT INTO t VALUES ('1', map('a', 'ab'))")
+ checkAnswer(spark.table("t"), Row("1", Map(("a", "ab" + " " * 3))))
+ }
+ }
+
+ test("char type values should be padded: nested in both map key and value") {
+ withTable("t") {
+ sql(s"CREATE TABLE t(i STRING, c MAP<CHAR(5), CHAR(10)>) USING $format")
+ sql("INSERT INTO t VALUES ('1', map('a', 'ab'))")
+ checkAnswer(spark.table("t"), Row("1", Map(("a" + " " * 4, "ab" + " " * 8))))
+ }
+ }
+
+ test("char type values should be padded: nested in struct of array") {
+ withTable("t") {
+ sql(s"CREATE TABLE t(i STRING, c STRUCT<c: ARRAY<CHAR(5)>>) USING $format")
+ sql("INSERT INTO t VALUES ('1', struct(array('a', 'ab')))")
+ checkAnswer(spark.table("t"), Row("1", Row(Seq("a" + " " * 4, "ab" + " " * 3))))
+ }
+ }
+
+ test("char type values should be padded: nested in array of struct") {
+ withTable("t") {
+ sql(s"CREATE TABLE t(i STRING, c ARRAY<STRUCT<c: CHAR(5)>>) USING $format")
+ sql("INSERT INTO t VALUES ('1', array(struct('a'), struct('ab')))")
+ checkAnswer(spark.table("t"), Row("1", Seq(Row("a" + " " * 4), Row("ab" + " " * 3))))
+ }
+ }
+
+ test("char type values should be padded: nested in array of array") {
+ withTable("t") {
+ sql(s"CREATE TABLE t(i STRING, c ARRAY<ARRAY<CHAR(5)>>) USING $format")
+ sql("INSERT INTO t VALUES ('1', array(array('a', 'ab')))")
+ checkAnswer(spark.table("t"), Row("1", Seq(Seq("a" + " " * 4, "ab" + " " * 3))))
+ }
+ }
+
+ private def testTableWrite(f: String => Unit): Unit = {
+ withTable("t") { f("char") }
+ withTable("t") { f("varchar") }
+ }
+
+ test("length check for input string values: top-level columns") {
+ testTableWrite { typeName =>
+ sql(s"CREATE TABLE t(c $typeName(5)) USING $format")
+ sql("INSERT INTO t VALUES (null)")
+ checkAnswer(spark.table("t"), Row(null))
+ val e = intercept[SparkException](sql("INSERT INTO t VALUES ('123456')"))
+ assert(e.getCause.getMessage.contains(
+ s"input string '123456' exceeds $typeName type length limitation: 5"))
+ }
+ }
+
+ test("length check for input string values: partitioned columns") {
+ // DS V2 doesn't support partitioned table.
+ if (!conf.contains(SQLConf.DEFAULT_CATALOG.key)) {
+ testTableWrite { typeName =>
+ sql(s"CREATE TABLE t(i INT, c $typeName(5)) USING $format PARTITIONED BY (c)")
+ sql("INSERT INTO t VALUES (1, null)")
+ checkAnswer(spark.table("t"), Row(1, null))
+ val e = intercept[SparkException](sql("INSERT INTO t VALUES (1, '123456')"))
+ assert(e.getCause.getMessage.contains(
+ s"input string '123456' exceeds $typeName type length limitation: 5"))
+ }
+ }
+ }
+
+ test("length check for input string values: nested in struct") {
+ testTableWrite { typeName =>
+ sql(s"CREATE TABLE t(c STRUCT<c: $typeName(5)>) USING $format")
+ sql("INSERT INTO t SELECT struct(null)")
+ checkAnswer(spark.table("t"), Row(Row(null)))
+ val e = intercept[SparkException](sql("INSERT INTO t SELECT struct('123456')"))
+ assert(e.getCause.getMessage.contains(
+ s"input string '123456' exceeds $typeName type length limitation: 5"))
+ }
+ }
+
+ test("length check for input string values: nested in array") {
+ testTableWrite { typeName =>
+ sql(s"CREATE TABLE t(c ARRAY<$typeName(5)>) USING $format")
+ sql("INSERT INTO t VALUES (array(null))")
+ checkAnswer(spark.table("t"), Row(Seq(null)))
+ val e = intercept[SparkException](sql("INSERT INTO t VALUES (array('a', '123456'))"))
+ assert(e.getCause.getMessage.contains(
+ s"input string '123456' exceeds $typeName type length limitation: 5"))
+ }
+ }
+
+ test("length check for input string values: nested in map key") {
+ testTableWrite { typeName =>
+ sql(s"CREATE TABLE t(c MAP<$typeName(5), STRING>) USING $format")
+ val e = intercept[SparkException](sql("INSERT INTO t VALUES (map('123456', 'a'))"))
+ assert(e.getCause.getMessage.contains(
+ s"input string '123456' exceeds $typeName type length limitation: 5"))
+ }
+ }
+
+ test("length check for input string values: nested in map value") {
+ testTableWrite { typeName =>
+ sql(s"CREATE TABLE t(c MAP<STRING, $typeName(5)>) USING $format")
+ sql("INSERT INTO t VALUES (map('a', null))")
+ checkAnswer(spark.table("t"), Row(Map("a" -> null)))
+ val e = intercept[SparkException](sql("INSERT INTO t VALUES (map('a', '123456'))"))
+ assert(e.getCause.getMessage.contains(
+ s"input string '123456' exceeds $typeName type length limitation: 5"))
+ }
+ }
+
+ test("length check for input string values: nested in both map key and value") {
+ testTableWrite { typeName =>
+ sql(s"CREATE TABLE t(c MAP<$typeName(5), $typeName(5)>) USING $format")
+ val e1 = intercept[SparkException](sql("INSERT INTO t VALUES (map('123456', 'a'))"))
+ assert(e1.getCause.getMessage.contains(
+ s"input string '123456' exceeds $typeName type length limitation: 5"))
+ val e2 = intercept[SparkException](sql("INSERT INTO t VALUES (map('a', '123456'))"))
+ assert(e2.getCause.getMessage.contains(
+ s"input string '123456' exceeds $typeName type length limitation: 5"))
+ }
+ }
+
+ test("length check for input string values: nested in struct of array") {
+ testTableWrite { typeName =>
+ sql(s"CREATE TABLE t(c STRUCT<c: ARRAY<$typeName(5)>>) USING $format")
+ sql("INSERT INTO t SELECT struct(array(null))")
+ checkAnswer(spark.table("t"), Row(Row(Seq(null))))
+ val e = intercept[SparkException](sql("INSERT INTO t SELECT struct(array('123456'))"))
+ assert(e.getCause.getMessage.contains(
+ s"input string '123456' exceeds $typeName type length limitation: 5"))
+ }
+ }
+
+ test("length check for input string values: nested in array of struct") {
+ testTableWrite { typeName =>
+ sql(s"CREATE TABLE t(c ARRAY<STRUCT<c: $typeName(5)>>) USING $format")
+ sql("INSERT INTO t VALUES (array(struct(null)))")
+ checkAnswer(spark.table("t"), Row(Seq(Row(null))))
+ val e = intercept[SparkException](sql("INSERT INTO t VALUES (array(struct('123456')))"))
+ assert(e.getCause.getMessage.contains(
+ s"input string '123456' exceeds $typeName type length limitation: 5"))
+ }
+ }
+
+ test("length check for input string values: nested in array of array") {
+ testTableWrite { typeName =>
+ sql(s"CREATE TABLE t(c ARRAY<ARRAY<$typeName(5)>>) USING $format")
+ sql("INSERT INTO t VALUES (array(array(null)))")
+ checkAnswer(spark.table("t"), Row(Seq(Seq(null))))
+ val e = intercept[SparkException](sql("INSERT INTO t VALUES (array(array('123456')))"))
+ assert(e.getCause.getMessage.contains(
+ s"input string '123456' exceeds $typeName type length limitation: 5"))
+ }
+ }
+
+ test("length check for input string values: with trailing spaces") {
+ withTable("t") {
+ sql(s"CREATE TABLE t(c1 CHAR(5), c2 VARCHAR(5)) USING $format")
+ sql("INSERT INTO t VALUES ('12 ', '12 ')")
+ sql("INSERT INTO t VALUES ('1234 ', '1234 ')")
+ checkAnswer(spark.table("t"), Seq(
+ Row("12" + " " * 3, "12 "),
+ Row("1234 ", "1234 ")))
+ }
+ }
+
+ test("length check for input string values: with implicit cast") {
+ withTable("t") {
+ sql(s"CREATE TABLE t(c1 CHAR(5), c2 VARCHAR(5)) USING $format")
+ sql("INSERT INTO t VALUES (1234, 1234)")
+ checkAnswer(spark.table("t"), Row("1234 ", "1234"))
+ val e1 = intercept[SparkException](sql("INSERT INTO t VALUES (123456, 1)"))
+ assert(e1.getCause.getMessage.contains(
+ "input string '123456' exceeds char type length limitation: 5"))
+ val e2 = intercept[SparkException](sql("INSERT INTO t VALUES (1, 123456)"))
+ assert(e2.getCause.getMessage.contains(
+ "input string '123456' exceeds varchar type length limitation: 5"))
+ }
+ }
+
+ private def testConditions(df: DataFrame, conditions: Seq[(String, Boolean)]): Unit = {
+ checkAnswer(df.selectExpr(conditions.map(_._1): _*), Row.fromSeq(conditions.map(_._2)))
+ }
+
+ test("char type comparison: top-level columns") {
+ withTable("t") {
+ sql(s"CREATE TABLE t(c1 CHAR(2), c2 CHAR(5)) USING $format")
+ sql("INSERT INTO t VALUES ('a', 'a')")
+ testConditions(spark.table("t"), Seq(
+ ("c1 = 'a'", true),
+ ("'a' = c1", true),
+ ("c1 = 'a '", true),
+ ("c1 > 'a'", false),
+ ("c1 IN ('a', 'b')", true),
+ ("c1 = c2", true),
+ ("c1 < c2", false),
+ ("c1 IN (c2)", true)))
+ }
+ }
+
+ test("char type comparison: partitioned columns") {
+ withTable("t") {
+ sql(s"CREATE TABLE t(i INT, c1 CHAR(2), c2 CHAR(5)) USING $format PARTITIONED BY (c1, c2)")
+ sql("INSERT INTO t VALUES (1, 'a', 'a')")
+ testConditions(spark.table("t"), Seq(
+ ("c1 = 'a'", true),
+ ("'a' = c1", true),
+ ("c1 = 'a '", true),
+ ("c1 > 'a'", false),
+ ("c1 IN ('a', 'b')", true),
+ ("c1 = c2", true),
+ ("c1 < c2", false),
+ ("c1 IN (c2)", true)))
+ }
+ }
+
+ test("char type comparison: join") {
+ withTable("t1", "t2") {
+ sql(s"CREATE TABLE t1(c CHAR(2)) USING $format")
+ sql(s"CREATE TABLE t2(c CHAR(5)) USING $format")
+ sql("INSERT INTO t1 VALUES ('a')")
+ sql("INSERT INTO t2 VALUES ('a')")
+ checkAnswer(sql("SELECT t1.c FROM t1 JOIN t2 ON t1.c = t2.c"), Row("a "))
+ }
+ }
+
+ test("char type comparison: nested in struct") {
+ withTable("t") {
+ sql(s"CREATE TABLE t(c1 STRUCT<c: CHAR(2)>, c2 STRUCT<c: CHAR(5)>) USING $format")
+ sql("INSERT INTO t VALUES (struct('a'), struct('a'))")
+ testConditions(spark.table("t"), Seq(
+ ("c1 = c2", true),
+ ("c1 < c2", false),
+ ("c1 IN (c2)", true)))
+ }
+ }
+
+ test("char type comparison: nested in array") {
+ withTable("t") {
+ sql(s"CREATE TABLE t(c1 ARRAY<CHAR(2)>, c2 ARRAY<CHAR(5)>) USING $format")
+ sql("INSERT INTO t VALUES (array('a', 'b'), array('a', 'b'))")
+ testConditions(spark.table("t"), Seq(
+ ("c1 = c2", true),
+ ("c1 < c2", false),
+ ("c1 IN (c2)", true)))
+ }
+ }
+
+ test("char type comparison: nested in struct of array") {
+ withTable("t") {
+ sql("CREATE TABLE t(c1 STRUCT<a: ARRAY<CHAR(2)>>, c2 STRUCT<a: ARRAY<CHAR(5)>>) " +
+ s"USING $format")
+ sql("INSERT INTO t VALUES (struct(array('a', 'b')), struct(array('a', 'b')))")
+ testConditions(spark.table("t"), Seq(
+ ("c1 = c2", true),
+ ("c1 < c2", false),
+ ("c1 IN (c2)", true)))
+ }
+ }
+
+ test("char type comparison: nested in array of struct") {
+ withTable("t") {
+ sql("CREATE TABLE t(c1 ARRAY<STRUCT<c: CHAR(2)>>, c2 ARRAY<STRUCT<c: CHAR(5)>>) " +
+ s"USING $format")
+ sql("INSERT INTO t VALUES (array(struct('a')), array(struct('a')))")
+ testConditions(spark.table("t"), Seq(
+ ("c1 = c2", true),
+ ("c1 < c2", false),
+ ("c1 IN (c2)", true)))
+ }
+ }
+
+ test("char type comparison: nested in array of array") {
Review comment:
Could you add char comparison tests for a map case, too?
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-733983504
**[Test build #131797 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131797/testReport)** for PR 30412 at commit [`662f297`](https://github.com/apache/spark/commit/662f2970cc4a87051d1eee67f884ba1ab945885b).
* This patch **fails SparkR unit 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] cloud-fan commented on a change in pull request #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30412:
URL: https://github.com/apache/spark/pull/30412#discussion_r528884694
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/CharVarcharTestSuite.scala
##########
@@ -0,0 +1,374 @@
+/*
+ * 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
+
+import org.apache.spark.{SparkConf, SparkException}
+import org.apache.spark.sql.connector.InMemoryTableCatalog
+import org.apache.spark.sql.internal.SQLConf
+import org.apache.spark.sql.test.{SharedSparkSession, SQLTestUtils}
+
+trait CharVarcharTestSuite extends QueryTest with SQLTestUtils {
+
+ def format: String
+
+ test("char type values should be padded: top-level columns") {
+ withTable("t") {
+ sql(s"CREATE TABLE t(i STRING, c CHAR(5)) USING $format")
+ sql("INSERT INTO t VALUES ('1', 'a')")
+ checkAnswer(spark.table("t"), Row("1", "a" + " " * 4))
Review comment:
We ensure that `LocalPlan.output` won't have char/varchar type and we have UT to verify it. It seems not necessary to check it again here.
For DDL command support, can we leave it to followup? This PR is already very big. I can create a ticket for 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 commented on pull request #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-732372664
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-734823439
**[Test build #131881 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131881/testReport)** for PR 30412 at commit [`73b99dc`](https://github.com/apache/spark/commit/73b99dc7a98f5d0673d309adba457cd19144be92).
* This patch **fails Spark unit 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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-733922316
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-732367902
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-734171079
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-734829598
**[Test build #131884 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131884/testReport)** for PR 30412 at commit [`73b99dc`](https://github.com/apache/spark/commit/73b99dc7a98f5d0673d309adba457cd19144be92).
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-734731027
**[Test build #131876 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131876/testReport)** for PR 30412 at commit [`3bbe7e7`](https://github.com/apache/spark/commit/3bbe7e784b0544dbd05fbe113ade387d080ba990).
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-733034804
**[Test build #131674 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131674/testReport)** for PR 30412 at commit [`38999b5`](https://github.com/apache/spark/commit/38999b535e78817d2647d186605618438f438220).
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-730418080
**[Test build #131356 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131356/testReport)** for PR 30412 at commit [`17d13e5`](https://github.com/apache/spark/commit/17d13e5944e247826a92403abb134a9dfd5e0cc6).
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-734683690
**[Test build #131866 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131866/testReport)** for PR 30412 at commit [`69adca5`](https://github.com/apache/spark/commit/69adca50f12fde7deefc7bef2e31d459526cac3c).
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-733870653
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-730418080
**[Test build #131356 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131356/testReport)** for PR 30412 at commit [`17d13e5`](https://github.com/apache/spark/commit/17d13e5944e247826a92403abb134a9dfd5e0cc6).
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-732449764
**[Test build #131561 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131561/testReport)** for PR 30412 at commit [`b21157d`](https://github.com/apache/spark/commit/b21157db8596342a406dd14832ae18e9632f21fb).
* This patch **fails Spark unit 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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-733966628
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] support char/varchar type
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-729798687
cc @dongjoon-hyun @viirya @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] AmplabJenkins commented on pull request #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-732360171
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-734704291
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-734826864
retest this please
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-730588014
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] support char/varchar type
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-729795850
**[Test build #131291 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131291/testReport)** for PR 30412 at commit [`a7f0730`](https://github.com/apache/spark/commit/a7f0730a48850c237313e9471fe63cbf2eef9a95).
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-732846865
**[Test build #131653 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131653/testReport)** for PR 30412 at commit [`f46e32f`](https://github.com/apache/spark/commit/f46e32fb1649023eed0ddab4cb23ca4a97b14a0f).
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-732504623
**[Test build #131597 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131597/testReport)** for PR 30412 at commit [`6182921`](https://github.com/apache/spark/commit/6182921657ae0757549d2544088ff4442d8a08a7).
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-734930279
**[Test build #131884 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131884/testReport)** for PR 30412 at commit [`73b99dc`](https://github.com/apache/spark/commit/73b99dc7a98f5d0673d309adba457cd19144be92).
* 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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-732360171
Merged build finished. Test FAILed.
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-735663488
thanks for the review, merging to master!
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-732320855
**[Test build #131561 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131561/testReport)** for PR 30412 at commit [`b21157d`](https://github.com/apache/spark/commit/b21157db8596342a406dd14832ae18e9632f21fb).
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-732583439
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-732450395
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-733984067
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-732756465
**[Test build #131637 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131637/testReport)** for PR 30412 at commit [`f46e32f`](https://github.com/apache/spark/commit/f46e32fb1649023eed0ddab4cb23ca4a97b14a0f).
----------------------------------------------------------------
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 a change in pull request #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
maropu commented on a change in pull request #30412:
URL: https://github.com/apache/spark/pull/30412#discussion_r529072022
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/CharVarcharTestSuite.scala
##########
@@ -0,0 +1,374 @@
+/*
+ * 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
+
+import org.apache.spark.{SparkConf, SparkException}
+import org.apache.spark.sql.connector.InMemoryTableCatalog
+import org.apache.spark.sql.internal.SQLConf
+import org.apache.spark.sql.test.{SharedSparkSession, SQLTestUtils}
+
+trait CharVarcharTestSuite extends QueryTest with SQLTestUtils {
+
+ def format: String
+
+ test("char type values should be padded: top-level columns") {
+ withTable("t") {
+ sql(s"CREATE TABLE t(i STRING, c CHAR(5)) USING $format")
+ sql("INSERT INTO t VALUES ('1', 'a')")
+ checkAnswer(spark.table("t"), Row("1", "a" + " " * 4))
Review comment:
The followup looks fine.
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-733324371
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-733945195
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] support char/varchar type
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30412:
URL: https://github.com/apache/spark/pull/30412#discussion_r526241812
##########
File path: sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala
##########
@@ -1000,18 +995,10 @@ private[hive] object HiveClientImpl extends Logging {
/** Builds the native StructField from Hive's FieldSchema. */
def fromHiveColumn(hc: FieldSchema): StructField = {
val columnType = getSparkSQLDataType(hc)
- val replacedVoidType = HiveVoidType.replaceVoidType(columnType)
Review comment:
This comes from https://github.com/apache/spark/commit/339eec5f321fac07e01441a3fd83024f4b92a0d6#diff-45c9b065d76b237bcfecda83b8ee08c1ff6592d6f85acca09c0fa01472e056afR1009 but is actually useless. The test still 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 removed a comment on pull request #30412: [SPARK-33480][SQL] support char/varchar type
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-729964068
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-730493450
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] support char/varchar type
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30412:
URL: https://github.com/apache/spark/pull/30412#discussion_r526227775
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala
##########
@@ -3088,7 +3088,12 @@ class Analyzer(override val catalogManager: CatalogManager)
val projection = TableOutputResolver.resolveOutputColumns(
v2Write.table.name, v2Write.table.output, v2Write.query, v2Write.isByName, conf)
if (projection != v2Write.query) {
- v2Write.withNewQuery(projection)
+ val cleanedTable = v2Write.table match {
+ case r: DataSourceV2Relation =>
+ r.copy(output = r.output.map(CharVarcharUtils.cleanAttrMetadata))
Review comment:
We remove the char/varchar metadata after length check expressions are added, so that we don't do it repeatedly and this rule is idempotent.
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-732991018
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-732583439
----------------------------------------------------------------
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 a change in pull request #30412: [SPARK-33480][SQL] support char/varchar type
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #30412:
URL: https://github.com/apache/spark/pull/30412#discussion_r526253162
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/types/CharType.scala
##########
@@ -0,0 +1,36 @@
+/*
+ * 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.types
+
+import scala.math.Ordering
+import scala.reflect.runtime.universe.typeTag
+
+import org.apache.spark.annotation.Experimental
+import org.apache.spark.unsafe.types.UTF8String
+
+@Experimental
+case class CharType(length: Int) extends AtomicType {
Review comment:
The PR description says `To be safe, this PR doesn't add char/varchar type to the core type system.`, but this adds `CharType` into `org.apache.spark.sql.types`. Do you revise a little the PR description? It seems that you wanted to mention the SQL execution parts.
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-733110220
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-732372213
**[Test build #131564 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131564/testReport)** for PR 30412 at commit [`6182921`](https://github.com/apache/spark/commit/6182921657ae0757549d2544088ff4442d8a08a7).
* This patch **fails Spark unit 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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-734782652
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-732993082
**[Test build #131665 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131665/testReport)** for PR 30412 at commit [`b6d74c4`](https://github.com/apache/spark/commit/b6d74c481ecb651286685490b1beded99f0d50f9).
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-733899128
**[Test build #131803 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131803/testReport)** for PR 30412 at commit [`9850d38`](https://github.com/apache/spark/commit/9850d38d09d9545368cd7da8d5d65fc71043eac6).
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-734782652
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30412:
URL: https://github.com/apache/spark/pull/30412#discussion_r529618309
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CheckAnalysis.scala
##########
@@ -94,6 +94,10 @@ trait CheckAnalysis extends PredicateHelper {
case p if p.analyzed => // Skip already analyzed sub-plans
+ case leaf: LeafNode if leaf.output.map(_.dataType).exists(CharVarcharUtils.hasCharVarchar) =>
Review comment:
@maropu I changed it back as it's pretty risky to get output from arbitrary logical plans. An example of the error:
```
Caused by: sbt.ForkMain$ForkError: java.lang.AssertionError: assertion failed: Scalar subquery should have only one column
at scala.Predef$.assert(Predef.scala:223)
at org.apache.spark.sql.catalyst.expressions.ScalarSubquery.dataType(subquery.scala:229)
at org.apache.spark.sql.catalyst.expressions.Alias.toAttribute(namedExpressions.scala:181)
at org.apache.spark.sql.catalyst.plans.logical.Project.$anonfun$output$1(basicLogicalOperators.scala:61)
```
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-733985079
**[Test build #131798 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131798/testReport)** for PR 30412 at commit [`8f05eee`](https://github.com/apache/spark/commit/8f05eeeb36b3953a1f0500c7eb0bd664fa4cf70b).
* 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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-732993082
**[Test build #131665 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131665/testReport)** for PR 30412 at commit [`b6d74c4`](https://github.com/apache/spark/commit/b6d74c481ecb651286685490b1beded99f0d50f9).
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-730424894
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/131356/
Test FAILed.
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] support char/varchar type
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30412:
URL: https://github.com/apache/spark/pull/30412#discussion_r526230011
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/ApplyCharTypePadding.scala
##########
@@ -0,0 +1,135 @@
+/*
+ * 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.execution.datasources
+
+import org.apache.spark.sql.catalyst.catalog.HiveTableRelation
+import org.apache.spark.sql.catalyst.expressions.{Attribute, BinaryComparison, Expression, In, Literal, StringRPad}
+import org.apache.spark.sql.catalyst.plans.logical.{LogicalPlan, Project}
+import org.apache.spark.sql.catalyst.rules.Rule
+import org.apache.spark.sql.catalyst.util.CharVarcharUtils
+import org.apache.spark.sql.execution.datasources.v2.DataSourceV2Relation
+import org.apache.spark.sql.types.{CharType, StringType}
+import org.apache.spark.unsafe.types.UTF8String
+
+/**
+ * This rule applies char type padding in two places:
+ * 1. When reading values from column/field of type CHAR(N), right-pad the values to length N.
+ * 2. When comparing char type column/field with string literal or char type column/field,
+ * right-pad the shorter one to the longer length.
+ */
+object ApplyCharTypePadding extends Rule[LogicalPlan] {
+
+ override def apply(plan: LogicalPlan): LogicalPlan = {
+ val padded = plan.resolveOperatorsUpWithNewOutput {
+ case r: LogicalRelation =>
+ val projectList = CharVarcharUtils.charTypePadding(r.output)
+ if (projectList == r.output) {
+ r -> Nil
+ } else {
+ val cleanedOutput = r.output.map(CharVarcharUtils.cleanAttrMetadata)
Review comment:
same here, we remove the char/varchar metadata to make this rule idempotent.
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-733029359
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30412:
URL: https://github.com/apache/spark/pull/30412#discussion_r528875470
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CheckAnalysis.scala
##########
@@ -94,6 +94,10 @@ trait CheckAnalysis extends PredicateHelper {
case p if p.analyzed => // Skip already analyzed sub-plans
+ case p if p.output.map(_.dataType).exists(CharVarcharUtils.hasCharVarchar) =>
+ throw new IllegalStateException(
+ "[BUG] logical plan should not have output of char/varchar type: " + p)
Review comment:
I choose `IllegalStateException` because this should not happen. Thanks for catching this corner case and I've fixed it in `DataType.fromDDL`.
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-732450395
Merged build finished. Test FAILed.
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-734930827
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-733873411
**[Test build #131798 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131798/testReport)** for PR 30412 at commit [`8f05eee`](https://github.com/apache/spark/commit/8f05eeeb36b3953a1f0500c7eb0bd664fa4cf70b).
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-733899128
**[Test build #131803 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131803/testReport)** for PR 30412 at commit [`9850d38`](https://github.com/apache/spark/commit/9850d38d09d9545368cd7da8d5d65fc71043eac6).
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-732372670
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/131564/
Test FAILed.
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30412:
URL: https://github.com/apache/spark/pull/30412#discussion_r528882090
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/CharVarcharUtils.scala
##########
@@ -0,0 +1,277 @@
+/*
+ * 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 scala.collection.mutable
+
+import org.apache.spark.sql.catalyst.expressions._
+import org.apache.spark.sql.catalyst.parser.CatalystSqlParser
+import org.apache.spark.sql.types._
+
+object CharVarcharUtils {
+
+ private val CHAR_VARCHAR_TYPE_STRING_METADATA_KEY = "__CHAR_VARCHAR_TYPE_STRING"
+
+ /**
+ * Replaces CharType/VarcharType with StringType recursively in the given struct type. If a
+ * top-level StructField's data type is CharType/VarcharType or has nested CharType/VarcharType,
+ * this method will add the original type string to the StructField's metadata, so that we can
+ * re-construct the original data type with CharType/VarcharType later when needed.
+ */
+ def replaceCharVarcharWithStringInSchema(st: StructType): StructType = {
+ StructType(st.map { field =>
+ if (hasCharVarchar(field.dataType)) {
+ val metadata = new MetadataBuilder().withMetadata(field.metadata)
+ .putString(CHAR_VARCHAR_TYPE_STRING_METADATA_KEY, field.dataType.sql).build()
+ field.copy(dataType = replaceCharVarcharWithString(field.dataType), metadata = metadata)
+ } else {
+ field
+ }
+ })
+ }
+
+ /**
+ * Returns true if the given data type is CharType/VarcharType or has nested CharType/VarcharType.
+ */
+ def hasCharVarchar(dt: DataType): Boolean = {
+ dt.existsRecursively(f => f.isInstanceOf[CharType] || f.isInstanceOf[VarcharType])
+ }
+
+ /**
+ * Replaces CharType/VarcharType with StringType recursively in the given data type.
+ */
+ def replaceCharVarcharWithString(dt: DataType): DataType = dt match {
+ case ArrayType(et, nullable) =>
+ ArrayType(replaceCharVarcharWithString(et), nullable)
+ case MapType(kt, vt, nullable) =>
+ MapType(replaceCharVarcharWithString(kt), replaceCharVarcharWithString(vt), nullable)
+ case StructType(fields) =>
+ StructType(fields.map { field =>
+ field.copy(dataType = replaceCharVarcharWithString(field.dataType))
+ })
+ case _: CharType => StringType
+ case _: VarcharType => StringType
+ case _ => dt
+ }
+
+ /**
+ * Removes the metadata entry that contains the original type string of CharType/VarcharType from
+ * the given attribute's metadata.
+ */
+ def cleanAttrMetadata(attr: AttributeReference): AttributeReference = {
+ val cleaned = new MetadataBuilder().withMetadata(attr.metadata)
+ .remove(CHAR_VARCHAR_TYPE_STRING_METADATA_KEY).build()
+ attr.withMetadata(cleaned)
+ }
+
+ /**
+ * Re-construct the original data type from the type string in the given metadata.
+ * This is needed when dealing with char/varchar columns/fields.
+ */
+ def getRawType(metadata: Metadata): Option[DataType] = {
+ if (metadata.contains(CHAR_VARCHAR_TYPE_STRING_METADATA_KEY)) {
+ Some(CatalystSqlParser.parseRawDataType(
+ metadata.getString(CHAR_VARCHAR_TYPE_STRING_METADATA_KEY)))
+ } else {
+ None
+ }
+ }
+
+ /**
+ * Re-construct the original StructType from the type strings in the metadata of StructFields.
+ * This is needed when dealing with char/varchar columns/fields.
+ */
+ def getRawSchema(schema: StructType): StructType = {
Review comment:
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] cloud-fan commented on a change in pull request #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30412:
URL: https://github.com/apache/spark/pull/30412#discussion_r529253616
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/types/DataType.scala
##########
@@ -124,13 +125,16 @@ abstract class DataType extends AbstractDataType {
object DataType {
private val FIXED_DECIMAL = """decimal\(\s*(\d+)\s*,\s*(\-?\d+)\s*\)""".r
+ private val CHAR_TYPE = """char\(\s*(\d+)\s*\)""".r
+ private val VARCHAR_TYPE = """varchar\(\s*(\d+)\s*\)""".r
Review comment:
We don't. We can set a default length/max length in followups.
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30412:
URL: https://github.com/apache/spark/pull/30412#discussion_r526927962
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/types/CharType.scala
##########
@@ -0,0 +1,36 @@
+/*
+ * 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.types
+
+import scala.math.Ordering
+import scala.reflect.runtime.universe.typeTag
+
+import org.apache.spark.annotation.Experimental
+import org.apache.spark.unsafe.types.UTF8String
+
+@Experimental
+case class CharType(length: Int) extends AtomicType {
Review comment:
PR description updated
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-732988019
**[Test build #131653 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131653/testReport)** for PR 30412 at commit [`f46e32f`](https://github.com/apache/spark/commit/f46e32fb1649023eed0ddab4cb23ca4a97b14a0f).
* This patch **fails Spark unit 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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-733985770
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30412:
URL: https://github.com/apache/spark/pull/30412#discussion_r526633910
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/types/CharType.scala
##########
@@ -0,0 +1,36 @@
+/*
+ * 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.types
+
+import scala.math.Ordering
+import scala.reflect.runtime.universe.typeTag
+
+import org.apache.spark.annotation.Experimental
+import org.apache.spark.unsafe.types.UTF8String
+
+@Experimental
+case class CharType(length: Int) extends AtomicType {
Review comment:
It's already there: https://github.com/apache/spark/pull/30412/files#diff-aacc5ed42589c636615a3c09a44fa6a5248195242c9fbe0e996db17471cd35fdL70
I just moved it to a new file and remove the parent class.
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-732838910
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-732347473
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/36163/
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30412:
URL: https://github.com/apache/spark/pull/30412#discussion_r526634504
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/types/CharType.scala
##########
@@ -0,0 +1,36 @@
+/*
+ * 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.types
+
+import scala.math.Ordering
+import scala.reflect.runtime.universe.typeTag
+
+import org.apache.spark.annotation.Experimental
+import org.apache.spark.unsafe.types.UTF8String
+
+@Experimental
+case class CharType(length: Int) extends AtomicType {
Review comment:
And type system means a lot more, e.g. expression type check, InternalRow.get, etc. Maybe I should say "type system framework"?
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-734829598
**[Test build #131884 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131884/testReport)** for PR 30412 at commit [`73b99dc`](https://github.com/apache/spark/commit/73b99dc7a98f5d0673d309adba457cd19144be92).
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30412:
URL: https://github.com/apache/spark/pull/30412#discussion_r526640721
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CheckAnalysis.scala
##########
@@ -94,6 +94,10 @@ trait CheckAnalysis extends PredicateHelper {
case p if p.analyzed => // Skip already analyzed sub-plans
+ case leaf: LeafNode if leaf.output.map(_.dataType).exists(CharVarcharUtils.hasCharVarchar) =>
Review comment:
But it's safer to check all the nodes, I'll update 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] cloud-fan commented on pull request #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-733723789
@maropu @dongjoon-hyun any more comments?
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-734709360
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-732557513
**[Test build #131597 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131597/testReport)** for PR 30412 at commit [`6182921`](https://github.com/apache/spark/commit/6182921657ae0757549d2544088ff4442d8a08a7).
* This patch **fails Spark unit 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 removed a comment on pull request #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-732756465
**[Test build #131637 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131637/testReport)** for PR 30412 at commit [`f46e32f`](https://github.com/apache/spark/commit/f46e32fb1649023eed0ddab4cb23ca4a97b14a0f).
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-734826951
**[Test build #131876 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131876/testReport)** for PR 30412 at commit [`3bbe7e7`](https://github.com/apache/spark/commit/3bbe7e784b0544dbd05fbe113ade387d080ba990).
* This patch **fails Spark unit 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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-732367871
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/36165/
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-730586798
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-734700150
**[Test build #131866 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131866/testReport)** for PR 30412 at commit [`69adca5`](https://github.com/apache/spark/commit/69adca50f12fde7deefc7bef2e31d459526cac3c).
* This patch **fails Spark unit 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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-733202282
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-733922316
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-734810322
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-734695503
Not that, before this PR, people can already create varchar type column but it has no length check during write. People can already create char type column with hive table but it truncates input string during write. This PR doesn't make things worse.
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-733415048
**[Test build #131716 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131716/testReport)** for PR 30412 at commit [`38999b5`](https://github.com/apache/spark/commit/38999b535e78817d2647d186605618438f438220).
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-733265378
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-730478758
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/35960/
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-733945203
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-734731027
**[Test build #131876 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131876/testReport)** for PR 30412 at commit [`3bbe7e7`](https://github.com/apache/spark/commit/3bbe7e784b0544dbd05fbe113ade387d080ba990).
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-734171079
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-733415048
**[Test build #131716 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131716/testReport)** for PR 30412 at commit [`38999b5`](https://github.com/apache/spark/commit/38999b535e78817d2647d186605618438f438220).
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-734828730
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-733202278
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #30412:
URL: https://github.com/apache/spark/pull/30412#discussion_r530824544
##########
File path: docs/sql-ref-datatypes.md
##########
@@ -37,6 +37,8 @@ Spark SQL and DataFrames support the following data types:
- `DecimalType`: Represents arbitrary-precision signed decimal numbers. Backed internally by `java.math.BigDecimal`. A `BigDecimal` consists of an arbitrary precision integer unscaled value and a 32-bit integer scale.
* String type
- `StringType`: Represents character string values.
+ - `VarcharType(length)`: A variant of `StringType` which has a length limitation. Data writing will fail if the input string exceeds the length limitation. Note: this type can only be used in table schema, not functions/operators.
+ - `CharType(length)`: A variant of `VarcharType(length)` which is fixed length. Reading column of type `VarcharType(n)` always returns string values of length `n`. Char type column comparison will pad the short one to the longer length.
Review comment:
Reading column of type `VarcharType(n)` -> Reading column of type `CharType(n)`?
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-734783592
**[Test build #131881 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131881/testReport)** for PR 30412 at commit [`73b99dc`](https://github.com/apache/spark/commit/73b99dc7a98f5d0673d309adba457cd19144be92).
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30412:
URL: https://github.com/apache/spark/pull/30412#discussion_r526640152
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala
##########
@@ -3088,7 +3088,12 @@ class Analyzer(override val catalogManager: CatalogManager)
val projection = TableOutputResolver.resolveOutputColumns(
v2Write.table.name, v2Write.table.output, v2Write.query, v2Write.isByName, conf)
if (projection != v2Write.query) {
- v2Write.withNewQuery(projection)
+ val cleanedTable = v2Write.table match {
+ case r: DataSourceV2Relation =>
+ r.copy(output = r.output.map(CharVarcharUtils.cleanAttrMetadata))
Review comment:
No it doesn't. Metadata is fine as it's harmless. We only need to watch out for some specific rules that look at the char/varchar metadata, and make sure they are idempotent.
As a fact, the added cast and length check expression is wrapped by an `Alias` which retains char/varchar metadata. So the output attributes of `Project` above the v2 relation still have metadata. It's necessary as we need to rely on it later to do padding for char type column comparison.
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-730424884
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30412:
URL: https://github.com/apache/spark/pull/30412#discussion_r526635139
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/types/VarcharType.scala
##########
@@ -0,0 +1,35 @@
+/*
+ * 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.types
+
+import scala.math.Ordering
+import scala.reflect.runtime.universe.typeTag
+
+import org.apache.spark.annotation.Experimental
+import org.apache.spark.unsafe.types.UTF8String
+
+@Experimental
+case class VarcharType(length: Int) extends AtomicType {
Review comment:
VarcharType was already there: https://github.com/apache/spark/pull/30412/files#diff-aacc5ed42589c636615a3c09a44fa6a5248195242c9fbe0e996db17471cd35fdL79
I think it's a good idea to check the negative value, but adding max length should be done separately.
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-730545928
**[Test build #131365 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131365/testReport)** for PR 30412 at commit [`c239834`](https://github.com/apache/spark/commit/c239834ef7ce730910d2d3a64f858c2c53dbc6a5).
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] support char/varchar type
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-729795850
**[Test build #131291 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131291/testReport)** for PR 30412 at commit [`a7f0730`](https://github.com/apache/spark/commit/a7f0730a48850c237313e9471fe63cbf2eef9a95).
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] support char/varchar type
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-729844962
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/35895/
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-730586798
Merged build finished. Test FAILed.
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-732939546
**[Test build #131654 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131654/testReport)** for PR 30412 at commit [`e5fb41a`](https://github.com/apache/spark/commit/e5fb41aa2aa220b0750c6d6812e34075a9c118b2).
* This patch **fails Spark unit 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 removed a comment on pull request #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-734061563
**[Test build #131827 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131827/testReport)** for PR 30412 at commit [`671471f`](https://github.com/apache/spark/commit/671471f9dad6a3cffa4c5442daba32424d9d3c13).
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30412:
URL: https://github.com/apache/spark/pull/30412#discussion_r526632969
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/TableOutputResolver.scala
##########
@@ -93,19 +94,17 @@ object TableOutputResolver {
tableAttr.metadata == queryExpr.metadata) {
Some(queryExpr)
} else {
- // Renaming is needed for handling the following cases like
- // 1) Column names/types do not match, e.g., INSERT INTO TABLE tab1 SELECT 1, 2
- // 2) Target tables have column metadata
- storeAssignmentPolicy match {
+ val casted = storeAssignmentPolicy match {
case StoreAssignmentPolicy.ANSI =>
- Some(Alias(
- AnsiCast(queryExpr, tableAttr.dataType, Option(conf.sessionLocalTimeZone)),
- tableAttr.name)(explicitMetadata = Option(tableAttr.metadata)))
+ AnsiCast(queryExpr, tableAttr.dataType, Option(conf.sessionLocalTimeZone))
case _ =>
- Some(Alias(
- Cast(queryExpr, tableAttr.dataType, Option(conf.sessionLocalTimeZone)),
- tableAttr.name)(explicitMetadata = Option(tableAttr.metadata)))
+ Cast(queryExpr, tableAttr.dataType, Option(conf.sessionLocalTimeZone))
}
+ val strLenChecked = CharVarcharUtils.stringLengthCheck(casted, tableAttr)
Review comment:
> To avoid accidentally adding the length check exprs again, we cannot remove the metadata at the same time as adding the exprs?
We can't access the table relation here, only table output attrs.
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-734704291
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-732326747
**[Test build #131564 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131564/testReport)** for PR 30412 at commit [`6182921`](https://github.com/apache/spark/commit/6182921657ae0757549d2544088ff4442d8a08a7).
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-734076827
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-732372664
Merged build finished. Test FAILed.
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-733811335
**[Test build #131786 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131786/testReport)** for PR 30412 at commit [`8789494`](https://github.com/apache/spark/commit/8789494b87a17d5371b61731bbcee2386caf9074).
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-732351936
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/36165/
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] support char/varchar type
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-729844978
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-732450399
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/131561/
Test FAILed.
----------------------------------------------------------------
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 a change in pull request #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
maropu commented on a change in pull request #30412:
URL: https://github.com/apache/spark/pull/30412#discussion_r529075728
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/CharVarcharTestSuite.scala
##########
@@ -0,0 +1,374 @@
+/*
+ * 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
+
+import org.apache.spark.{SparkConf, SparkException}
+import org.apache.spark.sql.connector.InMemoryTableCatalog
+import org.apache.spark.sql.internal.SQLConf
+import org.apache.spark.sql.test.{SharedSparkSession, SQLTestUtils}
+
+trait CharVarcharTestSuite extends QueryTest with SQLTestUtils {
+
+ def format: String
+
+ test("char type values should be padded: top-level columns") {
+ withTable("t") {
+ sql(s"CREATE TABLE t(i STRING, c CHAR(5)) USING $format")
+ sql("INSERT INTO t VALUES ('1', 'a')")
+ checkAnswer(spark.table("t"), Row("1", "a" + " " * 4))
+ }
+ }
+
+ test("char type values should be padded: partitioned columns") {
+ // DS V2 doesn't support partitioned table.
+ if (!conf.contains(SQLConf.DEFAULT_CATALOG.key)) {
+ withTable("t") {
+ sql(s"CREATE TABLE t(i STRING, c CHAR(5)) USING $format PARTITIONED BY (c)")
+ sql("INSERT INTO t VALUES ('1', 'a')")
+ checkAnswer(spark.table("t"), Row("1", "a" + " " * 4))
+ }
+ }
+ }
+
+ test("char type values should be padded: nested in struct") {
+ withTable("t") {
+ sql(s"CREATE TABLE t(i STRING, c STRUCT<c: CHAR(5)>) USING $format")
+ sql("INSERT INTO t VALUES ('1', struct('a'))")
+ checkAnswer(spark.table("t"), Row("1", Row("a" + " " * 4)))
+ }
+ }
+
+ test("char type values should be padded: nested in array") {
+ withTable("t") {
+ sql(s"CREATE TABLE t(i STRING, c ARRAY<CHAR(5)>) USING $format")
+ sql("INSERT INTO t VALUES ('1', array('a', 'ab'))")
+ checkAnswer(spark.table("t"), Row("1", Seq("a" + " " * 4, "ab" + " " * 3)))
+ }
+ }
+
+ test("char type values should be padded: nested in map key") {
+ withTable("t") {
+ sql(s"CREATE TABLE t(i STRING, c MAP<CHAR(5), STRING>) USING $format")
+ sql("INSERT INTO t VALUES ('1', map('a', 'ab'))")
+ checkAnswer(spark.table("t"), Row("1", Map(("a" + " " * 4, "ab"))))
+ }
+ }
+
+ test("char type values should be padded: nested in map value") {
+ withTable("t") {
+ sql(s"CREATE TABLE t(i STRING, c MAP<STRING, CHAR(5)>) USING $format")
+ sql("INSERT INTO t VALUES ('1', map('a', 'ab'))")
+ checkAnswer(spark.table("t"), Row("1", Map(("a", "ab" + " " * 3))))
+ }
+ }
+
+ test("char type values should be padded: nested in both map key and value") {
+ withTable("t") {
+ sql(s"CREATE TABLE t(i STRING, c MAP<CHAR(5), CHAR(10)>) USING $format")
+ sql("INSERT INTO t VALUES ('1', map('a', 'ab'))")
+ checkAnswer(spark.table("t"), Row("1", Map(("a" + " " * 4, "ab" + " " * 8))))
+ }
+ }
+
+ test("char type values should be padded: nested in struct of array") {
+ withTable("t") {
+ sql(s"CREATE TABLE t(i STRING, c STRUCT<c: ARRAY<CHAR(5)>>) USING $format")
+ sql("INSERT INTO t VALUES ('1', struct(array('a', 'ab')))")
+ checkAnswer(spark.table("t"), Row("1", Row(Seq("a" + " " * 4, "ab" + " " * 3))))
+ }
+ }
+
+ test("char type values should be padded: nested in array of struct") {
+ withTable("t") {
+ sql(s"CREATE TABLE t(i STRING, c ARRAY<STRUCT<c: CHAR(5)>>) USING $format")
+ sql("INSERT INTO t VALUES ('1', array(struct('a'), struct('ab')))")
+ checkAnswer(spark.table("t"), Row("1", Seq(Row("a" + " " * 4), Row("ab" + " " * 3))))
+ }
+ }
+
+ test("char type values should be padded: nested in array of array") {
+ withTable("t") {
+ sql(s"CREATE TABLE t(i STRING, c ARRAY<ARRAY<CHAR(5)>>) USING $format")
+ sql("INSERT INTO t VALUES ('1', array(array('a', 'ab')))")
+ checkAnswer(spark.table("t"), Row("1", Seq(Seq("a" + " " * 4, "ab" + " " * 3))))
+ }
+ }
+
+ private def testTableWrite(f: String => Unit): Unit = {
+ withTable("t") { f("char") }
+ withTable("t") { f("varchar") }
+ }
+
+ test("length check for input string values: top-level columns") {
+ testTableWrite { typeName =>
+ sql(s"CREATE TABLE t(c $typeName(5)) USING $format")
+ sql("INSERT INTO t VALUES (null)")
+ checkAnswer(spark.table("t"), Row(null))
+ val e = intercept[SparkException](sql("INSERT INTO t VALUES ('123456')"))
+ assert(e.getCause.getMessage.contains(
+ s"input string '123456' exceeds $typeName type length limitation: 5"))
+ }
+ }
+
+ test("length check for input string values: partitioned columns") {
+ // DS V2 doesn't support partitioned table.
+ if (!conf.contains(SQLConf.DEFAULT_CATALOG.key)) {
+ testTableWrite { typeName =>
+ sql(s"CREATE TABLE t(i INT, c $typeName(5)) USING $format PARTITIONED BY (c)")
+ sql("INSERT INTO t VALUES (1, null)")
+ checkAnswer(spark.table("t"), Row(1, null))
+ val e = intercept[SparkException](sql("INSERT INTO t VALUES (1, '123456')"))
+ assert(e.getCause.getMessage.contains(
+ s"input string '123456' exceeds $typeName type length limitation: 5"))
+ }
+ }
+ }
+
+ test("length check for input string values: nested in struct") {
+ testTableWrite { typeName =>
+ sql(s"CREATE TABLE t(c STRUCT<c: $typeName(5)>) USING $format")
+ sql("INSERT INTO t SELECT struct(null)")
+ checkAnswer(spark.table("t"), Row(Row(null)))
+ val e = intercept[SparkException](sql("INSERT INTO t SELECT struct('123456')"))
+ assert(e.getCause.getMessage.contains(
+ s"input string '123456' exceeds $typeName type length limitation: 5"))
+ }
+ }
+
+ test("length check for input string values: nested in array") {
+ testTableWrite { typeName =>
+ sql(s"CREATE TABLE t(c ARRAY<$typeName(5)>) USING $format")
+ sql("INSERT INTO t VALUES (array(null))")
+ checkAnswer(spark.table("t"), Row(Seq(null)))
+ val e = intercept[SparkException](sql("INSERT INTO t VALUES (array('a', '123456'))"))
+ assert(e.getCause.getMessage.contains(
+ s"input string '123456' exceeds $typeName type length limitation: 5"))
+ }
+ }
+
+ test("length check for input string values: nested in map key") {
+ testTableWrite { typeName =>
+ sql(s"CREATE TABLE t(c MAP<$typeName(5), STRING>) USING $format")
+ val e = intercept[SparkException](sql("INSERT INTO t VALUES (map('123456', 'a'))"))
+ assert(e.getCause.getMessage.contains(
+ s"input string '123456' exceeds $typeName type length limitation: 5"))
+ }
+ }
+
+ test("length check for input string values: nested in map value") {
+ testTableWrite { typeName =>
+ sql(s"CREATE TABLE t(c MAP<STRING, $typeName(5)>) USING $format")
+ sql("INSERT INTO t VALUES (map('a', null))")
+ checkAnswer(spark.table("t"), Row(Map("a" -> null)))
+ val e = intercept[SparkException](sql("INSERT INTO t VALUES (map('a', '123456'))"))
+ assert(e.getCause.getMessage.contains(
+ s"input string '123456' exceeds $typeName type length limitation: 5"))
+ }
+ }
+
+ test("length check for input string values: nested in both map key and value") {
+ testTableWrite { typeName =>
+ sql(s"CREATE TABLE t(c MAP<$typeName(5), $typeName(5)>) USING $format")
+ val e1 = intercept[SparkException](sql("INSERT INTO t VALUES (map('123456', 'a'))"))
+ assert(e1.getCause.getMessage.contains(
+ s"input string '123456' exceeds $typeName type length limitation: 5"))
+ val e2 = intercept[SparkException](sql("INSERT INTO t VALUES (map('a', '123456'))"))
+ assert(e2.getCause.getMessage.contains(
+ s"input string '123456' exceeds $typeName type length limitation: 5"))
+ }
+ }
+
+ test("length check for input string values: nested in struct of array") {
+ testTableWrite { typeName =>
+ sql(s"CREATE TABLE t(c STRUCT<c: ARRAY<$typeName(5)>>) USING $format")
+ sql("INSERT INTO t SELECT struct(array(null))")
+ checkAnswer(spark.table("t"), Row(Row(Seq(null))))
+ val e = intercept[SparkException](sql("INSERT INTO t SELECT struct(array('123456'))"))
+ assert(e.getCause.getMessage.contains(
+ s"input string '123456' exceeds $typeName type length limitation: 5"))
+ }
+ }
+
+ test("length check for input string values: nested in array of struct") {
+ testTableWrite { typeName =>
+ sql(s"CREATE TABLE t(c ARRAY<STRUCT<c: $typeName(5)>>) USING $format")
+ sql("INSERT INTO t VALUES (array(struct(null)))")
+ checkAnswer(spark.table("t"), Row(Seq(Row(null))))
+ val e = intercept[SparkException](sql("INSERT INTO t VALUES (array(struct('123456')))"))
+ assert(e.getCause.getMessage.contains(
+ s"input string '123456' exceeds $typeName type length limitation: 5"))
+ }
+ }
+
+ test("length check for input string values: nested in array of array") {
+ testTableWrite { typeName =>
+ sql(s"CREATE TABLE t(c ARRAY<ARRAY<$typeName(5)>>) USING $format")
+ sql("INSERT INTO t VALUES (array(array(null)))")
+ checkAnswer(spark.table("t"), Row(Seq(Seq(null))))
+ val e = intercept[SparkException](sql("INSERT INTO t VALUES (array(array('123456')))"))
+ assert(e.getCause.getMessage.contains(
+ s"input string '123456' exceeds $typeName type length limitation: 5"))
+ }
+ }
+
+ test("length check for input string values: with trailing spaces") {
+ withTable("t") {
+ sql(s"CREATE TABLE t(c1 CHAR(5), c2 VARCHAR(5)) USING $format")
+ sql("INSERT INTO t VALUES ('12 ', '12 ')")
+ sql("INSERT INTO t VALUES ('1234 ', '1234 ')")
+ checkAnswer(spark.table("t"), Seq(
+ Row("12" + " " * 3, "12 "),
+ Row("1234 ", "1234 ")))
+ }
+ }
+
+ test("length check for input string values: with implicit cast") {
+ withTable("t") {
+ sql(s"CREATE TABLE t(c1 CHAR(5), c2 VARCHAR(5)) USING $format")
+ sql("INSERT INTO t VALUES (1234, 1234)")
+ checkAnswer(spark.table("t"), Row("1234 ", "1234"))
+ val e1 = intercept[SparkException](sql("INSERT INTO t VALUES (123456, 1)"))
+ assert(e1.getCause.getMessage.contains(
+ "input string '123456' exceeds char type length limitation: 5"))
+ val e2 = intercept[SparkException](sql("INSERT INTO t VALUES (1, 123456)"))
+ assert(e2.getCause.getMessage.contains(
+ "input string '123456' exceeds varchar type length limitation: 5"))
+ }
+ }
+
+ private def testConditions(df: DataFrame, conditions: Seq[(String, Boolean)]): Unit = {
+ checkAnswer(df.selectExpr(conditions.map(_._1): _*), Row.fromSeq(conditions.map(_._2)))
+ }
+
+ test("char type comparison: top-level columns") {
+ withTable("t") {
+ sql(s"CREATE TABLE t(c1 CHAR(2), c2 CHAR(5)) USING $format")
+ sql("INSERT INTO t VALUES ('a', 'a')")
+ testConditions(spark.table("t"), Seq(
+ ("c1 = 'a'", true),
+ ("'a' = c1", true),
+ ("c1 = 'a '", true),
+ ("c1 > 'a'", false),
+ ("c1 IN ('a', 'b')", true),
+ ("c1 = c2", true),
+ ("c1 < c2", false),
+ ("c1 IN (c2)", true)))
+ }
+ }
+
+ test("char type comparison: partitioned columns") {
+ withTable("t") {
+ sql(s"CREATE TABLE t(i INT, c1 CHAR(2), c2 CHAR(5)) USING $format PARTITIONED BY (c1, c2)")
+ sql("INSERT INTO t VALUES (1, 'a', 'a')")
+ testConditions(spark.table("t"), Seq(
+ ("c1 = 'a'", true),
+ ("'a' = c1", true),
+ ("c1 = 'a '", true),
+ ("c1 > 'a'", false),
+ ("c1 IN ('a', 'b')", true),
+ ("c1 = c2", true),
+ ("c1 < c2", false),
+ ("c1 IN (c2)", true)))
+ }
+ }
+
+ test("char type comparison: join") {
+ withTable("t1", "t2") {
+ sql(s"CREATE TABLE t1(c CHAR(2)) USING $format")
+ sql(s"CREATE TABLE t2(c CHAR(5)) USING $format")
+ sql("INSERT INTO t1 VALUES ('a')")
+ sql("INSERT INTO t2 VALUES ('a')")
+ checkAnswer(sql("SELECT t1.c FROM t1 JOIN t2 ON t1.c = t2.c"), Row("a "))
+ }
+ }
+
+ test("char type comparison: nested in struct") {
+ withTable("t") {
+ sql(s"CREATE TABLE t(c1 STRUCT<c: CHAR(2)>, c2 STRUCT<c: CHAR(5)>) USING $format")
+ sql("INSERT INTO t VALUES (struct('a'), struct('a'))")
+ testConditions(spark.table("t"), Seq(
+ ("c1 = c2", true),
+ ("c1 < c2", false),
+ ("c1 IN (c2)", true)))
+ }
+ }
+
+ test("char type comparison: nested in array") {
+ withTable("t") {
+ sql(s"CREATE TABLE t(c1 ARRAY<CHAR(2)>, c2 ARRAY<CHAR(5)>) USING $format")
+ sql("INSERT INTO t VALUES (array('a', 'b'), array('a', 'b'))")
+ testConditions(spark.table("t"), Seq(
+ ("c1 = c2", true),
+ ("c1 < c2", false),
+ ("c1 IN (c2)", true)))
+ }
+ }
+
+ test("char type comparison: nested in struct of array") {
+ withTable("t") {
+ sql("CREATE TABLE t(c1 STRUCT<a: ARRAY<CHAR(2)>>, c2 STRUCT<a: ARRAY<CHAR(5)>>) " +
+ s"USING $format")
+ sql("INSERT INTO t VALUES (struct(array('a', 'b')), struct(array('a', 'b')))")
+ testConditions(spark.table("t"), Seq(
+ ("c1 = c2", true),
+ ("c1 < c2", false),
+ ("c1 IN (c2)", true)))
+ }
+ }
+
+ test("char type comparison: nested in array of struct") {
+ withTable("t") {
+ sql("CREATE TABLE t(c1 ARRAY<STRUCT<c: CHAR(2)>>, c2 ARRAY<STRUCT<c: CHAR(5)>>) " +
+ s"USING $format")
+ sql("INSERT INTO t VALUES (array(struct('a')), array(struct('a')))")
+ testConditions(spark.table("t"), Seq(
+ ("c1 = c2", true),
+ ("c1 < c2", false),
+ ("c1 IN (c2)", true)))
+ }
+ }
+
+ test("char type comparison: nested in array of array") {
Review comment:
Ah, I see. If so, why the function `typeWithWiderCharLength` has an entry for `Map`? https://github.com/apache/spark/pull/30412/files#diff-16753285e80505e04c445ea8ccee1dde7ae601ed7bae224e212d90d395f57928R225-R226
It seems the function is only used for `addPaddingInStringComparison`.
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-732504623
**[Test build #131597 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131597/testReport)** for PR 30412 at commit [`6182921`](https://github.com/apache/spark/commit/6182921657ae0757549d2544088ff4442d8a08a7).
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-733007577
**[Test build #131665 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131665/testReport)** for PR 30412 at commit [`b6d74c4`](https://github.com/apache/spark/commit/b6d74c481ecb651286685490b1beded99f0d50f9).
* This patch **fails Spark unit 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] maropu commented on a change in pull request #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
maropu commented on a change in pull request #30412:
URL: https://github.com/apache/spark/pull/30412#discussion_r526645876
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/TableOutputResolver.scala
##########
@@ -93,19 +94,17 @@ object TableOutputResolver {
tableAttr.metadata == queryExpr.metadata) {
Some(queryExpr)
} else {
- // Renaming is needed for handling the following cases like
- // 1) Column names/types do not match, e.g., INSERT INTO TABLE tab1 SELECT 1, 2
- // 2) Target tables have column metadata
- storeAssignmentPolicy match {
+ val casted = storeAssignmentPolicy match {
case StoreAssignmentPolicy.ANSI =>
- Some(Alias(
- AnsiCast(queryExpr, tableAttr.dataType, Option(conf.sessionLocalTimeZone)),
- tableAttr.name)(explicitMetadata = Option(tableAttr.metadata)))
+ AnsiCast(queryExpr, tableAttr.dataType, Option(conf.sessionLocalTimeZone))
case _ =>
- Some(Alias(
- Cast(queryExpr, tableAttr.dataType, Option(conf.sessionLocalTimeZone)),
- tableAttr.name)(explicitMetadata = Option(tableAttr.metadata)))
+ Cast(queryExpr, tableAttr.dataType, Option(conf.sessionLocalTimeZone))
}
+ val strLenChecked = CharVarcharUtils.stringLengthCheck(casted, tableAttr)
Review comment:
> We can't access the table relation here, only table output attrs.
Oh, I see.
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-730493430
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/35960/
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-733987932
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-734683690
**[Test build #131866 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131866/testReport)** for PR 30412 at commit [`69adca5`](https://github.com/apache/spark/commit/69adca50f12fde7deefc7bef2e31d459526cac3c).
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-732838910
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-734061563
**[Test build #131827 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131827/testReport)** for PR 30412 at commit [`671471f`](https://github.com/apache/spark/commit/671471f9dad6a3cffa4c5442daba32424d9d3c13).
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-734828730
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-732939815
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-734810322
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30412:
URL: https://github.com/apache/spark/pull/30412#discussion_r529253246
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/CharVarcharTestSuite.scala
##########
@@ -0,0 +1,374 @@
+/*
+ * 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
+
+import org.apache.spark.{SparkConf, SparkException}
+import org.apache.spark.sql.connector.InMemoryTableCatalog
+import org.apache.spark.sql.internal.SQLConf
+import org.apache.spark.sql.test.{SharedSparkSession, SQLTestUtils}
+
+trait CharVarcharTestSuite extends QueryTest with SQLTestUtils {
+
+ def format: String
+
+ test("char type values should be padded: top-level columns") {
+ withTable("t") {
+ sql(s"CREATE TABLE t(i STRING, c CHAR(5)) USING $format")
+ sql("INSERT INTO t VALUES ('1', 'a')")
+ checkAnswer(spark.table("t"), Row("1", "a" + " " * 4))
+ }
+ }
+
+ test("char type values should be padded: partitioned columns") {
+ // DS V2 doesn't support partitioned table.
+ if (!conf.contains(SQLConf.DEFAULT_CATALOG.key)) {
+ withTable("t") {
+ sql(s"CREATE TABLE t(i STRING, c CHAR(5)) USING $format PARTITIONED BY (c)")
+ sql("INSERT INTO t VALUES ('1', 'a')")
+ checkAnswer(spark.table("t"), Row("1", "a" + " " * 4))
+ }
+ }
+ }
+
+ test("char type values should be padded: nested in struct") {
+ withTable("t") {
+ sql(s"CREATE TABLE t(i STRING, c STRUCT<c: CHAR(5)>) USING $format")
+ sql("INSERT INTO t VALUES ('1', struct('a'))")
+ checkAnswer(spark.table("t"), Row("1", Row("a" + " " * 4)))
+ }
+ }
+
+ test("char type values should be padded: nested in array") {
+ withTable("t") {
+ sql(s"CREATE TABLE t(i STRING, c ARRAY<CHAR(5)>) USING $format")
+ sql("INSERT INTO t VALUES ('1', array('a', 'ab'))")
+ checkAnswer(spark.table("t"), Row("1", Seq("a" + " " * 4, "ab" + " " * 3)))
+ }
+ }
+
+ test("char type values should be padded: nested in map key") {
+ withTable("t") {
+ sql(s"CREATE TABLE t(i STRING, c MAP<CHAR(5), STRING>) USING $format")
+ sql("INSERT INTO t VALUES ('1', map('a', 'ab'))")
+ checkAnswer(spark.table("t"), Row("1", Map(("a" + " " * 4, "ab"))))
+ }
+ }
+
+ test("char type values should be padded: nested in map value") {
+ withTable("t") {
+ sql(s"CREATE TABLE t(i STRING, c MAP<STRING, CHAR(5)>) USING $format")
+ sql("INSERT INTO t VALUES ('1', map('a', 'ab'))")
+ checkAnswer(spark.table("t"), Row("1", Map(("a", "ab" + " " * 3))))
+ }
+ }
+
+ test("char type values should be padded: nested in both map key and value") {
+ withTable("t") {
+ sql(s"CREATE TABLE t(i STRING, c MAP<CHAR(5), CHAR(10)>) USING $format")
+ sql("INSERT INTO t VALUES ('1', map('a', 'ab'))")
+ checkAnswer(spark.table("t"), Row("1", Map(("a" + " " * 4, "ab" + " " * 8))))
+ }
+ }
+
+ test("char type values should be padded: nested in struct of array") {
+ withTable("t") {
+ sql(s"CREATE TABLE t(i STRING, c STRUCT<c: ARRAY<CHAR(5)>>) USING $format")
+ sql("INSERT INTO t VALUES ('1', struct(array('a', 'ab')))")
+ checkAnswer(spark.table("t"), Row("1", Row(Seq("a" + " " * 4, "ab" + " " * 3))))
+ }
+ }
+
+ test("char type values should be padded: nested in array of struct") {
+ withTable("t") {
+ sql(s"CREATE TABLE t(i STRING, c ARRAY<STRUCT<c: CHAR(5)>>) USING $format")
+ sql("INSERT INTO t VALUES ('1', array(struct('a'), struct('ab')))")
+ checkAnswer(spark.table("t"), Row("1", Seq(Row("a" + " " * 4), Row("ab" + " " * 3))))
+ }
+ }
+
+ test("char type values should be padded: nested in array of array") {
+ withTable("t") {
+ sql(s"CREATE TABLE t(i STRING, c ARRAY<ARRAY<CHAR(5)>>) USING $format")
+ sql("INSERT INTO t VALUES ('1', array(array('a', 'ab')))")
+ checkAnswer(spark.table("t"), Row("1", Seq(Seq("a" + " " * 4, "ab" + " " * 3))))
+ }
+ }
+
+ private def testTableWrite(f: String => Unit): Unit = {
+ withTable("t") { f("char") }
+ withTable("t") { f("varchar") }
+ }
+
+ test("length check for input string values: top-level columns") {
+ testTableWrite { typeName =>
+ sql(s"CREATE TABLE t(c $typeName(5)) USING $format")
+ sql("INSERT INTO t VALUES (null)")
+ checkAnswer(spark.table("t"), Row(null))
+ val e = intercept[SparkException](sql("INSERT INTO t VALUES ('123456')"))
+ assert(e.getCause.getMessage.contains(
+ s"input string '123456' exceeds $typeName type length limitation: 5"))
+ }
+ }
+
+ test("length check for input string values: partitioned columns") {
+ // DS V2 doesn't support partitioned table.
+ if (!conf.contains(SQLConf.DEFAULT_CATALOG.key)) {
+ testTableWrite { typeName =>
+ sql(s"CREATE TABLE t(i INT, c $typeName(5)) USING $format PARTITIONED BY (c)")
+ sql("INSERT INTO t VALUES (1, null)")
+ checkAnswer(spark.table("t"), Row(1, null))
+ val e = intercept[SparkException](sql("INSERT INTO t VALUES (1, '123456')"))
+ assert(e.getCause.getMessage.contains(
+ s"input string '123456' exceeds $typeName type length limitation: 5"))
+ }
+ }
+ }
+
+ test("length check for input string values: nested in struct") {
+ testTableWrite { typeName =>
+ sql(s"CREATE TABLE t(c STRUCT<c: $typeName(5)>) USING $format")
+ sql("INSERT INTO t SELECT struct(null)")
+ checkAnswer(spark.table("t"), Row(Row(null)))
+ val e = intercept[SparkException](sql("INSERT INTO t SELECT struct('123456')"))
+ assert(e.getCause.getMessage.contains(
+ s"input string '123456' exceeds $typeName type length limitation: 5"))
+ }
+ }
+
+ test("length check for input string values: nested in array") {
+ testTableWrite { typeName =>
+ sql(s"CREATE TABLE t(c ARRAY<$typeName(5)>) USING $format")
+ sql("INSERT INTO t VALUES (array(null))")
+ checkAnswer(spark.table("t"), Row(Seq(null)))
+ val e = intercept[SparkException](sql("INSERT INTO t VALUES (array('a', '123456'))"))
+ assert(e.getCause.getMessage.contains(
+ s"input string '123456' exceeds $typeName type length limitation: 5"))
+ }
+ }
+
+ test("length check for input string values: nested in map key") {
+ testTableWrite { typeName =>
+ sql(s"CREATE TABLE t(c MAP<$typeName(5), STRING>) USING $format")
+ val e = intercept[SparkException](sql("INSERT INTO t VALUES (map('123456', 'a'))"))
+ assert(e.getCause.getMessage.contains(
+ s"input string '123456' exceeds $typeName type length limitation: 5"))
+ }
+ }
+
+ test("length check for input string values: nested in map value") {
+ testTableWrite { typeName =>
+ sql(s"CREATE TABLE t(c MAP<STRING, $typeName(5)>) USING $format")
+ sql("INSERT INTO t VALUES (map('a', null))")
+ checkAnswer(spark.table("t"), Row(Map("a" -> null)))
+ val e = intercept[SparkException](sql("INSERT INTO t VALUES (map('a', '123456'))"))
+ assert(e.getCause.getMessage.contains(
+ s"input string '123456' exceeds $typeName type length limitation: 5"))
+ }
+ }
+
+ test("length check for input string values: nested in both map key and value") {
+ testTableWrite { typeName =>
+ sql(s"CREATE TABLE t(c MAP<$typeName(5), $typeName(5)>) USING $format")
+ val e1 = intercept[SparkException](sql("INSERT INTO t VALUES (map('123456', 'a'))"))
+ assert(e1.getCause.getMessage.contains(
+ s"input string '123456' exceeds $typeName type length limitation: 5"))
+ val e2 = intercept[SparkException](sql("INSERT INTO t VALUES (map('a', '123456'))"))
+ assert(e2.getCause.getMessage.contains(
+ s"input string '123456' exceeds $typeName type length limitation: 5"))
+ }
+ }
+
+ test("length check for input string values: nested in struct of array") {
+ testTableWrite { typeName =>
+ sql(s"CREATE TABLE t(c STRUCT<c: ARRAY<$typeName(5)>>) USING $format")
+ sql("INSERT INTO t SELECT struct(array(null))")
+ checkAnswer(spark.table("t"), Row(Row(Seq(null))))
+ val e = intercept[SparkException](sql("INSERT INTO t SELECT struct(array('123456'))"))
+ assert(e.getCause.getMessage.contains(
+ s"input string '123456' exceeds $typeName type length limitation: 5"))
+ }
+ }
+
+ test("length check for input string values: nested in array of struct") {
+ testTableWrite { typeName =>
+ sql(s"CREATE TABLE t(c ARRAY<STRUCT<c: $typeName(5)>>) USING $format")
+ sql("INSERT INTO t VALUES (array(struct(null)))")
+ checkAnswer(spark.table("t"), Row(Seq(Row(null))))
+ val e = intercept[SparkException](sql("INSERT INTO t VALUES (array(struct('123456')))"))
+ assert(e.getCause.getMessage.contains(
+ s"input string '123456' exceeds $typeName type length limitation: 5"))
+ }
+ }
+
+ test("length check for input string values: nested in array of array") {
+ testTableWrite { typeName =>
+ sql(s"CREATE TABLE t(c ARRAY<ARRAY<$typeName(5)>>) USING $format")
+ sql("INSERT INTO t VALUES (array(array(null)))")
+ checkAnswer(spark.table("t"), Row(Seq(Seq(null))))
+ val e = intercept[SparkException](sql("INSERT INTO t VALUES (array(array('123456')))"))
+ assert(e.getCause.getMessage.contains(
+ s"input string '123456' exceeds $typeName type length limitation: 5"))
+ }
+ }
+
+ test("length check for input string values: with trailing spaces") {
+ withTable("t") {
+ sql(s"CREATE TABLE t(c1 CHAR(5), c2 VARCHAR(5)) USING $format")
+ sql("INSERT INTO t VALUES ('12 ', '12 ')")
+ sql("INSERT INTO t VALUES ('1234 ', '1234 ')")
+ checkAnswer(spark.table("t"), Seq(
+ Row("12" + " " * 3, "12 "),
+ Row("1234 ", "1234 ")))
+ }
+ }
+
+ test("length check for input string values: with implicit cast") {
+ withTable("t") {
+ sql(s"CREATE TABLE t(c1 CHAR(5), c2 VARCHAR(5)) USING $format")
+ sql("INSERT INTO t VALUES (1234, 1234)")
+ checkAnswer(spark.table("t"), Row("1234 ", "1234"))
+ val e1 = intercept[SparkException](sql("INSERT INTO t VALUES (123456, 1)"))
+ assert(e1.getCause.getMessage.contains(
+ "input string '123456' exceeds char type length limitation: 5"))
+ val e2 = intercept[SparkException](sql("INSERT INTO t VALUES (1, 123456)"))
+ assert(e2.getCause.getMessage.contains(
+ "input string '123456' exceeds varchar type length limitation: 5"))
+ }
+ }
+
+ private def testConditions(df: DataFrame, conditions: Seq[(String, Boolean)]): Unit = {
+ checkAnswer(df.selectExpr(conditions.map(_._1): _*), Row.fromSeq(conditions.map(_._2)))
+ }
+
+ test("char type comparison: top-level columns") {
+ withTable("t") {
+ sql(s"CREATE TABLE t(c1 CHAR(2), c2 CHAR(5)) USING $format")
+ sql("INSERT INTO t VALUES ('a', 'a')")
+ testConditions(spark.table("t"), Seq(
+ ("c1 = 'a'", true),
+ ("'a' = c1", true),
+ ("c1 = 'a '", true),
+ ("c1 > 'a'", false),
+ ("c1 IN ('a', 'b')", true),
+ ("c1 = c2", true),
+ ("c1 < c2", false),
+ ("c1 IN (c2)", true)))
+ }
+ }
+
+ test("char type comparison: partitioned columns") {
+ withTable("t") {
+ sql(s"CREATE TABLE t(i INT, c1 CHAR(2), c2 CHAR(5)) USING $format PARTITIONED BY (c1, c2)")
+ sql("INSERT INTO t VALUES (1, 'a', 'a')")
+ testConditions(spark.table("t"), Seq(
+ ("c1 = 'a'", true),
+ ("'a' = c1", true),
+ ("c1 = 'a '", true),
+ ("c1 > 'a'", false),
+ ("c1 IN ('a', 'b')", true),
+ ("c1 = c2", true),
+ ("c1 < c2", false),
+ ("c1 IN (c2)", true)))
+ }
+ }
+
+ test("char type comparison: join") {
+ withTable("t1", "t2") {
+ sql(s"CREATE TABLE t1(c CHAR(2)) USING $format")
+ sql(s"CREATE TABLE t2(c CHAR(5)) USING $format")
+ sql("INSERT INTO t1 VALUES ('a')")
+ sql("INSERT INTO t2 VALUES ('a')")
+ checkAnswer(sql("SELECT t1.c FROM t1 JOIN t2 ON t1.c = t2.c"), Row("a "))
+ }
+ }
+
+ test("char type comparison: nested in struct") {
+ withTable("t") {
+ sql(s"CREATE TABLE t(c1 STRUCT<c: CHAR(2)>, c2 STRUCT<c: CHAR(5)>) USING $format")
+ sql("INSERT INTO t VALUES (struct('a'), struct('a'))")
+ testConditions(spark.table("t"), Seq(
+ ("c1 = c2", true),
+ ("c1 < c2", false),
+ ("c1 IN (c2)", true)))
+ }
+ }
+
+ test("char type comparison: nested in array") {
+ withTable("t") {
+ sql(s"CREATE TABLE t(c1 ARRAY<CHAR(2)>, c2 ARRAY<CHAR(5)>) USING $format")
+ sql("INSERT INTO t VALUES (array('a', 'b'), array('a', 'b'))")
+ testConditions(spark.table("t"), Seq(
+ ("c1 = c2", true),
+ ("c1 < c2", false),
+ ("c1 IN (c2)", true)))
+ }
+ }
+
+ test("char type comparison: nested in struct of array") {
+ withTable("t") {
+ sql("CREATE TABLE t(c1 STRUCT<a: ARRAY<CHAR(2)>>, c2 STRUCT<a: ARRAY<CHAR(5)>>) " +
+ s"USING $format")
+ sql("INSERT INTO t VALUES (struct(array('a', 'b')), struct(array('a', 'b')))")
+ testConditions(spark.table("t"), Seq(
+ ("c1 = c2", true),
+ ("c1 < c2", false),
+ ("c1 IN (c2)", true)))
+ }
+ }
+
+ test("char type comparison: nested in array of struct") {
+ withTable("t") {
+ sql("CREATE TABLE t(c1 ARRAY<STRUCT<c: CHAR(2)>>, c2 ARRAY<STRUCT<c: CHAR(5)>>) " +
+ s"USING $format")
+ sql("INSERT INTO t VALUES (array(struct('a')), array(struct('a')))")
+ testConditions(spark.table("t"), Seq(
+ ("c1 = c2", true),
+ ("c1 < c2", false),
+ ("c1 IN (c2)", true)))
+ }
+ }
+
+ test("char type comparison: nested in array of array") {
Review comment:
You are right, I should remove 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] SparkQA commented on pull request #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-734153761
**[Test build #131827 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131827/testReport)** for PR 30412 at commit [`671471f`](https://github.com/apache/spark/commit/671471f9dad6a3cffa4c5442daba32424d9d3c13).
* 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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-734858011
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30412:
URL: https://github.com/apache/spark/pull/30412#discussion_r526632252
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CheckAnalysis.scala
##########
@@ -94,6 +94,10 @@ trait CheckAnalysis extends PredicateHelper {
case p if p.analyzed => // Skip already analyzed sub-plans
+ case leaf: LeafNode if leaf.output.map(_.dataType).exists(CharVarcharUtils.hasCharVarchar) =>
Review comment:
We can not, as the parser that is used to parse the schema string replaces char/varchar with string. This is the existing behavior.
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-732846865
**[Test build #131653 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131653/testReport)** for PR 30412 at commit [`f46e32f`](https://github.com/apache/spark/commit/f46e32fb1649023eed0ddab4cb23ca4a97b14a0f).
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-734076827
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-730586527
**[Test build #131365 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131365/testReport)** for PR 30412 at commit [`c239834`](https://github.com/apache/spark/commit/c239834ef7ce730910d2d3a64f858c2c53dbc6a5).
* This patch **fails Spark unit 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 #30412: [SPARK-33480][SQL] support char/varchar type
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-729964068
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-730493450
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
maropu commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-734681874
okay, it looks good to me. Btw, this new feature will land in the v3.1 release? There are still some remaining works (e.g., https://github.com/apache/spark/pull/30412#discussion_r528884694 and https://github.com/apache/spark/pull/30412#discussion_r529253616) though. cc: @HyukjinKwon because of a v3.1 release manager
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-730424852
**[Test build #131356 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131356/testReport)** for PR 30412 at commit [`17d13e5`](https://github.com/apache/spark/commit/17d13e5944e247826a92403abb134a9dfd5e0cc6).
* 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 removed a comment on pull request #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-733966628
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-732326747
**[Test build #131564 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131564/testReport)** for PR 30412 at commit [`6182921`](https://github.com/apache/spark/commit/6182921657ae0757549d2544088ff4442d8a08a7).
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-732838546
**[Test build #131637 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131637/testReport)** for PR 30412 at commit [`f46e32f`](https://github.com/apache/spark/commit/f46e32fb1649023eed0ddab4cb23ca4a97b14a0f).
* This patch **fails Spark unit 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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-732939815
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-732853983
**[Test build #131654 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131654/testReport)** for PR 30412 at commit [`e5fb41a`](https://github.com/apache/spark/commit/e5fb41aa2aa220b0750c6d6812e34075a9c118b2).
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] support char/varchar type
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-729827025
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/35895/
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-733505877
**[Test build #131716 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131716/testReport)** for PR 30412 at commit [`38999b5`](https://github.com/apache/spark/commit/38999b535e78817d2647d186605618438f438220).
* 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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-733324371
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #30412:
URL: https://github.com/apache/spark/pull/30412#discussion_r530846502
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/Column.scala
##########
@@ -1181,7 +1181,9 @@ class Column(val expr: Expression) extends Logging {
* @group expr_ops
* @since 1.3.0
*/
- def cast(to: DataType): Column = withExpr { Cast(expr, to) }
+ def cast(to: DataType): Column = withExpr {
+ Cast(expr, CharVarcharUtils.replaceCharVarcharWithString(to))
+ }
Review comment:
So we can do `cast(CharType)`? It actually casts to StringType? But don't we loss length info?
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/types/VarcharType.scala
##########
@@ -0,0 +1,37 @@
+/*
+ * 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.types
+
+import scala.math.Ordering
+import scala.reflect.runtime.universe.typeTag
+
+import org.apache.spark.annotation.Experimental
+import org.apache.spark.unsafe.types.UTF8String
+
+@Experimental
+case class VarcharType(length: Int) extends AtomicType {
+ require(length >= 0, "The length if varchar type cannot be negative.")
Review comment:
if -> of
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/types/CharType.scala
##########
@@ -0,0 +1,38 @@
+/*
+ * 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.types
+
+import scala.math.Ordering
+import scala.reflect.runtime.universe.typeTag
+
+import org.apache.spark.annotation.Experimental
+import org.apache.spark.unsafe.types.UTF8String
+
+@Experimental
+case class CharType(length: Int) extends AtomicType {
+ require(length >= 0, "The length if char type cannot be negative.")
Review comment:
if -> of
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/CharVarcharUtils.scala
##########
@@ -0,0 +1,275 @@
+/*
+ * 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 scala.collection.mutable
+
+import org.apache.spark.sql.catalyst.expressions._
+import org.apache.spark.sql.catalyst.parser.CatalystSqlParser
+import org.apache.spark.sql.types._
+
+object CharVarcharUtils {
+
+ private val CHAR_VARCHAR_TYPE_STRING_METADATA_KEY = "__CHAR_VARCHAR_TYPE_STRING"
+
+ /**
+ * Replaces CharType/VarcharType with StringType recursively in the given struct type. If a
+ * top-level StructField's data type is CharType/VarcharType or has nested CharType/VarcharType,
+ * this method will add the original type string to the StructField's metadata, so that we can
+ * re-construct the original data type with CharType/VarcharType later when needed.
+ */
+ def replaceCharVarcharWithStringInSchema(st: StructType): StructType = {
+ StructType(st.map { field =>
+ if (hasCharVarchar(field.dataType)) {
+ val metadata = new MetadataBuilder().withMetadata(field.metadata)
+ .putString(CHAR_VARCHAR_TYPE_STRING_METADATA_KEY, field.dataType.sql).build()
+ field.copy(dataType = replaceCharVarcharWithString(field.dataType), metadata = metadata)
+ } else {
+ field
+ }
+ })
+ }
+
+ /**
+ * Returns true if the given data type is CharType/VarcharType or has nested CharType/VarcharType.
+ */
+ def hasCharVarchar(dt: DataType): Boolean = {
+ dt.existsRecursively(f => f.isInstanceOf[CharType] || f.isInstanceOf[VarcharType])
+ }
+
+ /**
+ * Replaces CharType/VarcharType with StringType recursively in the given data type.
+ */
+ def replaceCharVarcharWithString(dt: DataType): DataType = dt match {
+ case ArrayType(et, nullable) =>
+ ArrayType(replaceCharVarcharWithString(et), nullable)
+ case MapType(kt, vt, nullable) =>
+ MapType(replaceCharVarcharWithString(kt), replaceCharVarcharWithString(vt), nullable)
+ case StructType(fields) =>
+ StructType(fields.map { field =>
+ field.copy(dataType = replaceCharVarcharWithString(field.dataType))
+ })
+ case _: CharType => StringType
+ case _: VarcharType => StringType
+ case _ => dt
+ }
+
+ /**
+ * Removes the metadata entry that contains the original type string of CharType/VarcharType from
+ * the given attribute's metadata.
+ */
+ def cleanAttrMetadata(attr: AttributeReference): AttributeReference = {
+ val cleaned = new MetadataBuilder().withMetadata(attr.metadata)
+ .remove(CHAR_VARCHAR_TYPE_STRING_METADATA_KEY).build()
+ attr.withMetadata(cleaned)
+ }
+
+ /**
+ * Re-construct the original data type from the type string in the given metadata.
+ * This is needed when dealing with char/varchar columns/fields.
+ */
+ def getRawType(metadata: Metadata): Option[DataType] = {
+ if (metadata.contains(CHAR_VARCHAR_TYPE_STRING_METADATA_KEY)) {
+ Some(CatalystSqlParser.parseRawDataType(
+ metadata.getString(CHAR_VARCHAR_TYPE_STRING_METADATA_KEY)))
+ } else {
+ None
+ }
+ }
+
+ /**
+ * Returns expressions to apply read-side char type padding for the given attributes. String
+ * values should be right-padded to N characters if it's from a CHAR(N) column/field.
+ */
+ def charTypePadding(output: Seq[AttributeReference]): Seq[NamedExpression] = {
+ output.map { attr =>
+ getRawType(attr.metadata).filter { rawType =>
+ rawType.existsRecursively(_.isInstanceOf[CharType])
+ }.map { rawType =>
+ Alias(charTypePadding(attr, rawType), attr.name)(explicitMetadata = Some(attr.metadata))
+ }.getOrElse(attr)
+ }
+ }
+
+ private def charTypePadding(expr: Expression, dt: DataType): Expression = dt match {
+ case CharType(length) => StringRPad(expr, Literal(length))
+
+ case StructType(fields) =>
+ val struct = CreateNamedStruct(fields.zipWithIndex.flatMap { case (f, i) =>
+ Seq(Literal(f.name), charTypePadding(GetStructField(expr, i, Some(f.name)), f.dataType))
+ })
+ if (expr.nullable) {
+ If(IsNull(expr), Literal(null, struct.dataType), struct)
+ } else {
+ struct
+ }
+
+ case ArrayType(et, containsNull) => charTypePaddingInArray(expr, et, containsNull)
+
+ case MapType(kt, vt, valueContainsNull) =>
+ val newKeys = charTypePaddingInArray(MapKeys(expr), kt, containsNull = false)
+ val newValues = charTypePaddingInArray(MapValues(expr), vt, valueContainsNull)
+ MapFromArrays(newKeys, newValues)
+
+ case _ => expr
+ }
+
+ private def charTypePaddingInArray(
+ arr: Expression, et: DataType, containsNull: Boolean): Expression = {
+ val param = NamedLambdaVariable("x", replaceCharVarcharWithString(et), containsNull)
+ val func = LambdaFunction(charTypePadding(param, et), Seq(param))
+ ArrayTransform(arr, func)
+ }
+
+ /**
+ * Returns an expression to apply write-side char type padding for the given expression. A string
Review comment:
char type padding -> char type checking?
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-732360156
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/36163/
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-733870653
**[Test build #131797 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131797/testReport)** for PR 30412 at commit [`662f297`](https://github.com/apache/spark/commit/662f2970cc4a87051d1eee67f884ba1ab945885b).
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-732676986
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-733954524
**[Test build #131803 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131803/testReport)** for PR 30412 at commit [`9850d38`](https://github.com/apache/spark/commit/9850d38d09d9545368cd7da8d5d65fc71043eac6).
* This patch **fails Spark unit 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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-730571635
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/35969/
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-730586810
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-734709360
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-733413366
retest this please
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-735902402
Thank you, @cloud-fan and all!
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-733029359
----------------------------------------------------------------
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 a change in pull request #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
maropu commented on a change in pull request #30412:
URL: https://github.com/apache/spark/pull/30412#discussion_r526598186
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CheckAnalysis.scala
##########
@@ -94,6 +94,10 @@ trait CheckAnalysis extends PredicateHelper {
case p if p.analyzed => // Skip already analyzed sub-plans
+ case leaf: LeafNode if leaf.output.map(_.dataType).exists(CharVarcharUtils.hasCharVarchar) =>
Review comment:
> To simplify the implementation, this PR doesn't propagate char/varchar type info through functions/operators(e.g. substring).
For checking it does not propagates char/varchar types, how about checking this in all logical nodes? It seems, in the current implementation, we can pass char/varchar types into logical plans, e.g., via `functions.from_json(col, schema having char/vchar types)`?
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/CharVarcharUtils.scala
##########
@@ -0,0 +1,277 @@
+/*
+ * 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 scala.collection.mutable
+
+import org.apache.spark.sql.catalyst.expressions._
+import org.apache.spark.sql.catalyst.parser.CatalystSqlParser
+import org.apache.spark.sql.types._
+
+object CharVarcharUtils {
+
+ private val CHAR_VARCHAR_TYPE_STRING_METADATA_KEY = "__CHAR_VARCHAR_TYPE_STRING"
+
+ /**
+ * Replaces CharType/VarcharType with StringType recursively in the given struct type. If a
+ * top-level StructField's data type is CharType/VarcharType or has nested CharType/VarcharType,
+ * this method will add the original type string to the StructField's metadata, so that we can
+ * re-construct the original data type with CharType/VarcharType later when needed.
+ */
+ def replaceCharVarcharWithStringInSchema(st: StructType): StructType = {
+ StructType(st.map { field =>
+ if (hasCharVarchar(field.dataType)) {
+ val metadata = new MetadataBuilder().withMetadata(field.metadata)
+ .putString(CHAR_VARCHAR_TYPE_STRING_METADATA_KEY, field.dataType.sql).build()
+ field.copy(dataType = replaceCharVarcharWithString(field.dataType), metadata = metadata)
+ } else {
+ field
+ }
+ })
+ }
+
+ /**
+ * Returns true if the given data type is CharType/VarcharType or has nested CharType/VarcharType.
+ */
+ def hasCharVarchar(dt: DataType): Boolean = {
+ dt.existsRecursively(f => f.isInstanceOf[CharType] || f.isInstanceOf[VarcharType])
+ }
+
+ /**
+ * Replaces CharType/VarcharType with StringType recursively in the given data type.
+ */
+ def replaceCharVarcharWithString(dt: DataType): DataType = dt match {
+ case ArrayType(et, nullable) =>
+ ArrayType(replaceCharVarcharWithString(et), nullable)
+ case MapType(kt, vt, nullable) =>
+ MapType(replaceCharVarcharWithString(kt), replaceCharVarcharWithString(vt), nullable)
+ case StructType(fields) =>
+ StructType(fields.map { field =>
+ field.copy(dataType = replaceCharVarcharWithString(field.dataType))
+ })
+ case _: CharType => StringType
+ case _: VarcharType => StringType
+ case _ => dt
+ }
+
+ /**
+ * Removes the metadata entry that contains the original type string of CharType/VarcharType from
+ * the given attribute's metadata.
+ */
+ def cleanAttrMetadata(attr: AttributeReference): AttributeReference = {
+ val cleaned = new MetadataBuilder().withMetadata(attr.metadata)
+ .remove(CHAR_VARCHAR_TYPE_STRING_METADATA_KEY).build()
+ attr.withMetadata(cleaned)
+ }
+
+ /**
+ * Re-construct the original data type from the type string in the given metadata.
+ * This is needed when dealing with char/varchar columns/fields.
+ */
+ def getRawType(metadata: Metadata): Option[DataType] = {
+ if (metadata.contains(CHAR_VARCHAR_TYPE_STRING_METADATA_KEY)) {
+ Some(CatalystSqlParser.parseRawDataType(
+ metadata.getString(CHAR_VARCHAR_TYPE_STRING_METADATA_KEY)))
+ } else {
+ None
+ }
+ }
+
+ /**
+ * Re-construct the original StructType from the type strings in the metadata of StructFields.
+ * This is needed when dealing with char/varchar columns/fields.
+ */
+ def getRawSchema(schema: StructType): StructType = {
+ StructType(schema.map { field =>
+ getRawType(field.metadata).map(rawType => field.copy(dataType = rawType)).getOrElse(field)
+ })
+ }
+
+ /**
+ * Returns expressions to apply read-side char type padding for the given attributes. String
+ * values should be right-padded to N characters if it's from a CHAR(N) column/field.
+ */
+ def charTypePadding(output: Seq[AttributeReference]): Seq[NamedExpression] = {
+ output.map { attr =>
+ getRawType(attr.metadata).filter { rawType =>
+ rawType.existsRecursively(_.isInstanceOf[CharType])
+ }.map { rawType =>
+ Alias(charTypePadding(attr, rawType), attr.name)(explicitMetadata = Some(attr.metadata))
+ }.getOrElse(attr)
+ }
+ }
+
+ private def charTypePadding(expr: Expression, dt: DataType): Expression = dt match {
+ case CharType(length) => StringRPad(expr, Literal(length))
+
+ case StructType(fields) =>
+ CreateNamedStruct(fields.zipWithIndex.flatMap { case (f, i) =>
+ Seq(Literal(f.name), charTypePadding(GetStructField(expr, i, Some(f.name)), f.dataType))
+ })
+
+ case ArrayType(et, containsNull) => charTypePaddingInArray(expr, et, containsNull)
+
+ case MapType(kt, vt, valueContainsNull) =>
+ val newKeys = charTypePaddingInArray(MapKeys(expr), kt, containsNull = false)
+ val newValues = charTypePaddingInArray(MapValues(expr), vt, valueContainsNull)
+ MapFromArrays(newKeys, newValues)
+
+ case _ => expr
+ }
+
+ private def charTypePaddingInArray(
+ arr: Expression, et: DataType, containsNull: Boolean): Expression = {
+ val param = NamedLambdaVariable("x", replaceCharVarcharWithString(et), containsNull)
+ val func = LambdaFunction(charTypePadding(param, et), Seq(param))
+ ArrayTransform(arr, func)
+ }
+
+ /**
+ * Returns an expression to apply write-side char type padding for the given expression. A string
+ * value can not exceed N characters if it's written into a CHAR(N)/VARCHAR(N) column/field.
+ */
+ def stringLengthCheck(expr: Expression, targetAttr: Attribute): Expression = {
+ getRawType(targetAttr.metadata).map { rawType =>
+ stringLengthCheck(expr, rawType)
+ }.getOrElse(expr)
+ }
+
+ private def stringLengthCheck(expr: Expression, dt: DataType): Expression = dt match {
+ case CharType(length) =>
+ val trimmed = StringTrimRight(expr)
+ val errorMsg = Concat(Seq(
Review comment:
nit: How about extract `errorMsg` as a private method?
```
private def raiseError(expr: Expression, typeName: String, length: Int): Expression = {
val errorMsg = Concat(Seq(
Literal("input string '"),
expr,
Literal(s"' exceeds $typeName type length limitation: $length")))
Cast(RaiseError(errorMsg), StringType)
}
private def stringLengthCheck(expr: Expression, dt: DataType): Expression = dt match {
case CharType(length) =>
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(expr, "char", length),
trimmed)
...
```
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala
##########
@@ -3088,7 +3088,12 @@ class Analyzer(override val catalogManager: CatalogManager)
val projection = TableOutputResolver.resolveOutputColumns(
v2Write.table.name, v2Write.table.output, v2Write.query, v2Write.isByName, conf)
if (projection != v2Write.query) {
- v2Write.withNewQuery(projection)
+ val cleanedTable = v2Write.table match {
+ case r: DataSourceV2Relation =>
+ r.copy(output = r.output.map(CharVarcharUtils.cleanAttrMetadata))
Review comment:
Does the current implementation assume the analyzer removes the metadata in plans before the optimizer phase? If so, how about checking if plans don't have the metadata in `CheckAnalysis`?
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/TableOutputResolver.scala
##########
@@ -93,19 +94,17 @@ object TableOutputResolver {
tableAttr.metadata == queryExpr.metadata) {
Some(queryExpr)
} else {
- // Renaming is needed for handling the following cases like
- // 1) Column names/types do not match, e.g., INSERT INTO TABLE tab1 SELECT 1, 2
- // 2) Target tables have column metadata
- storeAssignmentPolicy match {
+ val casted = storeAssignmentPolicy match {
case StoreAssignmentPolicy.ANSI =>
- Some(Alias(
- AnsiCast(queryExpr, tableAttr.dataType, Option(conf.sessionLocalTimeZone)),
- tableAttr.name)(explicitMetadata = Option(tableAttr.metadata)))
+ AnsiCast(queryExpr, tableAttr.dataType, Option(conf.sessionLocalTimeZone))
case _ =>
- Some(Alias(
- Cast(queryExpr, tableAttr.dataType, Option(conf.sessionLocalTimeZone)),
- tableAttr.name)(explicitMetadata = Option(tableAttr.metadata)))
+ Cast(queryExpr, tableAttr.dataType, Option(conf.sessionLocalTimeZone))
}
+ val strLenChecked = CharVarcharUtils.stringLengthCheck(casted, tableAttr)
Review comment:
To avoid accidentally adding the length check exprs again, we cannot remove the metadata at the same time as adding the exprs?
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala
##########
@@ -473,7 +473,13 @@ class SessionCatalog(
val table = formatTableName(name.table)
requireDbExists(db)
requireTableExists(TableIdentifier(table, Some(db)))
- externalCatalog.getTable(db, table)
+ removeCharVarcharFromTableSchema(externalCatalog.getTable(db, table))
+ }
+
+ // We replace char/varchar with string type in the table schema, as Spark's type system doesn't
+ // support char/varchar yet.
+ private def removeCharVarcharFromTableSchema(t: CatalogTable): CatalogTable = {
Review comment:
Could you inline this in L476?
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/types/VarcharType.scala
##########
@@ -0,0 +1,35 @@
+/*
+ * 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.types
+
+import scala.math.Ordering
+import scala.reflect.runtime.universe.typeTag
+
+import org.apache.spark.annotation.Experimental
+import org.apache.spark.unsafe.types.UTF8String
+
+@Experimental
+case class VarcharType(length: Int) extends AtomicType {
Review comment:
Also, how about setting a reasonable max length for these types like postgresql does so?
```
postgres=# create table t (v char(100000000));
ERROR: length for type char cannot exceed 10485760
LINE 1: create table t (v char(100000000));
```
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/types/VarcharType.scala
##########
@@ -0,0 +1,35 @@
+/*
+ * 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.types
+
+import scala.math.Ordering
+import scala.reflect.runtime.universe.typeTag
+
+import org.apache.spark.annotation.Experimental
+import org.apache.spark.unsafe.types.UTF8String
+
+@Experimental
+case class VarcharType(length: Int) extends AtomicType {
Review comment:
How about checking if `length` has a valid number in the constructor of char/vchar?
```
if (length > 0) {
throw new AnalysisException("XXX")
}
```
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/TableOutputResolver.scala
##########
@@ -93,19 +94,17 @@ object TableOutputResolver {
tableAttr.metadata == queryExpr.metadata) {
Some(queryExpr)
} else {
- // Renaming is needed for handling the following cases like
- // 1) Column names/types do not match, e.g., INSERT INTO TABLE tab1 SELECT 1, 2
- // 2) Target tables have column metadata
- storeAssignmentPolicy match {
+ val casted = storeAssignmentPolicy match {
case StoreAssignmentPolicy.ANSI =>
- Some(Alias(
- AnsiCast(queryExpr, tableAttr.dataType, Option(conf.sessionLocalTimeZone)),
- tableAttr.name)(explicitMetadata = Option(tableAttr.metadata)))
+ AnsiCast(queryExpr, tableAttr.dataType, Option(conf.sessionLocalTimeZone))
case _ =>
- Some(Alias(
- Cast(queryExpr, tableAttr.dataType, Option(conf.sessionLocalTimeZone)),
- tableAttr.name)(explicitMetadata = Option(tableAttr.metadata)))
+ Cast(queryExpr, tableAttr.dataType, Option(conf.sessionLocalTimeZone))
}
+ val strLenChecked = CharVarcharUtils.stringLengthCheck(casted, tableAttr)
Review comment:
nit: `strLenChecked` -> `exprWithStrLenCheck`?
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-733265378
----------------------------------------------------------------
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 a change in pull request #30412: [SPARK-33480][SQL] support char/varchar type
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #30412:
URL: https://github.com/apache/spark/pull/30412#discussion_r526254628
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/execution/datasources/v2/DataSourceV2Relation.scala
##########
@@ -147,8 +147,10 @@ object DataSourceV2Relation {
catalog: Option[CatalogPlugin],
identifier: Option[Identifier],
options: CaseInsensitiveStringMap): DataSourceV2Relation = {
- val output = table.schema().toAttributes
- DataSourceV2Relation(table, output, catalog, identifier, options)
+ // The v2 source may return schema containing char/varchar type. We replace char/varchar
+ // with string type here as Spark's type system doesn't support char/varchar yet.
Review comment:
`with string type` -> `with string type with metadata`?
----------------------------------------------------------------
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] ulysses-you commented on a change in pull request #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
ulysses-you commented on a change in pull request #30412:
URL: https://github.com/apache/spark/pull/30412#discussion_r529097018
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/types/DataType.scala
##########
@@ -124,13 +125,16 @@ abstract class DataType extends AbstractDataType {
object DataType {
private val FIXED_DECIMAL = """decimal\(\s*(\d+)\s*,\s*(\-?\d+)\s*\)""".r
+ private val CHAR_TYPE = """char\(\s*(\d+)\s*\)""".r
+ private val VARCHAR_TYPE = """varchar\(\s*(\d+)\s*\)""".r
Review comment:
A question, do we need to support default length if user not give a special value ?
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-733110220
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-730424884
Merged build finished. Test FAILed.
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30412:
URL: https://github.com/apache/spark/pull/30412#discussion_r528880969
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/CharVarcharTestSuite.scala
##########
@@ -0,0 +1,374 @@
+/*
+ * 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
+
+import org.apache.spark.{SparkConf, SparkException}
+import org.apache.spark.sql.connector.InMemoryTableCatalog
+import org.apache.spark.sql.internal.SQLConf
+import org.apache.spark.sql.test.{SharedSparkSession, SQLTestUtils}
+
+trait CharVarcharTestSuite extends QueryTest with SQLTestUtils {
+
+ def format: String
+
+ test("char type values should be padded: top-level columns") {
+ withTable("t") {
+ sql(s"CREATE TABLE t(i STRING, c CHAR(5)) USING $format")
+ sql("INSERT INTO t VALUES ('1', 'a')")
+ checkAnswer(spark.table("t"), Row("1", "a" + " " * 4))
+ }
+ }
+
+ test("char type values should be padded: partitioned columns") {
+ // DS V2 doesn't support partitioned table.
+ if (!conf.contains(SQLConf.DEFAULT_CATALOG.key)) {
+ withTable("t") {
+ sql(s"CREATE TABLE t(i STRING, c CHAR(5)) USING $format PARTITIONED BY (c)")
+ sql("INSERT INTO t VALUES ('1', 'a')")
+ checkAnswer(spark.table("t"), Row("1", "a" + " " * 4))
+ }
+ }
+ }
+
+ test("char type values should be padded: nested in struct") {
+ withTable("t") {
+ sql(s"CREATE TABLE t(i STRING, c STRUCT<c: CHAR(5)>) USING $format")
+ sql("INSERT INTO t VALUES ('1', struct('a'))")
+ checkAnswer(spark.table("t"), Row("1", Row("a" + " " * 4)))
+ }
+ }
+
+ test("char type values should be padded: nested in array") {
+ withTable("t") {
+ sql(s"CREATE TABLE t(i STRING, c ARRAY<CHAR(5)>) USING $format")
+ sql("INSERT INTO t VALUES ('1', array('a', 'ab'))")
+ checkAnswer(spark.table("t"), Row("1", Seq("a" + " " * 4, "ab" + " " * 3)))
+ }
+ }
+
+ test("char type values should be padded: nested in map key") {
+ withTable("t") {
+ sql(s"CREATE TABLE t(i STRING, c MAP<CHAR(5), STRING>) USING $format")
+ sql("INSERT INTO t VALUES ('1', map('a', 'ab'))")
+ checkAnswer(spark.table("t"), Row("1", Map(("a" + " " * 4, "ab"))))
+ }
+ }
+
+ test("char type values should be padded: nested in map value") {
+ withTable("t") {
+ sql(s"CREATE TABLE t(i STRING, c MAP<STRING, CHAR(5)>) USING $format")
+ sql("INSERT INTO t VALUES ('1', map('a', 'ab'))")
+ checkAnswer(spark.table("t"), Row("1", Map(("a", "ab" + " " * 3))))
+ }
+ }
+
+ test("char type values should be padded: nested in both map key and value") {
+ withTable("t") {
+ sql(s"CREATE TABLE t(i STRING, c MAP<CHAR(5), CHAR(10)>) USING $format")
+ sql("INSERT INTO t VALUES ('1', map('a', 'ab'))")
+ checkAnswer(spark.table("t"), Row("1", Map(("a" + " " * 4, "ab" + " " * 8))))
+ }
+ }
+
+ test("char type values should be padded: nested in struct of array") {
+ withTable("t") {
+ sql(s"CREATE TABLE t(i STRING, c STRUCT<c: ARRAY<CHAR(5)>>) USING $format")
+ sql("INSERT INTO t VALUES ('1', struct(array('a', 'ab')))")
+ checkAnswer(spark.table("t"), Row("1", Row(Seq("a" + " " * 4, "ab" + " " * 3))))
+ }
+ }
+
+ test("char type values should be padded: nested in array of struct") {
+ withTable("t") {
+ sql(s"CREATE TABLE t(i STRING, c ARRAY<STRUCT<c: CHAR(5)>>) USING $format")
+ sql("INSERT INTO t VALUES ('1', array(struct('a'), struct('ab')))")
+ checkAnswer(spark.table("t"), Row("1", Seq(Row("a" + " " * 4), Row("ab" + " " * 3))))
+ }
+ }
+
+ test("char type values should be padded: nested in array of array") {
+ withTable("t") {
+ sql(s"CREATE TABLE t(i STRING, c ARRAY<ARRAY<CHAR(5)>>) USING $format")
+ sql("INSERT INTO t VALUES ('1', array(array('a', 'ab')))")
+ checkAnswer(spark.table("t"), Row("1", Seq(Seq("a" + " " * 4, "ab" + " " * 3))))
+ }
+ }
+
+ private def testTableWrite(f: String => Unit): Unit = {
+ withTable("t") { f("char") }
+ withTable("t") { f("varchar") }
+ }
+
+ test("length check for input string values: top-level columns") {
+ testTableWrite { typeName =>
+ sql(s"CREATE TABLE t(c $typeName(5)) USING $format")
+ sql("INSERT INTO t VALUES (null)")
+ checkAnswer(spark.table("t"), Row(null))
+ val e = intercept[SparkException](sql("INSERT INTO t VALUES ('123456')"))
+ assert(e.getCause.getMessage.contains(
+ s"input string '123456' exceeds $typeName type length limitation: 5"))
+ }
+ }
+
+ test("length check for input string values: partitioned columns") {
+ // DS V2 doesn't support partitioned table.
+ if (!conf.contains(SQLConf.DEFAULT_CATALOG.key)) {
+ testTableWrite { typeName =>
+ sql(s"CREATE TABLE t(i INT, c $typeName(5)) USING $format PARTITIONED BY (c)")
+ sql("INSERT INTO t VALUES (1, null)")
+ checkAnswer(spark.table("t"), Row(1, null))
+ val e = intercept[SparkException](sql("INSERT INTO t VALUES (1, '123456')"))
+ assert(e.getCause.getMessage.contains(
+ s"input string '123456' exceeds $typeName type length limitation: 5"))
+ }
+ }
+ }
+
+ test("length check for input string values: nested in struct") {
+ testTableWrite { typeName =>
+ sql(s"CREATE TABLE t(c STRUCT<c: $typeName(5)>) USING $format")
+ sql("INSERT INTO t SELECT struct(null)")
+ checkAnswer(spark.table("t"), Row(Row(null)))
+ val e = intercept[SparkException](sql("INSERT INTO t SELECT struct('123456')"))
+ assert(e.getCause.getMessage.contains(
+ s"input string '123456' exceeds $typeName type length limitation: 5"))
+ }
+ }
+
+ test("length check for input string values: nested in array") {
+ testTableWrite { typeName =>
+ sql(s"CREATE TABLE t(c ARRAY<$typeName(5)>) USING $format")
+ sql("INSERT INTO t VALUES (array(null))")
+ checkAnswer(spark.table("t"), Row(Seq(null)))
+ val e = intercept[SparkException](sql("INSERT INTO t VALUES (array('a', '123456'))"))
+ assert(e.getCause.getMessage.contains(
+ s"input string '123456' exceeds $typeName type length limitation: 5"))
+ }
+ }
+
+ test("length check for input string values: nested in map key") {
+ testTableWrite { typeName =>
+ sql(s"CREATE TABLE t(c MAP<$typeName(5), STRING>) USING $format")
+ val e = intercept[SparkException](sql("INSERT INTO t VALUES (map('123456', 'a'))"))
+ assert(e.getCause.getMessage.contains(
+ s"input string '123456' exceeds $typeName type length limitation: 5"))
+ }
+ }
+
+ test("length check for input string values: nested in map value") {
+ testTableWrite { typeName =>
+ sql(s"CREATE TABLE t(c MAP<STRING, $typeName(5)>) USING $format")
+ sql("INSERT INTO t VALUES (map('a', null))")
+ checkAnswer(spark.table("t"), Row(Map("a" -> null)))
+ val e = intercept[SparkException](sql("INSERT INTO t VALUES (map('a', '123456'))"))
+ assert(e.getCause.getMessage.contains(
+ s"input string '123456' exceeds $typeName type length limitation: 5"))
+ }
+ }
+
+ test("length check for input string values: nested in both map key and value") {
+ testTableWrite { typeName =>
+ sql(s"CREATE TABLE t(c MAP<$typeName(5), $typeName(5)>) USING $format")
+ val e1 = intercept[SparkException](sql("INSERT INTO t VALUES (map('123456', 'a'))"))
+ assert(e1.getCause.getMessage.contains(
+ s"input string '123456' exceeds $typeName type length limitation: 5"))
+ val e2 = intercept[SparkException](sql("INSERT INTO t VALUES (map('a', '123456'))"))
+ assert(e2.getCause.getMessage.contains(
+ s"input string '123456' exceeds $typeName type length limitation: 5"))
+ }
+ }
+
+ test("length check for input string values: nested in struct of array") {
+ testTableWrite { typeName =>
+ sql(s"CREATE TABLE t(c STRUCT<c: ARRAY<$typeName(5)>>) USING $format")
+ sql("INSERT INTO t SELECT struct(array(null))")
+ checkAnswer(spark.table("t"), Row(Row(Seq(null))))
+ val e = intercept[SparkException](sql("INSERT INTO t SELECT struct(array('123456'))"))
+ assert(e.getCause.getMessage.contains(
+ s"input string '123456' exceeds $typeName type length limitation: 5"))
+ }
+ }
+
+ test("length check for input string values: nested in array of struct") {
+ testTableWrite { typeName =>
+ sql(s"CREATE TABLE t(c ARRAY<STRUCT<c: $typeName(5)>>) USING $format")
+ sql("INSERT INTO t VALUES (array(struct(null)))")
+ checkAnswer(spark.table("t"), Row(Seq(Row(null))))
+ val e = intercept[SparkException](sql("INSERT INTO t VALUES (array(struct('123456')))"))
+ assert(e.getCause.getMessage.contains(
+ s"input string '123456' exceeds $typeName type length limitation: 5"))
+ }
+ }
+
+ test("length check for input string values: nested in array of array") {
+ testTableWrite { typeName =>
+ sql(s"CREATE TABLE t(c ARRAY<ARRAY<$typeName(5)>>) USING $format")
+ sql("INSERT INTO t VALUES (array(array(null)))")
+ checkAnswer(spark.table("t"), Row(Seq(Seq(null))))
+ val e = intercept[SparkException](sql("INSERT INTO t VALUES (array(array('123456')))"))
+ assert(e.getCause.getMessage.contains(
+ s"input string '123456' exceeds $typeName type length limitation: 5"))
+ }
+ }
+
+ test("length check for input string values: with trailing spaces") {
+ withTable("t") {
+ sql(s"CREATE TABLE t(c1 CHAR(5), c2 VARCHAR(5)) USING $format")
+ sql("INSERT INTO t VALUES ('12 ', '12 ')")
+ sql("INSERT INTO t VALUES ('1234 ', '1234 ')")
+ checkAnswer(spark.table("t"), Seq(
+ Row("12" + " " * 3, "12 "),
+ Row("1234 ", "1234 ")))
+ }
+ }
+
+ test("length check for input string values: with implicit cast") {
+ withTable("t") {
+ sql(s"CREATE TABLE t(c1 CHAR(5), c2 VARCHAR(5)) USING $format")
+ sql("INSERT INTO t VALUES (1234, 1234)")
+ checkAnswer(spark.table("t"), Row("1234 ", "1234"))
+ val e1 = intercept[SparkException](sql("INSERT INTO t VALUES (123456, 1)"))
+ assert(e1.getCause.getMessage.contains(
+ "input string '123456' exceeds char type length limitation: 5"))
+ val e2 = intercept[SparkException](sql("INSERT INTO t VALUES (1, 123456)"))
+ assert(e2.getCause.getMessage.contains(
+ "input string '123456' exceeds varchar type length limitation: 5"))
+ }
+ }
+
+ private def testConditions(df: DataFrame, conditions: Seq[(String, Boolean)]): Unit = {
+ checkAnswer(df.selectExpr(conditions.map(_._1): _*), Row.fromSeq(conditions.map(_._2)))
+ }
+
+ test("char type comparison: top-level columns") {
+ withTable("t") {
+ sql(s"CREATE TABLE t(c1 CHAR(2), c2 CHAR(5)) USING $format")
+ sql("INSERT INTO t VALUES ('a', 'a')")
+ testConditions(spark.table("t"), Seq(
+ ("c1 = 'a'", true),
+ ("'a' = c1", true),
+ ("c1 = 'a '", true),
+ ("c1 > 'a'", false),
+ ("c1 IN ('a', 'b')", true),
+ ("c1 = c2", true),
+ ("c1 < c2", false),
+ ("c1 IN (c2)", true)))
+ }
+ }
+
+ test("char type comparison: partitioned columns") {
+ withTable("t") {
+ sql(s"CREATE TABLE t(i INT, c1 CHAR(2), c2 CHAR(5)) USING $format PARTITIONED BY (c1, c2)")
+ sql("INSERT INTO t VALUES (1, 'a', 'a')")
+ testConditions(spark.table("t"), Seq(
+ ("c1 = 'a'", true),
+ ("'a' = c1", true),
+ ("c1 = 'a '", true),
+ ("c1 > 'a'", false),
+ ("c1 IN ('a', 'b')", true),
+ ("c1 = c2", true),
+ ("c1 < c2", false),
+ ("c1 IN (c2)", true)))
+ }
+ }
+
+ test("char type comparison: join") {
+ withTable("t1", "t2") {
+ sql(s"CREATE TABLE t1(c CHAR(2)) USING $format")
+ sql(s"CREATE TABLE t2(c CHAR(5)) USING $format")
+ sql("INSERT INTO t1 VALUES ('a')")
+ sql("INSERT INTO t2 VALUES ('a')")
+ checkAnswer(sql("SELECT t1.c FROM t1 JOIN t2 ON t1.c = t2.c"), Row("a "))
+ }
+ }
+
+ test("char type comparison: nested in struct") {
+ withTable("t") {
+ sql(s"CREATE TABLE t(c1 STRUCT<c: CHAR(2)>, c2 STRUCT<c: CHAR(5)>) USING $format")
+ sql("INSERT INTO t VALUES (struct('a'), struct('a'))")
+ testConditions(spark.table("t"), Seq(
+ ("c1 = c2", true),
+ ("c1 < c2", false),
+ ("c1 IN (c2)", true)))
+ }
+ }
+
+ test("char type comparison: nested in array") {
+ withTable("t") {
+ sql(s"CREATE TABLE t(c1 ARRAY<CHAR(2)>, c2 ARRAY<CHAR(5)>) USING $format")
+ sql("INSERT INTO t VALUES (array('a', 'b'), array('a', 'b'))")
+ testConditions(spark.table("t"), Seq(
+ ("c1 = c2", true),
+ ("c1 < c2", false),
+ ("c1 IN (c2)", true)))
+ }
+ }
+
+ test("char type comparison: nested in struct of array") {
+ withTable("t") {
+ sql("CREATE TABLE t(c1 STRUCT<a: ARRAY<CHAR(2)>>, c2 STRUCT<a: ARRAY<CHAR(5)>>) " +
+ s"USING $format")
+ sql("INSERT INTO t VALUES (struct(array('a', 'b')), struct(array('a', 'b')))")
+ testConditions(spark.table("t"), Seq(
+ ("c1 = c2", true),
+ ("c1 < c2", false),
+ ("c1 IN (c2)", true)))
+ }
+ }
+
+ test("char type comparison: nested in array of struct") {
+ withTable("t") {
+ sql("CREATE TABLE t(c1 ARRAY<STRUCT<c: CHAR(2)>>, c2 ARRAY<STRUCT<c: CHAR(5)>>) " +
+ s"USING $format")
+ sql("INSERT INTO t VALUES (array(struct('a')), array(struct('a')))")
+ testConditions(spark.table("t"), Seq(
+ ("c1 = c2", true),
+ ("c1 < c2", false),
+ ("c1 IN (c2)", true)))
+ }
+ }
+
+ test("char type comparison: nested in array of array") {
Review comment:
map type is not comparable.
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-733034804
**[Test build #131674 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131674/testReport)** for PR 30412 at commit [`38999b5`](https://github.com/apache/spark/commit/38999b535e78817d2647d186605618438f438220).
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
cloud-fan closed pull request #30412:
URL: https://github.com/apache/spark/pull/30412
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-734930827
----------------------------------------------------------------
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 #30412: [SPARK-33480][SQL] Support char/varchar type
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30412:
URL: https://github.com/apache/spark/pull/30412#issuecomment-734783592
**[Test build #131881 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131881/testReport)** for PR 30412 at commit [`73b99dc`](https://github.com/apache/spark/commit/73b99dc7a98f5d0673d309adba457cd19144be92).
----------------------------------------------------------------
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