jackylee-ch commented on code in PR #12495:
URL: https://github.com/apache/gluten/pull/12495#discussion_r3558318132
##########
cpp/velox/utils/ConfigExtractor.cc:
##########
@@ -246,6 +283,38 @@ std::string parquetSessionProperty(std::string_view key) {
} // namespace
+std::shared_ptr<facebook::velox::config::ConfigBase> mergeFileSystemConfigs(
+ const std::shared_ptr<facebook::velox::config::ConfigBase>& backendConf,
+ const std::shared_ptr<facebook::velox::config::ConfigBase>& runtimeConf) {
+ constexpr std::string_view kSparkHadoopFsPrefix = "spark.hadoop.fs.";
+
+ auto merged = backendConf->rawConfigs();
+ for (const auto& [key, value] : runtimeConf->rawConfigs()) {
+ if (key.starts_with(kSparkHadoopFsPrefix)) {
+ merged.insert_or_assign(key, value);
+ }
+ }
Review Comment:
Gluten requires C++20: `cpp/CMakeLists.txt` sets `CMAKE_CXX_STANDARD 20`
with `CMAKE_CXX_STANDARD_REQUIRED ON`, and this file already uses `starts_with`
in existing code. No compatibility change is needed.
##########
cpp/core/config/GlutenConfig.cc:
##########
@@ -33,9 +31,13 @@ std::optional<boost::regex> getRedactionRegex(const
std::unordered_map<std::stri
}
return std::nullopt;
}
Review Comment:
Handled in 11724eb11. `getRedactionRegex` now catches `boost::regex_error`
and falls back to `.*`, so dumping fails closed rather than disabling
redaction. `GlutenConfigTest.InvalidRedactionRegexFailsClosed` verifies
non-throwing behavior and that values are not exposed.
##########
backends-velox/src/main/scala/org/apache/gluten/datasource/VeloxDataSourceUtil.scala:
##########
@@ -31,25 +31,60 @@ import java.util
object VeloxDataSourceUtil {
def readSchema(files: Seq[FileStatus]): Option[StructType] = {
+ readSchema(files, new util.HashMap[String, String]())
+ }
+
+ def readSchema(
+ files: Seq[FileStatus],
+ fsConf: util.Map[String, String]): Option[StructType] = {
if (files.isEmpty) {
throw new IllegalArgumentException("No input file specified")
}
- readSchema(files.toList.head)
+ readSchema(files.toList.head, fsConf)
}
def readSchema(file: FileStatus): Option[StructType] = {
+ readSchema(file, new util.HashMap[String, String]())
+ }
+
+ def readSchema(
+ file: FileStatus,
+ fsConf: util.Map[String, String]): Option[StructType] = {
val allocator = ArrowBufferAllocators.contextInstance()
- val runtime = Runtimes.contextInstance(BackendsApiManager.getBackendName,
"VeloxWriter")
+ val runtime =
+ Runtimes.contextInstance(BackendsApiManager.getBackendName,
"VeloxWriter", fsConf)
val datasourceJniWrapper = VeloxDataSourceJniWrapper.create(runtime)
- val dsHandle =
- datasourceJniWrapper.init(file.getPath.toString, -1, new
util.HashMap[String, String]())
- val cSchema = ArrowSchema.allocateNew(allocator)
- datasourceJniWrapper.inspectSchema(dsHandle, cSchema.memoryAddress())
+ withDataSourceResource[ArrowSchema, Option[StructType]](
+ () => datasourceJniWrapper.init(file.getPath.toString, -1, fsConf),
+ () => ArrowSchema.allocateNew(allocator),
+ (dsHandle, cSchema) => {
+ datasourceJniWrapper.inspectSchema(dsHandle, cSchema.memoryAddress())
+
Option(SparkSchemaUtil.fromArrowSchema(ArrowAbiUtil.importToSchema(allocator,
cSchema)))
+ },
+ datasourceJniWrapper.close
+ )
+ }
+
+ private[apache] def withDataSourceResource[R <: AutoCloseable, T](
+ initialize: () => Long,
+ allocate: () => R,
+ use: (Long, R) => T,
+ closeHandle: Long => Unit): T = {
Review Comment:
Handled in 11724eb11. The helper is now `private[datasource]`, and its
resource lifecycle tests were moved to
`org.apache.gluten.datasource.VeloxDataSourceUtilSuite`, so no
`org.apache.spark.*` access is required.
--
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]
