Copilot commented on code in PR #21756:
URL: https://github.com/apache/datafusion/pull/21756#discussion_r3115713255
##########
datafusion/catalog-listing/src/helpers.rs:
##########
@@ -340,17 +340,28 @@ fn filter_partitions(
Ok(None)
}
+/// Returns `Ok(None)` when the file is not inside a valid partition path
+/// (e.g. a stale file in the table root directory). Such files are skipped
+/// because hive-style partition values are never null and there is no valid
+/// value to assign for non-partitioned files.
fn try_into_partitioned_file(
object_meta: ObjectMeta,
partition_cols: &[(String, DataType)],
table_path: &ListingTableUrl,
-) -> Result<PartitionedFile> {
+) -> Result<Option<PartitionedFile>> {
let cols = partition_cols.iter().map(|(name, _)| name.as_str());
let parsed = parse_partitions_for_path(table_path, &object_meta.location,
cols);
+ let Some(parsed) = parsed else {
+ debug!(
+ "Skipping file outside partition structure: {}",
+ object_meta.location
+ );
Review Comment:
This `debug!` log will often duplicate the `debug!` emitted inside
`parse_partitions_for_path` for the same file (it already logs on partition
mismatch). To avoid double logging/noisy listings, consider either removing
this log, lowering it to `trace!`, or logging only for the
`strip_prefix`-failure case (which currently returns `None` without logging).
```suggestion
```
--
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]
