deepakpanda93 commented on code in PR #19569:
URL: https://github.com/apache/hudi/pull/19569#discussion_r3747956455
##########
hudi-spark-datasource/hudi-spark-common/src/main/scala/org/apache/hudi/DefaultSource.scala:
##########
@@ -94,19 +94,32 @@ class DefaultSource extends RelationProvider
val path = optParams.get("path")
val readPathsStr = optParams.get(DataSourceReadOptions.READ_PATHS.key)
- if (path.isEmpty && readPathsStr.isEmpty) {
- throw new HoodieException(s"'path' or '${READ_PATHS.key()}' or both must
be specified.")
+ // These are three independent problems and each gets its own message.
They used to share one
+ // condition, so a user passing plain partition paths through read.paths
was told that their
+ // non-glob paths were unsupported glob paths, which is neither true nor
actionable.
+ //
+ // Checked before any storage handle is built, so a doomed call does not
open one first.
+ //
+ // read.paths is reported ahead of a glob in 'path' because it is the more
fundamental of the
+ // two: the option is gone entirely, and its replacement resolves the glob
case as well.
+ if (readPathsStr.isDefined) {
+ // Deliberately fires for an explicitly empty value too: setting the key
at all is asking for
+ // an option that no longer exists. 1.2.0 already rejected "" here,
since Some("") made the
+ // old readPaths.nonEmpty check true, so this keeps the rejection and
only reworks the wording.
+ throw new HoodieException(
+ s"'${READ_PATHS.key()}' is no longer supported as of Hudi 1.2.0.
${DefaultSource.LOAD_BASE_PATH_INSTEAD}")
Review Comment:
Verified, and you are right about the streaming half — I had traced this to
the incremental query
path only, and missed that streaming reaches the same relations.
`HoodieStreamSourceV1:186` and
`HoodieStreamSourceV2:162` construct `IncrementalRelationV1` /
`IncrementalRelationV2` directly, so a
streaming read reaches it too.
**It is reachable, not dead.** The bootstrap branch is entered when a commit
in the scanned range is
the metadata bootstrap instant (`IncrementalRelationV1:179-181`):
```scala
if (HoodieTimeline.METADATA_BOOTSTRAP_INSTANT_TS == commit.requestedTime) {
metaBootstrapFileIdToFullPath ++=
metadata.getFileIdAndFullPaths(basePath)...
}
...
if (metaBootstrapFileIdToFullPath.nonEmpty) {
df = sqlContext.sparkSession.read.format("hudi_v1").schema(prunedSchema)
.option(DataSourceReadOptions.READ_PATHS.key,
filteredMetaBootstrapFullPaths.mkString(","))
...
.load()
}
```
`METADATA_BOOTSTRAP_INSTANT_TS` is `"00000000000001"`, so this fires when an
incremental or streaming
read spans the bootstrap instant of a metadata-bootstrapped table. Narrow,
but a real scenario rather
than unreachable code. `DefaultSource.shortName()` is `"hudi_v1"` and
`.schema(...)` routes through
`SchemaRelationProvider`, so it lands in the method this PR touches, with
`READ_PATHS` set and no
`path`.
**Not a regression, and not fixed here.** It threw before this PR
(`readPaths.nonEmpty` in the old
combined guard) and it throws after; only the wording moves from the glob
message to the read.paths
message. `BaseFileOnlyRelation:162` reads `READ_PATHS` too and has been
equally unreachable since
1.2.0, for the same reason.
Your sharpest point is the one I had not put into words: on that path
**Hudi** set the option, not the
user, so "load the base path and filter on the partition columns" is advice
the caller cannot act on.
That was equally true of the old glob message, so no user is worse off, but
it does mean this PR
documents the option as fatal while Hudi still sets it internally.
Migrating those callers is the right fix and it is more than a message
change: they need rerouting to
the file group reader, which is the work tracked by #17327 / HUDI-8747 for
removing read paths
properly. Keeping it out of this PR so that this one stays a
behaviour-preserving message and docs
change, and filing it separately with the trace above.
--
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]
