HyukjinKwon commented on code in PR #58614:
URL: https://github.com/apache/spark/pull/58614#discussion_r3994452810
##########
connector/avro/src/test/scala/org/apache/spark/sql/avro/AvroSuite.scala:
##########
@@ -1255,6 +1255,64 @@ abstract class AvroSuite
assertExceptionMsg[FileNotFoundException](e, "File not_exists.avsc does
not exist")
}
+ // spark.sql.avro.schemaUrlAllowedSchemes is a static SQL config, so it
cannot be set with
Review Comment:
Done in 9655f9e. Added `AvroSchemaUrlAllowlistSuite` (`QueryTest with
SharedSparkSession`, overriding `sparkConf` to set the static allowlist) that
drives a real `spark.read ... load()`: an allowed scheme is permitted, a
disallowed scheme is rejected, and a session cannot relax it
(`CANNOT_MODIFY_STATIC_CONFIG` via both `spark.conf.set` and SQL `SET`). Kept
the direct-`AvroOptions` cases in `AvroSuite` on top of it.
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala:
##########
@@ -7016,6 +7016,20 @@ object SQLConf {
.booleanConf
.createWithDefault(true)
+ val AVRO_SCHEMA_URL_ALLOWED_SCHEMES =
+ buildStaticConf("spark.sql.avro.schemaUrlAllowedSchemes")
Review Comment:
Kept it with the other `spark.sql.avro.*` entries and added a comment noting
it is static; `buildStaticConf` registers the static key wherever it is
declared. 9655f9e.
##########
connector/avro/src/test/scala/org/apache/spark/sql/avro/AvroSuite.scala:
##########
@@ -1255,6 +1255,64 @@ abstract class AvroSuite
assertExceptionMsg[FileNotFoundException](e, "File not_exists.avsc does
not exist")
}
+ // spark.sql.avro.schemaUrlAllowedSchemes is a static SQL config, so it
cannot be set with
+ // withSQLConf; these drive AvroOptions directly under a SQLConf provided
via withExistingConf.
+ // A scheme-less local path resolves to the default file system ("file").
+ test("SPARK-59329: avroSchemaUrl scheme allowlist permits an allowed
scheme") {
+ val avroSchemaUrl = testFile("test_sub.avsc")
+ val hadoopConf = spark.sessionState.newHadoopConf()
+ val conf = new SQLConf()
+ conf.setConf(SQLConf.AVRO_SCHEMA_URL_ALLOWED_SCHEMES, Seq("file"))
+ SQLConf.withExistingConf(conf) {
+ val options = new AvroOptions(Map("avroSchemaUrl" -> avroSchemaUrl),
hadoopConf)
+ assert(options.schema.isDefined)
+ }
+ }
+
+ test("SPARK-59329: avroSchemaUrl allowlist rejects a disallowed scheme " +
+ "before opening the file system") {
+ // An explicit non-"file" scheme is rejected by the allowlist check, which
runs before the
+ // file system for the URL is instantiated -- so this surfaces the clean
allowlist error
+ // rather than a lower-level failure from trying to load the s3a file
system.
+ val hadoopConf = spark.sessionState.newHadoopConf()
+ val conf = new SQLConf()
+ conf.setConf(SQLConf.AVRO_SCHEMA_URL_ALLOWED_SCHEMES, Seq("file"))
+ SQLConf.withExistingConf(conf) {
+ val e = intercept[AnalysisException] {
+ new AvroOptions(Map("avroSchemaUrl" -> "s3a://bucket/user.avsc"),
hadoopConf)
Review Comment:
Done in 9655f9e. The rejection test now uses `S3A://bucket/user.avsc` and
asserts the message contains `The scheme 's3a'`, so dropping the
`.toLowerCase(Locale.ROOT)` on the scheme fails it.
--
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]
