dejankrak-db commented on code in PR #59305:
URL: https://github.com/apache/spark/pull/59305#discussion_r4260888879
##########
sql/api/src/main/scala/org/apache/spark/sql/catalyst/parser/parsers.scala:
##########
@@ -499,6 +499,7 @@ object AbstractParser extends Logging {
parser.parameter_substitution_enabled =
!conf.legacyParameterSubstitutionConstantsOnly
parser.legacy_identifier_clause_only = conf.legacyIdentifierClauseOnly
parser.single_character_pipe_operator_enabled =
conf.singleCharacterPipeOperatorEnabled
+ parser.file_type_enabled = conf.isFileTypeEnabled
Review Comment:
Thanks, good catch. `configureSplitterParser` now sets `file_type_enabled`
like the session parser, and `SqlStatementSplitterSuite` has a test with a
`BEGIN ... END` block that declares a `FILE EXTERNAL` column. The test fails
without the change (the block is split into `BEGIN CREATE TABLE t (f FILE
EXTERNAL);`, `SELECT 1;` and `END;`) and passes with it (c27450f76a4).
##########
sql/api/src/main/scala/org/apache/spark/sql/types/FileType.scala:
##########
@@ -0,0 +1,110 @@
+/*
+ * 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 org.apache.spark.annotation.Unstable
+
+/**
+ * The data type representing a reference to a file, with metadata describing
the file.
+ *
+ * In SQL the type is named `FILE`, and the `EXTERNAL` keyword is required
when declaring a
Review Comment:
Thanks. The class doc now says that the `EXTERNAL` requirement for table
columns is planned, that a bare `FILE` is accepted wherever a type is until
then, and gives the standalone JSON form of each mode (`"file external"` for
`FileType.external`, `"file"` for `FileType()`). I also removed the duplicate
sentence from the mode paragraph (c27450f76a4).
##########
sql/api/src/main/scala/org/apache/spark/sql/types/FileType.scala:
##########
@@ -0,0 +1,110 @@
+/*
+ * 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 org.apache.spark.annotation.Unstable
+
+/**
+ * The data type representing a reference to a file, with metadata describing
the file.
+ *
+ * In SQL the type is named `FILE`, and the `EXTERNAL` keyword is required
when declaring a
+ * column, as in `CREATE TABLE t (f FILE EXTERNAL)`. Its JSON representation
is `"file"` inside
+ * a struct field and `"file external"` for a standalone type.
+ *
+ * FileType is planned to be physically stored as a struct with the following
fields (see
+ * `FileType.FIELDS`), but it deliberately does not extend [[StructType]]:
+ * - uri (STRING): the opaque URI to the file
+ * - offset (BIGINT): the start of the referenced byte range
+ * - size (BIGINT): the size of the file in bytes
+ * - content_type (STRING): the MIME/content type of the file
+ * - checksum (STRING): an algorithm-tagged integrity token for the file,
e.g. "ETAG:<etag>"
+ * - inline (BINARY): the referenced bytes stored inline in the value
+ *
+ * FileType may be EXTERNAL (simple references, lifecycle independent of the
table) or UNKNOWN
+ * (mode not yet determined, such as the result of a cast or of a UDF). Mode
is part of type
+ * identity - files with different modes are distinct under `equals`. Table
column definitions
+ * are to require `EXTERNAL`.
+ *
+ * FileType is planned to be not comparable -- it cannot be used in join
expressions, grouping
+ * expressions, or as a partition/clustering column. Users can extract the uri
field for such
+ * operations. This type currently only guarantees the schema-level pieces
(DDL and JSON); the
+ * physical representation and most of the restrictions are added by later
parts.
+ *
+ * @param mode the mode for this FileType column
+ * @since 4.4.0
+ */
+@Unstable
+case class FileType(mode: FileTypeMode = FileTypeMode.UNKNOWN) extends
DataType {
+
+ override def defaultSize: Int = FileType.asStruct.defaultSize
+
+ override def typeName: String = mode match {
+ case FileTypeMode.EXTERNAL => "file external"
+ case _ => "file"
+ }
+
+ /**
+ * `acceptsType` ignores mode so `ExpectsInputTypes.inputTypes =
Seq(FileType())` (e.g.
+ * `GetStructField`) accepts FileType(EXTERNAL). `sameType` / `equals` still
distinguish modes.
Review Comment:
Agreed, nothing declares a FileType input type yet. I used your suggested
wording (c27450f76a4).
##########
sql/catalyst/src/test/scala/org/apache/spark/sql/types/DataTypeSuite.scala:
##########
@@ -1708,6 +1708,71 @@ class DataTypeSuite extends SparkFunSuite with SQLHelper
{
}
}
+ test("FileType: name, JSON and DDL round trip") {
+ // The mode is part of the type's identity.
+ assert(FileType.external != FileType())
+ assert(FileType() === FileType(FileTypeMode.UNKNOWN))
+ assert(DataTypes.FileType === FileType())
+ assert(FileType.external.typeName === "file external")
+ assert(FileType().typeName === "file")
+ // A type prints what was written: FILE EXTERNAL for an external file,
FILE for a bare FILE.
+ assert(FileType.external.sql === "FILE EXTERNAL")
+ assert(FileType().sql === "FILE")
+ assert(FileType.external.json === "\"file external\"")
+ assert(FileType().json === "\"file\"")
+ assert(DataType.fromJson("\"file external\"") === FileType.external)
+ // A bare "file" has no mode on its own, and is an external file inside a
struct field.
+ assert(DataType.fromJson("\"file\"") === FileType())
Review Comment:
Thanks. Added a test for the three cases: standalone array and map elements
with a bare `"file"` keep the mode unknown, the field shapes without `metadata`
or `nullable` read as external, and a schema with nested FILE types decodes
with `spark.sql.fileType.enabled` set to false (c27450f76a4).
--
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]
