sam-1112 commented on code in PR #7196:
URL: https://github.com/apache/datafusion-comet/pull/7196#discussion_r4432337937
##########
native/core/src/parquet/field_resolver.rs:
##########
@@ -204,6 +241,55 @@ impl<'a> FieldResolver<'a> {
)),
}
}
+
+ /// Resolve claims in file order, with id matches ahead of shadows and
name matches.
+ /// Repeated requested ids keep the last claimant, as Spark parquet-mr
does. Duplicate file ids are
+ /// checked across the whole requested group, including unprojected root
fields (#7006).
+ /// Name ambiguity is left to the caller: the root checks referenced
columns only, whereas
+ /// nested conversion validates every requested field in
[`Self::resolve_all`].
+ pub(crate) fn file_claims(&self) -> DataFusionResult<Vec<FieldClaim>> {
+ let mut claims = vec![FieldClaim::Unchanged; self.file.len()];
+ for requested in 0..self.requested.len() {
+ match self.select(requested)? {
+ Selection::ById(matched) => {
+ if let Some(file) = matched {
+ claims[file].claim(FieldClaim::IdMatch(requested));
+ }
+ for &file in self.name_matches(requested) {
+ claims[file].claim(FieldClaim::Shadowed);
+ }
+ }
+ Selection::ByName(matches) => {
+ for &file in matches {
+ claims[file].claim(FieldClaim::NameMatch(requested));
+ }
+ }
+ }
+ }
+ Ok(claims)
+ }
+
+ /// Resolve a nested group in requested order, giving each file field to
its strongest
+ /// claimant only. Missing and losing fields read NULL. Validate
independent selections
+ /// first, in requested order, so arbitration cannot hide ambiguity or
change which of
+ /// several invalid selections reports its error first.
+ pub(crate) fn resolve_all(&self) -> DataFusionResult<Vec<Option<usize>>> {
+ let mut resolved = (0..self.requested.len())
+ .map(|requested| self.resolve(requested))
+ .collect::<DataFusionResult<Vec<_>>>()?;
+ let claims = self.file_claims()?;
Review Comment:
<p>I added a fast path for nested groups resolved entirely by ID. These
groups skip both the claim vector and the shadow-name lookups. Mixed groups
initialize ownership bookkeeping at the first name selection, reusing the
preceding validated ID selections. Independent type validation and ambiguity
checks remain intact.</p>
<p>I reran six alternating pairs against main (<code>7deeba21a</code>),
using the same harness and lockfile, with LTO off and 16 codegen units for both
builds:</p>
Benchmark | main | Fast-path head | Change
-- | -- | -- | --
convert/64 | 13.880 µs | 13.694 µs | −1.34%
decode_and_convert/64 | 15.337 ms | 15.199 ms | −0.90%
convert/8192 | 13.990 µs | 13.782 µs | −1.49%
decode_and_convert/8192 | 1.112 ms | 1.110 ms | Within noise
--
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]
