sarutak commented on code in PR #59947:
URL: https://github.com/apache/spark/pull/59947#discussion_r4451429095
##########
docs/sql-migration-guide.md:
##########
@@ -30,6 +30,7 @@ license: |
- Since Spark 4.4, Data Source V2 file scans correctly evaluate deterministic
Python UDF filter predicates that reference only partition columns. Previously,
these predicates could be dropped, returning rows that did not satisfy the
filter (SPARK-60486). Data derived from affected query results may need to be
recomputed.
- Since Spark 4.4, quoted Variant path keys decode `\\`, `\/`, `\b`, `\f`,
`\n`, `\r`, `\t`, `\uXXXX` (including valid surrogate pairs), and escapes of
the enclosing quote (`\'` or `\"`). Previously, backslashes were literal, so
existing paths can match different keys or become invalid. For example, use
`$['a\\']` instead of `$['a\']`. Unknown escapes and unpaired surrogate escapes
stay literal. Dot-notation keys are unchanged. To keep the old behavior, set
`spark.sql.legacy.variantPathBackslashAsLiteral` to `true` before creating the
query.
+- Since Spark 4.4, `spark.addArtifact` and `spark.addArtifacts` take a file
whose name ends with `.sparkpkg` as a [native data source
package](sql-native-data-sources.html), also when it is added with `file=True`,
`pyfile=True` or `archive=True` in Python without Spark Connect, or with
`file=True` from a Spark Connect client, and fail with
`INVALID_NATIVE_DATA_SOURCE_PACKAGE` for such a file that is not a valid
package, or whose version Spark does not support, or that has no library for
the platform of the driver. Previously, it was added as a file, a Python file
or an archive. To add such a file as before, rename it. To add it as a file of
the session without the check, set the static configuration
`spark.sql.dataSource.native.enabled` to `false`, which disables native data
sources; in Python, it is then still not added as a Python file or an archive.
Review Comment:
Done in c0924efa51d with your wording: "Previously, it was added as a file,
a Python file or an archive, or ignored without a flag in Python without Spark
Connect."
##########
python/pyspark/sql/session.py:
##########
@@ -2275,7 +2275,13 @@ def addArtifacts(
----------
*path : tuple of str
Artifact's URIs to add. A native data source package (a
``.sparkpkg`` file) makes
- the native data sources it contains available to the session.
+ the native data sources it contains available to the session. It
fails for a
Review Comment:
Done in c0924efa51d: "The call fails for a file that is not a valid package,
...".
##########
docs/sql-native-data-sources.md:
##########
@@ -325,12 +325,27 @@ A session finds the packages in two places, and the
executors get them automatic
The packages of a session take precedence over the installed libraries, and
the package that a
session added last over its other packages, also the configured ones: a
session cannot remove an
-artifact, or add a file with the same name again, so it upgrades a package by
adding the new
-version under another file name, such as `rust_range-1.1.sparkpkg`. Distinct
file names are also
-needed because the sessions that are not isolated share the files they add. A
query fails if more
-than one configured package provides its data source. Spark ignores, and logs,
a package that it
-cannot read or whose version it does not support, unless its manifest lists
the data source that a
-query uses and no other package or installed library provides it.
+artifact, or add another file with the name of one that it added, so it
upgrades a package by adding
+the new version under another file name, such as `rust_range-1.1.sparkpkg`.
Distinct file names are
+also needed because the sessions that are not isolated share the files they
add. A query fails if
+more than one configured package provides its data source. `spark.addArtifact`
fails for a file that
Review Comment:
Done in c0924efa51d, without changing the wording except for moving one
sentence: the paragraph is now four, the precedence and the file names, the two
sentences about configured packages, the check of `spark.addArtifact` with what
a call does after a rejection and the files that cannot be read, and what the
check does not cover.
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/ffi/NativeDataSourcePackage.scala:
##########
@@ -177,18 +233,43 @@ object NativeDataSourcePackage extends Logging {
packages.asMap().keySet().asScala.exists(_.path == canonicalPath)
}
- private def invalidManifestError(file: File, reason: String, cause:
Throwable = null) = {
+ private def invalidManifestError(path: String, reason: String, cause:
Throwable = null) = {
QueryExecutionErrors.invalidNativeDataSourcePackageError(
- file.getPath, "INVALID_MANIFEST", Map("reason" -> reason), cause)
+ path, "INVALID_MANIFEST", Map("reason" -> reason), cause)
}
+ /** Reads the package in the given file. */
private def parse(file: File): ReadResult = {
+ readManifest(file, file.getPath).flatMap { manifest =>
+ try {
+ Right(NativeDataSourcePackage(file.getPath, sha256(file), manifest))
+ } catch {
+ case e: IOException => Left(InvalidNativeDataSourcePackage(
Review Comment:
Done in c0924efa51d: `cannotRead(file, e, dataSources = Nil)` next to
`invalidManifestError`, used by `tryRead` and `parse`, and an `invalidPackage`
helper that `cannotRead` and the `invalid` of `readManifest` share, and the doc
of `parse` now says that it reads the manifest and computes the checksum,
without the cache and the log of `tryRead`. Neither reason is tested: the
checksum case needs a file that fails after its manifest was read, and the case
in `tryRead` a file whose attributes cannot be read, which the description
lists as not covered.
--
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]
