brkyvz commented on code in PR #59640:
URL: https://github.com/apache/spark/pull/59640#discussion_r4449844983


##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/TypeUtils.scala:
##########
@@ -162,4 +165,123 @@ object TypeUtils extends QueryErrorsBase {
         messageParameters = scala.collection.immutable.Map.empty)
     }
   }
+
+  private def containsUnknownFileType(dataType: DataType): Boolean = {
+    dataType.existsRecursively {
+      case ft: FileType => ft.mode == FileTypeMode.UNKNOWN
+      case _ => false
+    }
+  }
+
+  /**
+   * Fails if a FILE in `dataType`, at any nesting depth, has no qualifier, as 
a FILE in a table
+   * column definition must be declared `FILE EXTERNAL`.
+   */
+  def checkFileTypeHasQualifier(dataType: DataType): Unit = {
+    if (containsUnknownFileType(dataType)) {
+      throw QueryCompilationErrors.fileTypeMissingQualifierError()
+    }
+  }
+
+  /**
+   * Fails if any of `columns`, which a write such as CREATE TABLE AS SELECT 
stores in a table,
+   * holds a FILE without a qualifier, as a table cannot store one.
+   */
+  def checkNoUnknownFileTypeWrite(columns: Seq[Attribute]): Unit = {
+    checkNoUnknownFileTypeInColumns(columns.map(c => Seq(c.name) -> 
c.dataType))
+  }
+
+  /**
+   * Fails if a column of `schema`, which a command such as CREATE TABLE LIKE 
stores in a new
+   * table, holds a FILE without a qualifier.
+   */
+  def checkNoUnknownFileTypeInTableSchema(schema: StructType): Unit = {
+    checkNoUnknownFileTypeInColumns(schema.fields.toSeq.map(f => Seq(f.name) 
-> f.dataType))
+  }

Review Comment:
   Should these checks live in a file such as `FileTypeChecks`?



##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CheckAnalysis.scala:
##########
@@ -1253,6 +1334,28 @@ trait CheckAnalysis extends LookupCatalog with 
QueryErrorsBase with PlanToString
     }
   }
 
+  /**
+   * Checks that the [[UserDefinedExpression]]s in a query declare only a 
plain FILE in their
+   * return type, as 
[[TypeUtils.checkNoQualifierInRegisteredRoutineReturnType]] does when a
+   * routine is registered. This covers routines whose signature is not 
checked when they are
+   * created, such as UDFs that are not registered and UDAF classes registered 
with
+   * `CREATE FUNCTION ... AS`. SQL functions are checked by `CREATE FUNCTION`. 
V2 catalog functions
+   * are covered only when they are bound as an [[ApplyFunctionExpression]]. A 
non-scalar Python
+   * UDF is skipped, as its data type is the schema of the rows it returns.
+   */
+  private def checkFileTypeInUserDefinedRoutines(plan: LogicalPlan): Unit = {
+    if (SQLConf.get.isFileTypeEnabled) {

Review Comment:
   Do all checks in this file need to validate the flag?



##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/TypeUtils.scala:
##########
@@ -70,7 +72,8 @@ object TypeUtils extends QueryErrorsBase {
   }
 
   def checkForMapKeyType(keyType: DataType): TypeCheckResult = {
-    if (keyType.existsRecursively(dt => dt.isInstanceOf[MapType] || 
dt.isInstanceOf[VariantType])) {
+    if (keyType.existsRecursively(dt => dt.isInstanceOf[MapType] || 
dt.isInstanceOf[VariantType] ||

Review Comment:
   a future follow-up - it may help to add traits to types to state whether 
they support ordering, support grouping etc



-- 
This is an automated message from the 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