dwsmith1983 commented on code in PR #6065: URL: https://github.com/apache/datafusion-comet/pull/6065#discussion_r4074039135
########## spark/src/main/scala/org/apache/comet/iceberg/IcebergStorageSchemes.scala: ########## @@ -0,0 +1,73 @@ +/* + * 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.comet.iceberg + +import java.util.Locale + +import org.apache.spark.internal.Logging + +import org.apache.comet.NativeBase + +/** + * The storage schemes the native Iceberg storage factory publishes over JNI, so the JVM scan and + * write gates decline what native cannot open instead of failing at execution. Every caller sits + * behind `isCometLoaded`, so the fallback constants are only consulted in a JVM where nothing + * runs natively; the pinning test in `CometScanSchemeFallbackSuite` keeps them equal to the + * native lists. + */ +private[comet] object IcebergStorageSchemes extends Logging { + + private[comet] val FallbackRead: Set[String] = Set("file", "s3", "s3a", "gs", "oss") + private[comet] val FallbackWrite: Set[String] = Set("file", "memory", "s3", "s3a", "gs") Review Comment: > Is there a path I'm missing where the fallback does real work? No. Both lazy sets are first touched inside `CometScanRule` and `CometExecRule`, whose `_apply` returns the plan untouched unless `isCometLoaded` is true, and `isCometLoaded` ends by evaluating `NativeBase.isLoaded`. So by the time either set is read the library is loaded. In 1d3f95d6b `load` returns the empty set with a warning when it is not, a blank list from a loaded library throws since native publishes a fixed non-empty constant and a blank one can only be a build bug, and the two fallback constants and the two pinning tests are gone. `load` takes the loaded flag as a parameter so the unloaded answer is tested without unloading the library, and one test checks the lazy sets against a direct parse of the probe. -- 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]
