cloud-fan commented on code in PR #58581:
URL: https://github.com/apache/spark/pull/58581#discussion_r4040155848


##########
python/pyspark/sql/connect/session.py:
##########
@@ -608,9 +652,13 @@ def createDataFrame(
             spark_types: List[Optional[DataType]]
             if isinstance(schema, StructType):
                 deduped_schema = cast(StructType, 
_deduplicate_field_names(schema))
-                spark_types = [field.dataType for field in 
deduped_schema.fields]
+                arrow_compatible_schema = cast(

Review Comment:
   Agreed. The pandas and pyarrow.Table lowering changes are gone, so 
first-class CHAR/VARCHAR support for those non-row inputs is outside this 
row-based PR and should remain follow-up work.
   
   <!-- SPARK_DEV_REVIEW_REPLY 
{"feedback_id":"inline:4037004993","thread_id":"inline:4037004993","verdict_sha256":"97b99468d98c3679edc8479bbc2a570c93408cead229f9aa41bdc01830abc558"}
 -->



##########
python/pyspark/sql/tests/connect/test_connect_basic.py:
##########
@@ -491,17 +514,246 @@ def test_schema(self):
         self._check_print_schema(query)
 
     def test_char_varchar_result_schema(self):
-        # SPARK-58794: Python Connect maps first-class CHAR/VARCHAR the same 
as classic.
-        query = "SELECT CAST('ab' AS CHAR(4)) AS c, CAST('cd' AS VARCHAR(6)) 
AS v"
+        # SPARK-59276: Python Connect maps first-class CHAR/VARCHAR the same 
as classic.
+        query = """
+            SELECT
+              CAST('ab' AS CHAR(4)) AS c,
+              CAST('cd' AS VARCHAR(6)) AS v,
+              CAST('ef' AS CHAR(4) COLLATE UTF8_LCASE) AS collated_c,
+              CAST('gh' AS VARCHAR(6) COLLATE UNICODE_CI) AS collated_v
+        """
         conf = {"spark.sql.charVarchar.standardSemantics.enabled": "true"}
         with self.both_conf(conf):
             classic_df = self.spark.sql(query)
             connect_df = self.connect.sql(query)
             self.assertEqual(classic_df.schema, connect_df.schema)
             self.assertEqual(classic_df.schema["c"].dataType, CharType(4))
             self.assertEqual(classic_df.schema["v"].dataType, VarcharType(6))
+            self.assertEqual(classic_df.schema["collated_c"].dataType, 
CharType(4, "UTF8_LCASE"))
+            self.assertEqual(classic_df.schema["collated_v"].dataType, 
VarcharType(6, "UNICODE_CI"))
             self.assertEqual(classic_df.collect(), connect_df.collect())
-            self.assertEqual(connect_df.collect(), [Row(c="ab  ", v="cd")])
+            self.assertEqual(
+                connect_df.collect(),
+                [Row(c="ab  ", v="cd", collated_c="ef  ", collated_v="gh")],
+            )
+
+    def test_create_dataframe_with_char_varchar_schema(self):
+        schema = StructType(
+            [
+                StructField("c", CharType(4)),
+                StructField("explicit_c", CharType(4, "UTF8_BINARY")),
+                StructField("v", VarcharType(3, "UTF8_LCASE")),
+                StructField(
+                    "nested",
+                    StructType(
+                        [
+                            StructField("c", CharType(3, "UTF8_BINARY")),
+                            StructField(
+                                "values",
+                                ArrayType(VarcharType(4, "UNICODE_CI"), 
containsNull=False),
+                            ),
+                            StructField(
+                                "lookup",
+                                MapType(
+                                    CharType(3, "UTF8_LCASE"),
+                                    VarcharType(4, "UTF8_BINARY"),
+                                    valueContainsNull=False,
+                                ),
+                            ),
+                        ]
+                    ),
+                ),
+            ]
+        )
+        rows = [
+            (
+                "ab",
+                "cd",
+                "ef",
+                Row(c="x", values=["gh", "ij"], lookup={"k": "lm"}),
+            )
+        ]
+        standard_conf = {
+            "spark.sql.charVarchar.standardSemantics.enabled": "true",
+            "spark.sql.legacy.charVarcharAsString": "false",
+        }
+        with self.both_conf(standard_conf):
+            df = self.connect.createDataFrame(rows, schema)
+            self.assertEqual(df.schema, schema)
+            self.assertEqual(
+                df.collect(),
+                [
+                    Row(
+                        c="ab  ",
+                        explicit_c="cd  ",
+                        v="ef",
+                        nested=Row(
+                            c="x  ",
+                            values=["gh", "ij"],
+                            lookup={"k  ": "lm"},
+                        ),
+                    )
+                ],
+            )
+            empty = self.connect.createDataFrame([], schema)
+            self.assertEqual(empty.schema, schema)
+            self.assertEqual(empty.collect(), [])
+
+        legacy_conf = {
+            "spark.sql.charVarchar.standardSemantics.enabled": "false",
+            "spark.sql.legacy.charVarcharAsString": "true",
+        }
+        with self.both_conf(legacy_conf):
+            expected = StructType(
+                [
+                    StructField("c", StringType()),
+                    StructField("explicit_c", StringType("UTF8_BINARY")),
+                    StructField("v", StringType("UTF8_LCASE")),
+                    StructField(
+                        "nested",
+                        StructType(
+                            [
+                                StructField("c", StringType("UTF8_BINARY")),
+                                StructField(
+                                    "values",
+                                    ArrayType(StringType("UNICODE_CI"), 
containsNull=False),
+                                ),
+                                StructField(
+                                    "lookup",
+                                    MapType(
+                                        StringType("UTF8_LCASE"),
+                                        StringType("UTF8_BINARY"),
+                                        valueContainsNull=False,
+                                    ),
+                                ),
+                            ]
+                        ),
+                    ),
+                ]
+            )
+            df = self.connect.createDataFrame(rows, schema)
+            self.assertEqual(df.schema, expected)
+            self.assertEqual(
+                df.collect(),
+                [
+                    Row(
+                        c="ab",
+                        explicit_c="cd",
+                        v="ef",
+                        nested=Row(
+                            c="x",
+                            values=["gh", "ij"],
+                            lookup={"k": "lm"},
+                        ),
+                    )
+                ],
+            )
+            empty = self.connect.createDataFrame([], schema)
+            self.assertEqual(empty.schema, expected)
+            self.assertEqual(empty.collect(), [])
+
+        default_conf = {
+            "spark.sql.charVarchar.standardSemantics.enabled": "false",
+            "spark.sql.legacy.charVarcharAsString": "false",
+        }
+        with self.both_conf(default_conf):
+            for data in (rows, []):
+                with self.assertRaises(AnalysisException) as ctx:
+                    self.connect.createDataFrame(data, schema).schema
+                self.check_error(
+                    exception=ctx.exception,
+                    errorClass="UNSUPPORTED_CHAR_OR_VARCHAR_AS_STRING",
+                )
+
+    def test_create_dataframe_with_udt_char_schema(self):
+        schema = StructType([StructField("value", CharValueUDT())])
+        rows = [(CharValue("ab"),)]
+
+        standard_conf = {
+            "spark.sql.charVarchar.standardSemantics.enabled": "true",
+            "spark.sql.legacy.charVarcharAsString": "false",
+        }
+        with self.both_conf(standard_conf):
+            populated = self.connect.createDataFrame(rows, schema)
+            self.assertEqual(populated.schema, schema)
+            self.assertEqual(populated.collect(), [Row(value=CharValue("ab  
"))])
+            self.assertEqual(self.connect.createDataFrame([], schema).schema, 
schema)
+
+        legacy_conf = {
+            "spark.sql.charVarchar.standardSemantics.enabled": "false",
+            "spark.sql.legacy.charVarcharAsString": "true",
+        }
+        with self.both_conf(legacy_conf):
+            expected = StructType([StructField("value", 
StringType("UTF8_LCASE"))])
+            populated = self.connect.createDataFrame(rows, schema)
+            self.assertEqual(populated.schema, expected)
+            self.assertEqual(populated.collect(), [Row(value="ab")])
+            self.assertEqual(self.connect.createDataFrame([], schema).schema, 
expected)
+
+        default_conf = {
+            "spark.sql.charVarchar.standardSemantics.enabled": "false",
+            "spark.sql.legacy.charVarcharAsString": "false",
+        }
+        with self.both_conf(default_conf):
+            for data in (rows, []):
+                with self.assertRaises(AnalysisException) as ctx:
+                    self.connect.createDataFrame(data, schema).schema
+                self.check_error(
+                    exception=ctx.exception,
+                    errorClass="UNSUPPORTED_CHAR_OR_VARCHAR_AS_STRING",
+                )
+
+    def test_create_dataframe_with_explicit_binary_string_schema(self):
+        explicit_binary = StringType("UTF8_BINARY")
+        schema = StructType(
+            [
+                StructField("implicit", StringType()),
+                StructField("s", explicit_binary),
+                StructField(
+                    "nested",
+                    StructType(
+                        [
+                            StructField("s", explicit_binary),
+                            StructField("a", ArrayType(explicit_binary)),
+                            StructField(
+                                "m",
+                                MapType(explicit_binary, explicit_binary),
+                            ),
+                        ]
+                    ),
+                ),
+            ]
+        )
+        rows = [
+            (
+                "implicit",
+                "direct",
+                Row(s="nested", a=["array"], m={"key": "value"}),
+            )
+        ]
+
+        populated = self.connect.createDataFrame(rows, schema)

Review Comment:
   Agreed. The ordinary StringType explicitness state and tests were removed, 
so nested StringType identity is no longer an owned behavior here and should 
remain follow-up work.
   
   <!-- SPARK_DEV_REVIEW_REPLY 
{"feedback_id":"inline:4037005001","thread_id":"inline:4037005001","verdict_sha256":"97b99468d98c3679edc8479bbc2a570c93408cead229f9aa41bdc01830abc558"}
 -->



##########
python/pyspark/sql/types.py:
##########
@@ -295,8 +298,11 @@ class StringType(AtomicType):
     providerICU = "icu"
     providers = [providerSpark, providerICU]
 
-    def __init__(self, collation: str = "UTF8_BINARY"):
-        self.collation = collation
+    __slots__ = ("_collation_explicit",)
+
+    def __init__(self, collation: str = _DEFAULT_STRING_COLLATION):

Review Comment:
   Confirmed. The sentinel and _collation_explicit state are gone, and 
StringType's public constructor signature and prior identity behavior are 
restored. Thanks.
   
   <!-- SPARK_DEV_REVIEW_REPLY 
{"feedback_id":"inline:4037005010","thread_id":"inline:4037005010","verdict_sha256":"97b99468d98c3679edc8479bbc2a570c93408cead229f9aa41bdc01830abc558"}
 -->



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to