asolimando commented on code in PR #25719:
URL: https://github.com/apache/datafusion/pull/25719#discussion_r4187966419
##########
datafusion/physical-plan/src/filter.rs:
##########
@@ -4465,4 +4538,212 @@ mod tests {
);
Ok(())
}
+
+ // ---------------------------------------------------------------
+ // Unit tests for compute_fallback_selectivity
+ // ---------------------------------------------------------------
+
+ #[test]
+ fn test_fallback_selectivity_single_handled_equality() {
+ // col_0 = <expr>, NDV(col_0) = 100 → selectivity = 1/100
+ let schema = Schema::new(vec![Field::new("a", DataType::Int32,
false)]);
+ let predicate: Arc<dyn PhysicalExpr> = binary(
+ col("a", &schema).unwrap(),
+ Operator::Eq,
+ lit(42i32),
+ &schema,
+ )
+ .unwrap();
+ let col_stats = vec![ColumnStatistics {
+ distinct_count: Precision::Inexact(100),
+ ..Default::default()
+ }];
+ let result = compute_fallback_selectivity(&predicate, &col_stats, 20);
+ assert!(
+ (result - 0.01).abs() < 1e-12,
+ "expected 1/100 = 0.01, got {result}"
+ );
+ }
+
+ #[test]
+ fn test_fallback_selectivity_multiple_unhandled_conjuncts() {
+ // s LIKE '%abc' AND t <> 'x' AND u IN ('p','q')
+ // None are handled equalities → selectivity = default once = 0.2
+ let schema = Schema::new(vec![
+ Field::new("s", DataType::Utf8, false),
+ Field::new("t", DataType::Utf8, false),
+ Field::new("u", DataType::Utf8, false),
+ ]);
+ // Simulate three non-equality conjuncts via NotEq operators
+ let pred1 = binary(
+ col("s", &schema).unwrap(),
+ Operator::NotEq,
+ lit("abc"),
+ &schema,
+ )
+ .unwrap();
+ let pred2 = binary(
+ col("t", &schema).unwrap(),
+ Operator::NotEq,
+ lit("x"),
+ &schema,
+ )
+ .unwrap();
+ let pred3 = binary(
+ col("u", &schema).unwrap(),
+ Operator::NotEq,
+ lit("p"),
+ &schema,
+ )
+ .unwrap();
+ let combined: Arc<dyn PhysicalExpr> = conjunction(vec![pred1, pred2,
pred3]);
+ let col_stats = vec![
+ ColumnStatistics::new_unknown(),
+ ColumnStatistics::new_unknown(),
+ ColumnStatistics::new_unknown(),
+ ];
+ let result = compute_fallback_selectivity(&combined, &col_stats, 20);
+ // default_selectivity applied exactly once: 0.2
+ assert!((result - 0.2).abs() < 1e-12, "expected 0.2, got {result}");
+ }
+
+ #[test]
+ fn test_fallback_selectivity_mixed_handled_and_unhandled() {
+ // col_a = 42 AND col_b <> 'x'
+ // col_a has NDV=50, col_b is unhandled
+ // selectivity = (1/50) * 0.2 = 0.004
+ let schema = Schema::new(vec![
+ Field::new("a", DataType::Int32, false),
+ Field::new("b", DataType::Utf8, false),
+ ]);
+ let eq_pred = binary(
+ col("a", &schema).unwrap(),
+ Operator::Eq,
+ lit(42i32),
+ &schema,
+ )
+ .unwrap();
+ let neq_pred = binary(
+ col("b", &schema).unwrap(),
+ Operator::NotEq,
+ lit("x"),
+ &schema,
+ )
+ .unwrap();
+ let combined: Arc<dyn PhysicalExpr> = conjunction(vec![eq_pred,
neq_pred]);
+ let col_stats = vec![
+ ColumnStatistics {
+ distinct_count: Precision::Inexact(50),
+ ..Default::default()
+ },
+ ColumnStatistics::new_unknown(),
+ ];
+ let result = compute_fallback_selectivity(&combined, &col_stats, 20);
+ let expected = (1.0 / 50.0) * 0.2;
+ assert!(
+ (result - expected).abs() < 1e-12,
+ "expected {expected}, got {result}"
+ );
+ }
+
+ #[test]
+ fn test_fallback_selectivity_col_eq_col_uses_max_ndv() {
+ // col_a = col_b, NDV(a)=100, NDV(b)=200
+ // selectivity = 1 / max(100, 200) = 1/200
+ let schema = Schema::new(vec![
+ Field::new("a", DataType::Int32, false),
+ Field::new("b", DataType::Int32, false),
+ ]);
+ let predicate: Arc<dyn PhysicalExpr> = binary(
+ col("a", &schema).unwrap(),
+ Operator::Eq,
+ col("b", &schema).unwrap(),
+ &schema,
+ )
+ .unwrap();
+ let col_stats = vec![
+ ColumnStatistics {
+ distinct_count: Precision::Inexact(100),
+ ..Default::default()
+ },
+ ColumnStatistics {
+ distinct_count: Precision::Inexact(200),
+ ..Default::default()
+ },
+ ];
+ let result = compute_fallback_selectivity(&predicate, &col_stats, 20);
+ assert!(
+ (result - 1.0 / 200.0).abs() < 1e-12,
+ "expected 1/200 = 0.005, got {result}"
+ );
+ }
+
+ #[test]
+ fn test_fallback_selectivity_no_conjuncts_returns_default() {
+ // A single non-equality predicate → default_selectivity once
+ let schema = Schema::new(vec![Field::new("a", DataType::Int32,
false)]);
+ let predicate: Arc<dyn PhysicalExpr> = binary(
+ col("a", &schema).unwrap(),
+ Operator::Gt,
+ lit(10i32),
+ &schema,
+ )
+ .unwrap();
+ let col_stats = vec![ColumnStatistics {
+ distinct_count: Precision::Inexact(100),
+ ..Default::default()
+ }];
+ let result = compute_fallback_selectivity(&predicate, &col_stats, 20);
+ assert!((result - 0.2).abs() < 1e-12, "expected 0.2, got {result}");
+ }
+
+ #[test]
+ fn test_fallback_selectivity_cast_col_not_handled() {
Review Comment:
This is not addressed yet. Your new
`test_filter_statistics_fallback_cast_expr_uses_default_selectivity` does not
test what its doc comment says: it uses a literal, not a scalar subquery, so
the predicate passes `check_support` and the fallback never runs.
The comments inside the test contradict the code, and the assertion
`num_rows != 1000` checks nothing about this PR.
The comment for the test is quoting my wording almost verbatim and it does
not convey _what_ we are testing, but _how_, which is not very informative for
people reading the code.
There is still no test of a `FilterExec` with a scalar subquery predicate,
which is the case this PR fixes.
--
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]
