sarutak opened a new pull request, #59947: URL: https://github.com/apache/spark/pull/59947
### What changes were proposed in this pull request? This PR makes `spark.addArtifact` fail for a native data source package (SPARK-60101) that Spark cannot use, instead of adding it to the session and ignoring it when a query looks the data source up. - When a session adds a `.sparkpkg` file, and `spark.sql.dataSource.native.enabled` is true, `ArtifactManager.addArtifact` now reads the package before it becomes an artifact of the session, and fails with `INVALID_NATIVE_DATA_SOURCE_PACKAGE.INVALID_MANIFEST` if Spark cannot read it, or with `INVALID_NATIVE_DATA_SOURCE_PACKAGE.UNSUPPORTED_ABI_VERSION` if its `abiVersion` is newer than the version of the interface that Spark supports. The error names the file that the user added, or the artifact, such as `files/rs-2.0.sparkpkg`, when there is no file of the user, as when a Spark Connect client sends the file. The session then has no file with that name, so it can add a valid package with the same file name. This applies to `spark.addArtifact` in Scala, Java and Python, and with Spark Connect. The check runs after the check for an artifact that already exists, so `ARTIFACT_ALREADY_EXISTS` still comes first, and a file that does not exist still fails as for the other artifacts. In Sc ala and Java, and with Spark Connect, a call that adds several artifacts does not add the ones after the rejected package, unlike after `ARTIFACT_ALREADY_EXISTS`, which does not stop the others. - Only the file artifacts (`files/`) with the extension are packages: the session no longer takes another kind of artifact with the extension, such as `pyfiles/x.sparkpkg`, which a Spark Connect client could send, as a package without checking it. - A package without a library for the platform of the driver is still accepted, and a query that uses it fails with `INVALID_NATIVE_DATA_SOURCE_PACKAGE.UNSUPPORTED_PLATFORM`, as before: the user sees that error, unlike a package that the lookup ignores. The packages in `spark.sql.dataSource.native.paths` are still ignored and logged when Spark cannot use them. - The message of `UNSUPPORTED_ABI_VERSION`, of both `INVALID_NATIVE_DATA_SOURCE_PACKAGE` and `INVALID_NATIVE_DATA_SOURCE_LIBRARY`, now says that the package or the library "implements" the version rather than "requires" it: the version in a manifest is the oldest that its library implements, and the version of a loaded library is the one that its table implements, which can also be older than the version of Spark. - The doc of `spark.sql.dataSource.native.paths` and the docs of native data sources now say that a package that a session adds takes precedence over the configured ones, and a comment in `NativeDataSourceRegistry.lookupPackage`, which only sees the configured packages, no longer gives the packages of a session as an example. ### Why are the changes needed? During the review of SPARK-60101, the package that a session added last was given precedence over its other packages and the configured ones, so that a session can upgrade a package by adding a new version under another file name: a session cannot remove an artifact. But the lookup drops the packages that Spark cannot use before it picks the last one. So a session that adds `rs-1.0.sparkpkg`, and then `rs-2.0.sparkpkg` whose manifest is broken or declares a newer `abiVersion`, keeps using rs-1.0, with only a warning in the log of the driver, which a Spark Connect client does not see. The user believes that the session uses the new version. Failing when the session adds the package reports the problem to the user who added it, also for a manifest that cannot be read at all, whose data sources Spark does not know when it looks one up. A session then does not get a package that Spark cannot read, or whose version it does not support, and it can recover by adding a fixed package. Since the file is checked before it becomes an artifact, an invalid package is never at the path of an artifact, so a concurrent add of the same file cannot take it as one that the session already added. ### Does this PR introduce _any_ user-facing change? No for native data sources, which are not released yet (4.4.0). One effect on an existing API: `spark.addArtifact` now rejects a file artifact named `*.sparkpkg` that Spark cannot read as a native data source package, or whose version it does not support, which earlier releases added as any other file, with `file=True` in Python or from a Spark Connect client, unless `spark.sql.dataSource.native.enabled` is false. Since SPARK-60101, such a file is taken as a package. ### How was this patch tested? A new test in `NativeDataSourceSuite`, `reject a package that a session adds when Spark cannot use it`, adds a package with a newer `abiVersion`, a package whose manifest names a library that it does not contain, a file that is not a zip, and a directory, each twice. It checks that each add fails with its error, that the session does not get the file, neither as a package nor in the files of the `SparkContext`, and keeps using the configured package, and that a valid package with the same file name can be added afterwards. It also checks that another file with the name of an artifact still fails with `ARTIFACT_ALREADY_EXISTS`, that a file that does not exist fails with `NoSuchFileException` as before, that a package without a library for the platform is added and fails when it is used, and that a `pyfiles/` artifact with the extension is not a package. Removing the check, the version check, the restriction to `files/`, the skip of a file that does not exist, or checking only regul ar files, inverting the condition on `spark.sql.dataSource.native.enabled`, naming the artifact for a local file, or checking before the check for an existing artifact, each makes it fail, and naming the staged file for a Spark Connect client makes the Connect test fail. A new test in `NativeDataSourceConnectSuite` adds a file that is not a package through a Spark Connect client, checks that the client gets the error, which names the artifact rather than the file in which the server staged it, and then adds a valid package with the same name. `test_add_native_data_source_package` in `pyspark.sql.tests.test_artifact` added files that are not packages, which are now rejected, so it now adds valid packages, and checks that an invalid one is rejected. `NativeDataSourceSuite`, `ColumnarDataSourceSuite`, `ArtifactManagerSuite`, `NativeDataSourceConnectSuite`, `AddArtifactsHandlerSuite`, `ArtifactStatusesHandlerSuite`, `SparkThrowableSuite` and that Python test pass locally on Java 22 on macOS, except two tests that are canceled without pandas. ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Kiro CLI -- 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]
