claudevdm opened a new pull request, #40337:
URL: https://github.com/apache/beam/pull/40337
* AddFiles: regression tests for column bounds and metrics-based partition
inference
These tests describe the correct behavior and fail on master; the commits
that follow fix them one group at a time. Failures at this commit:
Bounds (getFileMetrics):
- millis and nanos timestamps, nanos time: bounds stored in the file's unit
under a micros type, off by 1000x.
- millis time and uint32: ClassCastException (Integer to Long) while
collecting bounds.
Partition inference without a location prefix:
- all-null boolean partition column is registered under flag=false, an
all-null string column under the string "null", and a file lacking the column
under flag=false: the null partition goes through a path string and is parsed
back.
- all-null timestamp under day(ts) is refused: "Text 'null' could not be
parsed".
- a column with statistics switched off is registered under flag=false
although every value is true; an INT96 column is refused with the same parse
error instead of an unknown partition.
- an Avro file on a partitioned table fails the pipeline with a
NullPointerException (null bound maps).
- a millis timestamp file lands in day 19 (1970) instead of 2024-01-01.
- with write.metadata.metrics.default=none, an unrelated uint32 column fails
the pipeline with a ClassCastException during the partition recollection.
testMicrosTimestampBoundsAreUnchanged passes and stays as the control.
* AddFiles: never guess a partition for a column without bounds
Metrics-based partition inference treated a partition source column with no
bounds as all null. That holds for a column whose values are all null, but
bounds are also missing when statistics are switched off, for INT96 timestamps
(Iceberg collects nothing for them) and for Avro files, whose Metrics carry
null bound maps.
- An Avro file on a partitioned table failed the bundle with a
NullPointerException on metrics.lowerBounds().keySet(), so one Avro file failed
the job. Null bound maps are now read as empty.
- A column without bounds is an unknown partition (error row naming the
column and pointing at the location prefix) unless the null partition is
proven: the file is empty, the column's null count equals its value count, or
the Parquet file does not contain the column at all (a file written before the
column existed reads as null everywhere).
Turns green: testAvroFileOnAPartitionedTableIsAnUnknownPartition,
testPartitionColumnWithoutStatisticsIsAnUnknownPartition,
testInt96PartitionColumnIsAnUnknownPartition.
* AddFiles: set the metrics-inferred partition as values, not as a path
string
getPartitionFromMetrics returned PartitionKey.toPath() and the DataFile was
built with withPartitionPath, which parses each value back from text. A null
partition value is written as the text "null", so:
- boolean identity partition: Boolean.valueOf("null") files the data under
flag=false, and queries pruning on the partition return wrong rows;
- string identity partition: the file lands under the string "null";
- int, date and timestamp partitions: the parse fails and the file is
refused ("Text 'null' could not be parsed at index 0").
getPartitionFromMetrics now returns the PartitionKey and the DataFile is
built with withPartition. The location-prefix option still goes through the
path, where Iceberg maps __HIVE_DEFAULT_PARTITION__ to null.
Turns green: the all-null boolean, string and day(timestamp) tests and
testFileLackingThePartitionColumnRegistersUnderTheNullPartition.
* AddFiles: recollect partition metrics for the partition columns only
When the file's own metrics lack bounds for a partition source column (e.g.
write.metadata.metrics.default=none), the partition is inferred from metrics
recollected with "full" for the partition columns. Every other column fell back
to Iceberg's default mode, so bounds were computed for the whole file, and a
column Iceberg cannot collect bounds for (uint32, millis time:
ClassCastException) failed the bundle from a code path that only catches
UnknownPartitionException.
The recollection now sets the default mode to none, so only the partition
columns are read. It is also less work on wide files.
Turns green:
testUnrelatedColumnDoesNotBreakInferenceUnderMetricsModeNone.
* AddFiles: store column bounds in the unit of the Iceberg type
Files without field ids resolve through the name mapping, and their column
bounds are what partition inference and query pruning read, so they must be
encoded in the Iceberg type's unit. Iceberg's converter maps TIMESTAMP/TIME in
MILLIS or NANOS to micros types and uint32 to long, but
ParquetUtil.footerMetrics stores the file's raw statistic:
- TIMESTAMP(MILLIS|NANOS), TIME(NANOS): bounds off by 1000x, silently. A
2024 millis timestamp gets a bound in 1970; day(ts) puts the file in the wrong
partition and range predicates prune it away, while the data reads correctly.
- TIME(MILLIS) int32 and UINT32: ClassCastException (Integer to Long) while
collecting bounds, so the file is routed to errors.
Fix: BoundAdjustment, applied in getFileMetrics after the id mapping.
Affected int32 columns are presented to Iceberg without their annotation so it
computes plain int bounds instead of throwing; every affected lower/upper bound
is then rewritten as an 8-byte long in the Iceberg unit (millis x1000; nanos
/1000 with lower rounded down and upper up so bounds stay conservative; uint32
via toUnsignedLong). Counts are untouched. Files whose columns need no
adjustment take the old path unchanged.
Turns green: the six bounds tests' remaining failures and
testMillisTimestampFileLandsInItsDayPartition.
* AddFiles: refuse a file whose partition column holds both nulls and values
Found while fixing the partition inference; not in the fuzz findings. Column
bounds ignore nulls, so a partition source column holding true and null has
bounds true..true and the file was registered under flag=true. Its null rows
belong to the null partition, so a query pruning on the partition silently
loses them.
Such a file spans two partitions and is now an unknown partition (error row
naming the column). A transform that maps every value to null (void) is
unaffected: nulls and values land in the same partition.
Test and fix together: on the previous commit
testPartitionColumnWithNullsAndValuesIsAnUnknownPartition fails with the
file registered and no error row.
* fix boundadjustment
* comments
* missing file
* comments
* trigger tests
* move utils, tests
* fixes
* traverse from newest schema
**Please** add a meaningful description for your change here
------------------------
Thank you for your contribution! Follow this checklist to help us
incorporate your contribution quickly and easily:
- [ ] Mention the appropriate issue in your description (for example:
`addresses #123`), if applicable. This will automatically add a link to the
pull request in the issue. If you would like the issue to automatically close
on merging the pull request, comment `fixes #<ISSUE NUMBER>` instead.
- [ ] Update `CHANGES.md` with noteworthy changes.
- [ ] If this contribution is large, please file an Apache [Individual
Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
See the [Contributor Guide](https://beam.apache.org/contribute) for more
tips on [how to make review process
smoother](https://github.com/apache/beam/blob/master/CONTRIBUTING.md#make-the-reviewers-job-easier).
To check the build health, please visit
[https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md](https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md)
GitHub Actions Tests Status (on master branch)
------------------------------------------------------------------------------------------------
[](https://github.com/apache/beam/actions?query=workflow%3A%22Build+python+source+distribution+and+wheels%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Python+Tests%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Java+Tests%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Go+tests%22+branch%3Amaster+event%3Aschedule)
See [CI.md](https://github.com/apache/beam/blob/master/CI.md) for more
information about GitHub Actions CI or the [workflows
README](https://github.com/apache/beam/blob/master/.github/workflows/README.md)
to see a list of phrases to trigger workflows.
--
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]
