PHILO-HE commented on code in PR #6672: URL: https://github.com/apache/incubator-gluten/pull/6672#discussion_r1708964752
########## gluten-core/src/main/scala/org/apache/gluten/utils/FileIndexUtil.scala: ########## @@ -0,0 +1,41 @@ +/* + * 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.gluten.utils + +import org.apache.spark.sql.execution.datasources._ + +import java.net.URI + +object FileIndexUtil { + def getRootPath(index: FileIndex): Seq[String] = { + index.rootPaths + .filter(_.isAbsolute) + .map(_.toString) + .toSeq + } + + def distinctRootPaths(paths: Seq[String]): Seq[String] = { + // Skip native validation for local path, as local file system is always registered. + // For evey file schema, only one path is kept. Review Comment: schema -> scheme. Ditto for other places. ########## shims/common/src/main/scala/org/apache/gluten/GlutenConfig.scala: ########## @@ -1968,6 +1971,13 @@ object GlutenConfig { .booleanConf .createWithDefault(false) + val VELOX_SCAN_ROOT_PATH_SCHEMA_VALIDATION_ENABLED = + buildConf("spark.gluten.sql.scan.root.path.schema.validation.enabled") + .internal() + .doc("Enable root path schema validation for scan.") Review Comment: No need to expose "root path" to user, since it may be confusing. Suggestion: "When true, enable file path scheme validation for scan. Validation will fail if file scheme is not supported by registered file systems, which will cause scan operator fall back." ########## backends-velox/src/main/scala/org/apache/gluten/backendsapi/velox/VeloxBackend.scala: ########## @@ -73,7 +74,16 @@ object VeloxBackendSettings extends BackendSettingsApi { format: ReadFileFormat, fields: Array[StructField], partTable: Boolean, + rootPaths: Seq[String], paths: Seq[String]): ValidationResult = { Review Comment: What's the differences between `rootPaths` and `paths`? ########## backends-velox/src/test/scala/org/apache/gluten/execution/FallbackSuite.scala: ########## @@ -263,4 +264,43 @@ class FallbackSuite extends VeloxWholeStageTransformerSuite with AdaptiveSparkPl } } } + + test("fallback reader with unsupported filesystem") { Review Comment: Nit: "Fall back scan operator for unsupported file scheme" ########## shims/common/src/main/scala/org/apache/gluten/GlutenConfig.scala: ########## @@ -1968,6 +1971,13 @@ object GlutenConfig { .booleanConf .createWithDefault(false) + val VELOX_SCAN_ROOT_PATH_SCHEMA_VALIDATION_ENABLED = + buildConf("spark.gluten.sql.scan.root.path.schema.validation.enabled") Review Comment: Let's simplify the name. Suggestion: "spark.gluten.sql.scan.fileSchemeValidation.enabled". -- 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]
