spark-test commented on code in PR #59305:
URL: https://github.com/apache/spark/pull/59305#discussion_r4310732731
##########
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/DataTypeParserSuite.scala:
##########
@@ -88,6 +88,15 @@ class DataTypeParserSuite extends SparkFunSuite with
SQLHelper {
checkDataType("cHaR(27)", CharType(27))
checkDataType("BINARY", BinaryType)
checkDataType("void", NullType)
+ checkDataType("FILE EXTERNAL", FileType.external)
+ checkDataType("file external", FileType.external)
+ // A bare FILE is the type of an expression or a UDF result, not of a column
declaration.
Review Comment:
**Nit (P3):** This says a bare FILE is not the type of a column declaration,
but FileType documents that EXTERNAL enforcement is planned and bare FILE is
currently accepted wherever a type is. The comment can make this assertion look
like coverage of a restriction that this part does not implement. Could it
describe the UNKNOWN mode being tested, or qualify the column rule as planned?
**Verification:**
- **Inspection:** The revised explanation distinguishes present bare-FILE
parsing from planned table-column EXTERNAL enforcement and does not imply the
existing assertion tests a column restriction.
##########
sql/api/src/main/scala/org/apache/spark/sql/types/StructField.scala:
##########
@@ -92,9 +94,26 @@ case class StructField(
case _ => dt
}
+ // An external file in a field is the plain "file", the form of a schema
written without a
+ // mode, so that a schema which carries other engine-specific metadata for
it is written back
+ // as it was read. Only array and map types are visited, as in
`removeCollations`.
+ def fileTypeJson(dt: DataType): JValue = dt match {
+ case at: ArrayType if at.existsRecursively(_.isInstanceOf[FileType]) =>
Review Comment:
**Non-blocking (P2):** Each array guard scans the remaining subtree with
existsRecursively before fileTypeJson walks that subtree again; the map guard
does the same. For a field with d nested arrays ending in FileType.external,
the searches visit d, d-1, and so on remaining levels, adding quadratic work
for linear JSON output. The initial hasFile search adds another traversal. This
affects local schema serialization, rather than establishing a distributed
query slowdown. Direct array/map recursion can emit the same field-context JSON
without the repeated presence checks.
**Verification:**
- **Inspection:** Field-context FILE emission recurses directly over each
array/map container and delegates at struct boundaries, without an initial or
per-suffix FILE-presence traversal or a replacement repeated subtree scan.
- **Regression:** Nested arrays and maps, including FILE in either map side
and nested struct fields, preserve EXTERNAL and UNKNOWN modes and their
expected JSON spelling after direct serialization and decoding.
- **Compatibility:** Schemas without FILE retain their JSON structure and
nullability; standalone FILE modes, opaque field metadata, and mixed
FILE/string collation forms retain their existing spelling and round-trip
behavior, including explicit UTF8_BINARY and nondefault collations.
--
This is an automated message from the 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]
