github-actions[bot] commented on code in PR #66597:
URL: https://github.com/apache/doris/pull/66597#discussion_r3755321658
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/lance/source/LanceScanNode.java:
##########
@@ -154,14 +154,24 @@ public List<Split> getSplits(int numBackends) throws
UserException {
plannedVersion = metadata.getVersion();
plannedFragments = metadata.getFragments().size();
Set<Long> fragmentIds = new HashSet<>();
- List<Split> splits = new ArrayList<>(plannedFragments);
+ long targetRows = 1;
for (LanceTableMetadata.LanceFragmentInfo fragment :
metadata.getFragments()) {
if (!fragmentIds.add(fragment.getId())) {
throw new UserException("Duplicate Lance fragment id " +
fragment.getId()
+ " at dataset version " + metadata.getVersion());
}
- splits.add(new LanceSplit(metadata.getDatasetUri(),
metadata.getVersion(),
- fragment.getId(), fragment.getRowCount()));
+ targetRows = Math.max(targetRows,
Math.max(fragment.getPhysicalRows(), 1));
+ }
+
+ // Use the largest fragment as one standard split so smaller
fragments keep
+ // their relative row-count weight during backend assignment.
Physical rows drive
+ // the weight because the BE legacy reader scans physical batches
before deletions.
+ List<Split> splits = new ArrayList<>(plannedFragments);
+ for (LanceTableMetadata.LanceFragmentInfo fragment :
metadata.getFragments()) {
+ LanceSplit split = new LanceSplit(metadata.getDatasetUri(),
metadata.getVersion(),
+ fragment.getId(), fragment.getPhysicalRows());
Review Comment:
[P2] Keep stable-format fragments on a deletion-aware weight
Physical rows match the legacy reader, but [Lance 4.0.1 dispatches stable
storage to a different read
path](https://github.com/lance-format/lance/blob/v4.0.1/rust/lance/src/dataset/scanner.rs#L2657-L2688).
Its `FilteredReadExec` [turns the deletion vector into valid
ranges](https://github.com/lance-format/lance/blob/v4.0.1/rust/lance/src/io/exec/filtered_read.rs#L827-L839)
and passes those ranges to `read_ranges()` before data I/O. For a multi-page
stable fragment with 1,000,000 physical rows and 990,000 contiguous tombstones,
the reader can skip pages wholly covered by the deleted region and read/decode
only the live ranges plus boundary-page and deletion-metadata overhead, yet
this assigns the same weight as a fully live fragment. The earlier thread
covered legacy underweighting; this is the opposite failure in the separate
stable path. Please carry a format-aware work metric through both catalog and
TVF builders and cover a deletion-heavy stable fixture.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/lance/LanceMetadataLoader.java:
##########
@@ -86,7 +86,8 @@ private static LanceTableMetadata loadInternal(String
datasetUri, Map<String, St
List<LanceTableMetadata.LanceFragmentInfo> fragments = new
ArrayList<>();
for (Fragment fragment : dataset.getFragments()) {
fragments.add(new LanceTableMetadata.LanceFragmentInfo(
- fragment.getId(), fragment.metadata().getNumRows()));
+ fragment.getId(), fragment.metadata().getNumRows(),
+ fragment.metadata().getPhysicalRows()));
Review Comment:
[P2] Fence broken migration metadata before trusting `physicalRows`
The [pinned Lance
implementation](https://github.com/lance-format/lance/blob/v9.1.0-beta.3/rust/lance/src/dataset/fragment.rs#L1381-L1399)
does not trust cached `physical_rows` when `manifest.writer_version` is
absent: it opens a data file instead because [the v0.8.0 migration from older
deletion-bearing datasets could store a post-deletion count
there](https://github.com/lance-format/lance/issues/1531). Java
`Dataset#getFragments()` exports raw fragment metadata, so `getPhysicalRows()`
here bypasses that fence. For that supported migrated dataset class, a
deletion-heavy fragment can remain severely underweighted even though the BE
derives and reads its actual physical rows. This is distinct from the earlier
logical-row thread: switching from `getNumRows()` to this raw field still fails
when the populated cache is unvouched. Please use a trust-aware SDK/JNI count
or retain standard weights for such manifests, and cover the broken migration
fixture.
--
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]
