cloud-fan commented on code in PR #58581:
URL: https://github.com/apache/spark/pull/58581#discussion_r3960045571
##########
sql/connect/common/src/main/scala/org/apache/spark/sql/connect/SparkSession.scala:
##########
@@ -223,7 +223,9 @@ class SparkSession private[sql] (
/** @inheritdoc */
def createDataFrame(rows: java.util.List[Row], schema: StructType):
DataFrame = {
- createDataset(RowEncoder.encoderFor(schema),
rows.iterator().asScala).toDF()
+ // The client cannot observe the server's CHAR/VARCHAR configuration.
Encode an explicitly
+ // provided schema independently of the client's local configuration, as
for result schemas.
+ createDataset(RowEncoder.encoderForResultSchema(schema),
rows.iterator().asScala).toDF()
Review Comment:
**Blocking (P1):** `encoderForResultSchema` admits caller-provided
CHAR/VARCHAR regardless of the client flag, but an empty input is sent as a
schema-only `LocalRelation`. The server returns that relation already resolved
and skips the `.to(schema)` reconciliation used for non-empty data, so the
default server policy can reject the schema with one row yet expose first-class
`CHAR` with zero rows. Please validate or normalize the schema-only relation
with the same server policy as the data-bearing path, and add default-config
coverage for empty input.
**Recommended change:** Route schema-only local relations through the same
server-side CHAR/VARCHAR validation and normalization policy as non-empty local
data.
**Why this works:** Apply the server's existing caller-schema CHAR/VARCHAR
reconciliation before constructing a schema-only resolved LocalRelation,
matching the data-bearing LocalRelation path.
**Scope:** Connect client local DataFrame encoding, server LocalRelation
transformation for schema-only input, and focused empty-versus-non-empty
configuration tests.
**Compatibility:** Keep client-side support for result and caller schemas,
while making empty and non-empty local data honor the same authoritative server
configuration and existing rejection or normalization behavior.
**Risks:** Duplicating policy logic in the empty branch could drift from
Dataset.to(schema) behavior. Changing generic schema-only LocalRelation
handling must preserve legitimate engine-produced result schemas.
**Constraints:** The client must not decide the server session's
CHAR/VARCHAR policy. Schema legality and normalization must not depend on
whether the local row collection is empty. Do not rely on CheckAnalysis to
revisit a LocalRelation that is already resolved.
**Success:** Under each supported CHAR/VARCHAR server configuration, the
same caller schema is rejected, normalized, or preserved identically for empty
and non-empty createDataFrame input.
##########
python/pyspark/sql/types.py:
##########
@@ -1366,10 +1390,15 @@ def processDataType(dt: DataType, fieldPath: str) ->
None:
return collationMetadata
def _isCollatedString(self, dt: DataType) -> bool:
- return isinstance(dt, StringType) and not dt.isUTF8BinaryCollation()
+ if isinstance(dt, StringType):
+ return not dt.isUTF8BinaryCollation()
+ if isinstance(dt, (CharType, VarcharType)):
+ return not dt.isUTF8BinaryCollation()
Review Comment:
**Blocking (P1):** This treats explicit `UTF8_BINARY` like an omitted
CHAR/VARCHAR collation. If another leaf in the same field has a non-binary
collation, compatibility serialization strips both collations but emits
metadata only for the non-binary leaf. For example, a map from `CharType(4,
"UTF8_BINARY")` to `VarcharType(6, "UNICODE_CI")` comes back with an uncollated
key, which can later inherit a different default. Please emit CHAR/VARCHAR
metadata whenever the optional collation is present, including explicit binary,
in both the Python and Scala paths, and cover this mixed composite round trip.
**Recommended change:** Record metadata for every explicitly present
CHAR/VARCHAR collation, including UTF8_BINARY, and add one mixed composite
round-trip regression.
**Why this works:** Make Scala and Python StructField collation-metadata
emission presence-sensitive for CHAR/VARCHAR before compatibility serialization
strips inline collations.
**Scope:** Python and Scala StructField collation metadata extraction and
their composite schema JSON round-trip tests.
**Compatibility:** Preserve the existing JSON shape and StringType default
handling while adding restoration metadata for an explicitly present
CHAR/VARCHAR binary collation in both language implementations.
**Risks:** Changing StringType's separate default-collation convention could
alter existing schema JSON unnecessarily. Updating only Python would leave the
parallel Scala compatibility serializer asymmetric.
**Constraints:** Distinguish an absent CHAR/VARCHAR collation from explicit
UTF8_BINARY by optional-field presence, not collation value equality. Every
CHAR/VARCHAR collation removed by compatibility serialization must have a
matching restoration entry. Keep the established StringType metadata behavior
unchanged.
**Success:** A MapType or ArrayType containing both explicit UTF8_BINARY and
a non-binary CHAR/VARCHAR collation round-trips with every optional collation
state unchanged in Scala and Python.
--
This is an automated message from the 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]
