slachiewicz commented on code in PR #724:
URL: https://github.com/apache/hudi-rs/pull/724#discussion_r3971651239
##########
crates/core/tests/table_read_tests.rs:
##########
@@ -3477,3 +3477,143 @@ mod incremental_window_boundaries {
Ok(())
}
}
+
+mod hive_style_keygen_partition_pruning {
Review Comment:
Addressed in 76026c0: the three v6 hive-style tests now open the table with
`Table::new_with_options(...,
hoodie.keygen.timebased.timestamp.type=DATE_STRING)` so the keygen transform
engages, and the comments were rewritten to state that explicitly
(column-stats-only pruning is now documented as covered by
`custom_keygen_partition_paths`). Also added
`v9_table_prunes_through_its_persisted_keygen_config` which exercises the
transform from persisted table config alone. All rust tests pass on this head.
##########
crates/core/src/keygen/timestamp_based.rs:
##########
@@ -509,7 +482,13 @@ impl KeyGeneratorFilterTransformer for
TimestampBasedKeyGenerator {
let field = MetaField::PartitionPath.as_ref().to_string();
match filter.operator {
- ExprOperator::Eq | ExprOperator::Ne => {
+ // A negated predicate cannot prune, because this transform is
lossy: a
+ // partition holds every instant of its period, so `ts !=
2024-03-01T14:30:00Z`
+ // is satisfied by almost every row of `ts=2024-03-01`. Mapping
the negation
+ // onto the path would drop that whole partition and lose all of
them. Emit no
+ // partition filter and let the predicate be enforced per row.
+ ExprOperator::Ne | ExprOperator::NotIn => Ok(vec![]),
+ ExprOperator::Eq => {
let dt = self.parse_timestamp(&filter.values[0])?;
Review Comment:
Addressed in cc343d8: `parse_data_for_options` in
`crates/core/src/config/util.rs` now applies Java-properties unescaping (`\\:`,
`\\=`, `\\ `, `\t/\n/\r/\f`, `\uXXXX` with surrogate-pair recombination,
malformed escapes degrade to drop-the-backslash). This fixes the
`input.dateformat=yyyy-MM-dd'T'HH\\:mm\\:ss.SSSZ` -> `TimestampParsingError`
failure on 1.x tables, and
`v9_table_prunes_through_its_persisted_keygen_config` (added in 76026c0) pins
the end-to-end pruned read on the v9 fixture. Also strips `:SIMPLE/:TIMESTAMP`
annotations from `partition.fields` in `table.rs`.
##########
crates/core/tests/gold_parity_tests.rs:
##########
@@ -85,6 +85,13 @@ const NULL_TOKEN: &str = "<null>";
/// being compared.
const EXPECTED_WITHOUT_GOLD: &[&str] = &[
"table_hfile_log_block [MorAvro]",
+ // Partition-pruning fixtures. They exist to pin the on-disk partition
path shape a
+ // hive-style timestamp or custom key generator produces, which is
asserted directly in
+ // table_read_tests, so they carry no Hudi read snapshot.
+ "v6_customkeygen_hivestyle [Cow]",
Review Comment:
Addressed in 76026c0: added `mod custom_keygen_partition_paths` in
`table_read_tests.rs` with
`identity_field_prunes_like_a_plain_partition_column` (v6 fixture) and
`annotated_partition_field_names_still_resolve_v9` (v9 fixture, pins the
`:SIMPLE/:TIMESTAMP` stripping). The `EXPECTED_WITHOUT_GOLD` comment you
flagged now holds for all four fixtures.
--
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]
