From 76c7cd7d72f8f64c8151e9b5d7881944a4761b57 Mon Sep 17 00:00:00 2001 From: Frank McSherry Date: Thu, 27 Aug 2026 11:14:22 -0400 Subject: [PATCH 1/4] transform: make literal constraint detection index-directed `LiteralConstraints` converts a filter predicate to disjunctive normal form before consulting any index, then reads lookup values off the disjuncts. The expansion is multiplicative in the arity of every disjunction in the predicate, including disjunctions over columns that no index covers, so a query pairing an `IN` list with any other `OR` can expand far past what its answer requires. A guard bails out above a predicate size of 1000 nodes, and because it measures the predicate before expanding rather than the expansion it is about to build, it neither bounds the result nor admits cases it could afford. Past the guard the transform silently declines the index, so `shop_id = X AND sku_code IN (<500 values>)` on an index over `(shop_id, sku_code)` falls back to a full scan. Ask the question per candidate index instead. For a given list of key expressions, one pass over the predicate yields the values those expressions may take, and predicate structure that says nothing about them costs a single visit and contributes nothing. The answer is a disjunction of conjunctive boxes, where a box bounds each key field independently, so `a IN (..) AND b IN (..)` is one box denoting the cross product rather than a disjunct per pair, and the box count is bounded by the number of distinct key tuples the predicate admits rather than by how its disjunctions are arranged. Nothing is rewritten in order to be read, which removes the preparation and its lossy undo along with the heuristic that chose between them. Constraint removal becomes a per-predicate test: a predicate that is exactly a bound on the key can go, which is sound because the lookup values intersect what every predicate implies, so a retained predicate can only narrow the key further. Two behaviors that the normal form used to provide as side effects are kept explicitly. Contradictory disjuncts are pruned by a bottom-up pass that reuses the same bounds, covering contradictions that span two predicates. A literal `null` or `false` now reads as unsatisfiable, which is sound for the `AND`/`OR` trees that reach us because neither `min` nor `max` distinguishes `false` from `null` when asking whether a result is `true`, and a filter drops a `null` row just as it drops a `false` one. Adds sqllogictest coverage for an `IN` list on a column the index does not cover, for several such lists at once, for a covered list long enough to exceed any workable size guard, and for the distinction between a cross product that is inherent to a compound key and one that is incidental. --- src/transform/src/literal_constraints.rs | 719 +++++------------- .../src/literal_constraints/key_bounds.rs | 459 +++++++++++ .../transform/literal_constraints.slt | 176 +++++ 3 files changed, 818 insertions(+), 536 deletions(-) create mode 100644 src/transform/src/literal_constraints/key_bounds.rs diff --git a/src/transform/src/literal_constraints.rs b/src/transform/src/literal_constraints.rs index 003bc005ea772..e571a650aa100 100644 --- a/src/transform/src/literal_constraints.rs +++ b/src/transform/src/literal_constraints.rs @@ -12,24 +12,23 @@ //! the Get has a matching index. Convert these to `IndexedFilter` joins, which is a semi-join with //! a constant collection. //! +//! The detection is index-directed: for each candidate index we ask what the predicate says +//! about that index's key expressions, and read the answer off in a single pass. See +//! [`key_bounds`]. +//! //! E.g.: Logically, we go from something like //! `SELECT f1, f2, f3 FROM t WHERE t.f1 = lit1 AND t.f2 = lit2` //! to //! `SELECT f1, f2, f3 FROM t, (SELECT * FROM (VALUES (lit1, lit2))) as filter_list //! WHERE t.f1 = filter_list.column1 AND t.f2 = filter_list.column2` -use std::collections::{BTreeMap, BTreeSet}; +mod key_bounds; use itertools::Itertools; +use key_bounds::{KeyBounds, literal_constrained_exprs, prune_unsatisfiable}; use mz_expr::JoinImplementation::IndexedFilter; -use mz_expr::canonicalize::canonicalize_predicates; -use mz_expr::func::variadic::{And, Or}; -use mz_expr::visit::{Visit, VisitChildren}; -use mz_expr::{BinaryFunc, Id, MapFilterProject, MirRelationExpr, MirScalarExpr, VariadicFunc}; -use mz_ore::collections::CollectionExt; -use mz_ore::iter::IteratorExt; -use mz_ore::stack::RecursionLimitError; -use mz_ore::vec::swap_remove_multiple; +use mz_expr::visit::VisitChildren; +use mz_expr::{BinaryFunc, Id, MapFilterProject, MirRelationExpr, MirScalarExpr}; use mz_repr::{Diff, GlobalId, ReprRelationType, Row}; use crate::TransformCtx; @@ -76,135 +75,99 @@ impl LiteralConstraints { .. } = *relation { - let orig_mfp = mfp.clone(); - - // Preparation for the literal constraints detection. - Self::inline_literal_constraints(&mut mfp); - Self::list_of_predicates_to_and_of_predicates(&mut mfp); - Self::distribute_and_over_or(&mut mfp)?; - Self::unary_and(&mut mfp); - - /// The above preparation might make the MFP more complicated, so we'll later want to - /// either undo the preparation transformations or get back to `orig_mfp`. - fn undo_preparation( - mfp: &mut MapFilterProject, - orig_mfp: &MapFilterProject, - relation: &MirRelationExpr, - relation_type: ReprRelationType, - ) { - // undo list_of_predicates_to_and_of_predicates, distribute_and_over_or, unary_and - // (It undoes the latter 2 through `MirScalarExp::reduce`.) - LiteralConstraints::canonicalize_predicates(mfp, relation, relation_type); - // undo inline_literal_constraints - mfp.optimize(); - // We can usually undo, but sometimes not (see comment on `distribute_and_over_or`), - // so in those cases we might have a more complicated MFP than the original MFP - // (despite the removal of the literal constraints and/or contradicting OR args). - // So let's use the simpler one. - if LiteralConstraints::predicates_size(orig_mfp) - < LiteralConstraints::predicates_size(mfp) - { - *mfp = orig_mfp.clone(); - } - } - - let removed_contradicting_or_args = Self::remove_impossible_or_args(&mut mfp)?; - - // todo: We might want to also call `canonicalize_equivalences`, - // see near the end of literal_constraints.slt. - let inp_typ = typ.clone(); - let key_val = Self::detect_literal_constraints(&mfp, id, transform_ctx); - - match key_val { - None => { - // We didn't find a usable index, so no chance to remove literal constraints. - // But, we might have removed contradicting OR args. - if removed_contradicting_or_args { - undo_preparation(&mut mfp, &orig_mfp, relation, inp_typ); - } else { - // We didn't remove anything, so let's go with the original MFP. - mfp = orig_mfp; - } + // Detection reads a copy with CSE undone, so that a literal equality hidden + // behind a mapped column can still be matched against an index key. Nothing is + // rewritten unless we end up using an index, so there is no preparation to + // undo and no need to compare the result against the original MFP. + let mut probe_mfp = mfp.clone(); + Self::inline_literal_constraints(&mut probe_mfp); + + // Every expression the predicate pins to literal values anywhere. Treating + // these as a key asks what the predicate says about all of them at once, which + // answers two questions at no extra cost: whether the predicate contradicts + // itself, and what key a user whose index was too wide should have indexed. + let constrained = + literal_constrained_exprs(probe_mfp.predicates.iter().map(|(_, p)| p)); + + // Disjuncts that contradict themselves are dead weight in the filter, and + // pruning them needs no index. Done before detection so that detection sees the + // simplified predicate. + let (pruned, empty) = Self::prune_unsatisfiable(&mut probe_mfp, &constrained); + + if empty { + // Some expression is pinned to two different values, so nothing can pass + // the filter. This needs no index. + relation.take_safely(Some(inp_typ)); + } else if let Some((idx_id, key, possible_vals)) = + Self::detect_literal_constraints(&probe_mfp, id, &constrained, transform_ctx) + { + // The lookup enforces every predicate that is exactly a constraint on the + // key, so those can come out of the filter. + if Self::remove_literal_constraints(&mut probe_mfp, &key) || pruned { + // Redo the CSE that inlining undid. + probe_mfp.optimize(); + mfp = probe_mfp; } - Some((idx_id, key, possible_vals)) => { - // We found a usable index. We'll try to remove the corresponding literal - // constraints. - if Self::remove_literal_constraints(&mut mfp, &key) - || removed_contradicting_or_args - { - // We were able to remove the literal constraints or contradicting OR args, - // so we would like to use this new MFP, so we try undoing the preparation. - undo_preparation(&mut mfp, &orig_mfp, relation, inp_typ.clone()); - } else { - // We were not able to remove the literal constraint, so `mfp` is - // equivalent to `orig_mfp`, but `orig_mfp` is often simpler (or the same). - mfp = orig_mfp; - } - // We transform the Get into a semi-join with a constant collection. - - let inp_id = id.clone(); - let filter_list = MirRelationExpr::Constant { - rows: Ok(possible_vals + let inp_id = id.clone(); + let filter_list = MirRelationExpr::Constant { + rows: Ok(possible_vals + .iter() + .map(|val| (val.clone(), Diff::ONE)) + .collect()), + typ: ReprRelationType { + column_types: key .iter() - .map(|val| (val.clone(), Diff::ONE)) - .collect()), - typ: ReprRelationType { - column_types: key - .iter() - .map(|e| e.typ(&inp_typ.column_types).scalar_type.nullable(false)) - .collect(), - // (Note that the key inference for `MirRelationExpr::Constant` inspects - // the constant values to detect keys not listed within the node, but it - // can only detect a single-column key this way. A multi-column key is - // common here, so we explicitly add it.) - keys: vec![(0..key.len()).collect()], - }, - } - .arrange_by(&[(0..key.len()).map(MirScalarExpr::column).collect_vec()]); - - if possible_vals.is_empty() { - // Even better than what we were hoping for: Found contradicting - // literal constraints, so the whole relation is empty. - relation.take_safely(Some(inp_typ)); - } else { - // The common case: We need to build the join which is the main point of - // this transform. - *relation = MirRelationExpr::Join { - // It's important to keep the `filter_list` in the second position. - // Both the lowering and EXPLAIN depend on this. - inputs: vec![ - relation.clone().arrange_by(std::slice::from_ref(&key)), - filter_list, - ], - equivalences: key - .iter() - .enumerate() - .map(|(i, e)| { - vec![(*e).clone(), MirScalarExpr::column(i + inp_typ.arity())] - }) - .collect(), - implementation: IndexedFilter( - inp_id, - idx_id, - key.clone(), - possible_vals, - ), - }; + .map(|e| e.typ(&inp_typ.column_types).scalar_type.nullable(false)) + .collect(), + // (Note that the key inference for `MirRelationExpr::Constant` inspects + // the constant values to detect keys not listed within the node, but it + // can only detect a single-column key this way. A multi-column key is + // common here, so we explicitly add it.) + keys: vec![(0..key.len()).collect()], + }, + } + .arrange_by(&[(0..key.len()).map(MirScalarExpr::column).collect_vec()]); - // Rebuild the MFP to add the projection that removes the columns coming from - // the filter_list side of the join. - let (map, filter, project) = mfp.as_map_filter_project(); - mfp = MapFilterProject::new(inp_typ.arity() + key.len()) - .project(0..inp_typ.arity()) // make the join semi - .map(map) - .filter(filter) - .project(project); - mfp.optimize() - } + if possible_vals.is_empty() { + // Even better than what we were hoping for: Found contradicting + // literal constraints, so the whole relation is empty. + relation.take_safely(Some(inp_typ)); + } else { + // The common case: We need to build the join which is the main point of + // this transform. + *relation = MirRelationExpr::Join { + // It's important to keep the `filter_list` in the second position. + // Both the lowering and EXPLAIN depend on this. + inputs: vec![ + relation.clone().arrange_by(std::slice::from_ref(&key)), + filter_list, + ], + equivalences: key + .iter() + .enumerate() + .map(|(i, e)| { + vec![(*e).clone(), MirScalarExpr::column(i + inp_typ.arity())] + }) + .collect(), + implementation: IndexedFilter(inp_id, idx_id, key.clone(), possible_vals), + }; + + // Rebuild the MFP to add the projection that removes the columns coming from + // the filter_list side of the join. + let (map, filter, project) = mfp.as_map_filter_project(); + mfp = MapFilterProject::new(inp_typ.arity() + key.len()) + .project(0..inp_typ.arity()) // make the join semi + .map(map) + .filter(filter) + .project(project); + mfp.optimize() } + } else if pruned { + probe_mfp.optimize(); + mfp = probe_mfp; } } @@ -213,24 +176,27 @@ impl LiteralConstraints { Ok(()) } - /// Detects literal constraints in an MFP on top of a Get of `id`, and a matching index that can - /// be used to speed up the Filter of the MFP. + /// Detects literal constraints in an MFP on top of a Get of `id`, and a matching index that + /// can be used to speed up the Filter of the MFP. /// /// For example, if there is an index on `(f1, f2)`, and the Filter is /// `(f1 = 3 AND f2 = 5) OR (f1 = 7 AND f2 = 9)`, it returns `Some([f1, f2], [[3,5], [7,9]])`. /// - /// We can use an index if each argument of the OR includes a literal constraint on each of the - /// key fields of the index. Extra predicates inside the OR arguments are ok. + /// The question is asked once per candidate index, about that index's key expressions. + /// Predicate structure that says nothing about those expressions costs a single visit + /// and contributes nothing, which is what keeps the work linear in the predicate size + /// no matter how the disjunctions in it are arranged. /// /// Returns (idx_id, idx_key, values to lookup in the index). fn detect_literal_constraints( mfp: &MapFilterProject, get_id: GlobalId, + constrained: &[MirScalarExpr], transform_ctx: &mut TransformCtx, ) -> Option<(GlobalId, Vec, Vec)> { - // Checks whether an index with the specified key can be used to speed up the given filter. - // See comment of `IndexMatch`. - fn match_index(key: &[MirScalarExpr], or_args: &Vec) -> IndexMatch { + // Checks whether an index with the specified key can be used to speed up the given + // filter. See comment of `IndexMatch`. + fn match_index(key: &[MirScalarExpr], mfp: &MapFilterProject) -> IndexMatch { if key.is_empty() { // Nothing to do with an index that has an empty key. return IndexMatch::UnusableNoSubset; @@ -239,66 +205,32 @@ impl LiteralConstraints { // This is a weird index. Why does it have duplicate key expressions? return IndexMatch::UnusableNoSubset; } - let mut literal_values = Vec::new(); - let mut inv_cast_any = false; - // This starts with all key fields of the index. - // At the end, it will contain a subset S of index key fields such that if the index had - // only S as its key, then the index would be usable. - let mut usable_key_fields = key.iter().collect::>(); - let mut usable = true; - for or_arg in or_args { - let mut row = Row::default(); - let mut packer = row.packer(); - for key_field in key { - let and_args = or_arg.and_or_args(And.into()); - // Let's find a constraint for this key field - if let Some((literal, inv_cast)) = and_args - .iter() - .find_map(|and_arg| and_arg.expr_eq_literal(key_field)) - { - // (Note that the above find_map can find only 0 or 1 result, because - // of `remove_impossible_or_args`.) - packer.push(literal.unpack_first()); - inv_cast_any |= inv_cast; - } else { - // There is an `or_arg` where we didn't find a constraint for a key field, - // so the index is unusable. Throw out the field from the usable fields. - usable = false; - usable_key_fields.remove(key_field); - if usable_key_fields.is_empty() { - return IndexMatch::UnusableNoSubset; - } - } + let bounds = LiteralConstraints::key_bounds(mfp, key); + if bounds.bounds_every_field() { + match bounds.lookup_values() { + Some(vals) => IndexMatch::Usable(vals, bounds.inv_cast), + // Too many values to be worth looking up one at a time. There is no + // narrower index that would help, so there is nothing to advise. + None => IndexMatch::UnusableNoSubset, } - literal_values.push(row); - } - if usable { - // We should deduplicate, because a constraint can be duplicated by - // `distribute_and_over_or`. For example: `IN ('l1', 'l2') AND (a > 0 OR a < 5)`: - // the 2 args of the OR will cause the IN constraints to be duplicated. This doesn't - // alter the meaning of the expression when evaluated as a filter, but if we extract - // those literals 2 times into `literal_values` then the Peek code will look up - // those keys from the index 2 times, leading to duplicate results. - literal_values.sort(); - literal_values.dedup(); - IndexMatch::Usable(literal_values, inv_cast_any) } else { - if usable_key_fields.is_empty() { + let subset = bounds + .bounded_fields() + .into_iter() + .map(|i| key[i].clone()) + .collect_vec(); + if subset.is_empty() { IndexMatch::UnusableNoSubset } else { - IndexMatch::UnusableTooWide( - usable_key_fields.into_iter().cloned().collect_vec(), - ) + IndexMatch::UnusableTooWide(subset) } } } - let or_args = Self::get_or_args(mfp); - let index_matches = transform_ctx .indexes .indexes_on(get_id) - .map(|(index_id, key)| (index_id, key.to_owned(), match_index(key, &or_args))) + .map(|(index_id, key)| (index_id, key.to_owned(), match_index(key, mfp))) .collect_vec(); let result = index_matches @@ -314,8 +246,20 @@ impl LiteralConstraints { .max_by_key(|(_idx_id, key, _vals, inv_cast)| (key.len(), *inv_cast)) .map(|(idx_id, key, vals, _inv_cast)| (idx_id, key, vals)); - if result.is_none() && !or_args.is_empty() { + if result.is_none() { // Let's see if we can give a hint to the user. + // + // The recommendation is index-blind: gather every expression the predicate + // pins to literal values anywhere, then keep those it pins in all cases. An + // index on exactly those would have been usable. + let recommended_key = LiteralConstraints::key_bounds(mfp, constrained) + .bounded_fields() + .into_iter() + .map(|i| constrained[i].clone()) + .collect_vec(); + if recommended_key.is_empty() { + return result; + } index_matches .into_iter() .for_each(|(index_id, index_key, index_match)| { @@ -325,32 +269,11 @@ impl LiteralConstraints { assert!(!usable_subset.is_empty()); // Determine literal values that we would get if the index was on // `usable_subset`. - let literal_values = match match_index(&usable_subset, &or_args) { - IndexMatch::Usable(literal_vals, _) => literal_vals, - _ => unreachable!(), // `usable_subset` would make the index usable. + let bounds = LiteralConstraints::key_bounds(mfp, &usable_subset); + let Some(literal_values) = bounds.lookup_values() else { + return; }; - // Let's come up with a recommendation for what columns to index: - // Intersect literal constraints across all OR args. (Which might - // include columns that are NOT in this index, and therefore not in - // `usable_subset`.) - let recommended_key = or_args - .iter() - .map(|or_arg| { - let and_args = or_arg.and_or_args(And.into()); - and_args - .iter() - .filter_map(|and_arg| and_arg.any_expr_eq_literal()) - .collect::>() - }) - .reduce(|fields1, fields2| { - fields1.intersection(&fields2).cloned().collect() - }) - // The unwrap is safe because above we checked `!or_args.is_empty()` - .unwrap() - .into_iter() - .collect_vec(); - transform_ctx.df_meta.push_optimizer_notice_dedup( IndexTooWideForLiteralConstraints { index_id, @@ -358,7 +281,7 @@ impl LiteralConstraints { usable_subset, literal_values, index_on_id: get_id, - recommended_key, + recommended_key: recommended_key.clone(), }, ) } @@ -370,182 +293,64 @@ impl LiteralConstraints { result } - /// Removes the expressions that [LiteralConstraints::detect_literal_constraints] found, if - /// possible. Returns whether it removed anything. - /// For example, if the key of the detected literal constraint is just `f1`, and we have the - /// expression - /// `(f1 = 3 AND f2 = 5) OR (f1 = 7 AND f2 = 5)`, then this modifies it to `f2 = 5`. - /// However, if OR branches differ in their non-key parts, then we cannot remove the literal - /// constraint. For example, - /// `(f1 = 3 AND f2 = 5) OR (f1 = 7 AND f2 = 555)`, then we cannot remove the `f1` parts, - /// because then the filter wouldn't know whether to check `f2 = 5` or `f2 = 555`. - fn remove_literal_constraints(mfp: &mut MapFilterProject, key: &Vec) -> bool { - let or_args = Self::get_or_args(mfp); - if or_args.len() == 0 { - return false; + /// Prunes unsatisfiable disjuncts from every predicate. See [`prune_unsatisfiable`]. + /// + /// Returns whether anything was pruned, and whether the whole relation is now empty. + fn prune_unsatisfiable(mfp: &mut MapFilterProject, key: &[MirScalarExpr]) -> (bool, bool) { + if key.is_empty() { + return (false, false); } - - // In simple situations it would be enough to check here that if we remove the detected - // literal constraints from each OR arg, then the residual OR args are all equal. - // However, this wouldn't be able to perform the removal when the expression that should - // remain in the end has an OR. This is because conversion to DNF makes duplicates of - // every literal constraint, with different residuals. To also handle this case, we collect - // the possible residuals for every literal constraint row, and check that all sets are - // equal. Example: The user wrote - // `WHERE ((a=1 AND b=1) OR (a=2 AND b=2)) AND (c OR (d AND e))`. - // The DNF of this is - // `(a=1 AND b=1 AND c) OR (a=1 AND b=1 AND d AND e) OR (a=2 AND b=2 AND c) OR (a=2 AND b=2 AND d AND e)`. - // Then `constraints_to_residual_sets` will be: - // [ - // [`a=1`, `b=1`] -> {[`c`], [`d`, `e`]}, - // [`a=2`, `b=2`] -> {[`c`], [`d`, `e`]} - // ] - // After removing the literal constraints we have - // `c OR (d AND e)` - let mut constraints_to_residual_sets = BTreeMap::new(); - or_args.iter().for_each(|or_arg| { - let and_args = or_arg.and_or_args(And.into()); - let (mut constraints, mut residual): (Vec<_>, Vec<_>) = - and_args.iter().cloned().partition(|and_arg| { - key.iter() - .any(|key_field| matches!(and_arg.expr_eq_literal(key_field), Some(..))) - }); - // In every or_arg there has to be some literal constraints, otherwise - // `detect_literal_constraints` would have returned None. - assert!(constraints.len() >= 1); - // `remove_impossible_or_args` made sure that inside each or_arg, each - // expression can be literal constrained only once. So if we find one of the - // key fields being literal constrained, then it's definitely that literal - // constraint that detect_literal_constraints based one of its return values on. - // - // This is important, because without `remove_impossible_or_args`, we might - // have the situation here that or_arg would be something like - // `a = 5 AND a = 8`, of which `detect_literal_constraints` found only the `a = 5`, - // but here we would remove both the `a = 5` and the `a = 8`. - constraints.sort(); - residual.sort(); - let entry = constraints_to_residual_sets - .entry(constraints) - .or_insert_with(BTreeSet::new); - entry.insert(residual); - }); - let residual_sets = constraints_to_residual_sets - .into_iter() - .map(|(_constraints, residual_set)| residual_set) - .collect::>(); - if residual_sets.iter().all_equal() { - // We can remove the literal constraint - assert!(residual_sets.len() >= 1); // We already checked `or_args.len() == 0` above - let residual_set = residual_sets.into_iter().into_first(); - let new_pred = MirScalarExpr::call_variadic( - Or, - residual_set - .into_iter() - .map(|residual| MirScalarExpr::call_variadic(And, residual)) - .collect::>(), - ); - let (map, _predicates, project) = mfp.as_map_filter_project(); + let (map, mut predicates, project) = mfp.as_map_filter_project(); + let (changed, empty) = prune_unsatisfiable(predicates.iter_mut(), key); + if changed { *mfp = MapFilterProject::new(mfp.input_arity) .map(map) - .filter(std::iter::once(new_pred)) + .filter(predicates) .project(project); - - true - } else { - false } + (changed, empty) } - /// 1. Removes such OR args in which there are contradicting literal constraints. - /// 2. Also, if an OR arg doesn't have any contradiction, this fn just deduplicates - /// the AND arg list of that OR arg. (Might additionally sort all AND arg lists.) - /// - /// Returns whether it performed any removal or deduplication. + /// What the MFP's predicates jointly imply about `key`. /// - /// Example for 1: - /// ` OR (a = 5 AND a = 5 AND a = 8) OR ` - /// --> - /// ` OR ` - /// - /// Example for 2: - /// ` OR (a = 5 AND a = 5 AND b = 8) OR ` - /// --> - /// ` OR (a = 5 AND b = 8) OR ` - fn remove_impossible_or_args(mfp: &mut MapFilterProject) -> Result { - let mut or_args = Self::get_or_args(mfp); - if or_args.len() == 0 { - return Ok(false); - } - let mut to_remove = Vec::new(); - let mut changed = false; - or_args.iter_mut().enumerate().for_each(|(i, or_arg)| { - if let MirScalarExpr::CallVariadic { - func: VariadicFunc::And(And), - exprs: and_args, - } = or_arg - { - if and_args - .iter() - .any(|e| e.impossible_literal_equality_because_types()) - { - changed = true; - to_remove.push(i); - } else { - and_args.sort_by_key(|e: &MirScalarExpr| e.invert_casts_on_expr_eq_literal()); - let and_args_before_dedup = and_args.clone(); - and_args - .dedup_by_key(|e: &mut MirScalarExpr| e.invert_casts_on_expr_eq_literal()); - if *and_args != and_args_before_dedup { - changed = true; - } - // Deduplicated, so we cannot have something like `a = 5 AND a = 5`. - // This means that if we now have ` = AND = `, - // then `literal1` is definitely not the same as `literal2`. This means that this - // whole or_arg is a contradiction, because it's something like `a = 5 AND a = 8`. - let mut literal_constrained_exprs = and_args - .iter() - .filter_map(|and_arg| and_arg.any_expr_eq_literal()); - if !literal_constrained_exprs.all_unique() { - changed = true; - to_remove.push(i); - } - } - } else { - // `unary_and` made sure that each OR arg is an AND - unreachable!("OR arg was not an AND in remove_impossible_or_args"); - } - }); - // We remove the marked OR args. - // (If the OR has 0 or 1 args remaining, then `reduce_and_canonicalize_and_or` will later - // further simplify.) - swap_remove_multiple(&mut or_args, to_remove); - // Rebuild the MFP if needed - if changed { - let new_predicates = vec![MirScalarExpr::call_variadic(Or, or_args)]; - let (map, _predicates, project) = mfp.as_map_filter_project(); - *mfp = MapFilterProject::new(mfp.input_arity) - .map(map) - .filter(new_predicates) - .project(project); - Ok(true) - } else { - Ok(false) - } + /// The predicate list is an implicit conjunction, so the per-predicate bounds are + /// combined the same way an `AND` node's arguments would be. + fn key_bounds(mfp: &MapFilterProject, key: &[MirScalarExpr]) -> KeyBounds { + KeyBounds::conjunction(mfp.predicates.iter().map(|(_, p)| p), key) } - /// Returns the arguments of the predicate's top-level OR as a Vec. - /// If there is no top-level OR, then interpret the predicate as a 1-arg OR, i.e., return a - /// 1-element Vec. + /// Removes the predicates that are exactly constraints on `key`, since the lookup that + /// [LiteralConstraints::detect_literal_constraints] found now enforces them. Returns + /// whether it removed anything. /// - /// Assumes that [LiteralConstraints::list_of_predicates_to_and_of_predicates] has already run. - fn get_or_args(mfp: &MapFilterProject) -> Vec { - assert_eq!(mfp.predicates.len(), 1); // list_of_predicates_to_and_of_predicates ensured this - let (_, pred) = mfp.predicates.get(0).unwrap(); - pred.and_or_args(Or.into()) + /// A predicate is removable when it is equivalent to a bound on the key fields, so that + /// dropping it loses nothing. `(f1 = 3 AND f2 = 5) OR (f1 = 7 AND f2 = 5)` with a key + /// of just `f1` is not removable, because the residual `f2 = 5` is entangled with the + /// `f1` constraint. `f1 IN (3, 7) AND f2 = 5` is: the first predicate goes, the second + /// stays. + /// + /// NOTE: This is sound only because the lookup values are the intersection of what + /// *every* predicate implies, including the ones we keep. So the retained predicates + /// can only narrow the key further, never widen it past what a removed predicate + /// allowed. + fn remove_literal_constraints(mfp: &mut MapFilterProject, key: &[MirScalarExpr]) -> bool { + let (map, predicates, project) = mfp.as_map_filter_project(); + let kept = predicates + .into_iter() + .filter(|p| !KeyBounds::extract(p, key).exact()) + .collect_vec(); + if kept.len() == mfp.predicates.len() { + return false; + } + *mfp = MapFilterProject::new(mfp.input_arity) + .map(map) + .filter(kept) + .project(project); + true } - /// Makes the job of [LiteralConstraints::detect_literal_constraints] easier by undoing some CSE to - /// reconstruct literal constraints. + /// Makes the job of [LiteralConstraints::detect_literal_constraints] easier by undoing some + /// CSE to reconstruct literal constraints. fn inline_literal_constraints(mfp: &mut MapFilterProject) { let mut should_inline = vec![false; mfp.input_arity + mfp.expressions.len()]; // Mark those expressions for inlining that contain a subexpression of the form @@ -588,172 +393,14 @@ impl LiteralConstraints { // Perform the marked inlinings. mfp.perform_inlining(should_inline); } - - /// MFPs have a Vec of predicates `[p1, p2, ...]`, which logically represents `p1 AND p2 AND ...`. - /// This function performs this conversion. Note that it might create a variadic AND with - /// 0 or 1 args, so the resulting predicate Vec always has exactly 1 element. - fn list_of_predicates_to_and_of_predicates(mfp: &mut MapFilterProject) { - // Rebuild the MFP. (Unfortunately, we cannot modify the predicates in place, because MFP - // predicates also have a "before" field, which we need to update. (`filter` will recompute - // these.) - let (map, _predicates, project) = mfp.as_map_filter_project(); - let new_predicates = vec![MirScalarExpr::call_variadic( - And, - mfp.predicates.iter().map(|(_, p)| p.clone()).collect(), - )]; - *mfp = MapFilterProject::new(mfp.input_arity) - .map(map) - .filter(new_predicates) - .project(project); - } - - /// Call [mz_expr::canonicalize::canonicalize_predicates] on each of the predicates in the MFP. - fn canonicalize_predicates( - mfp: &mut MapFilterProject, - relation: &MirRelationExpr, - relation_type: ReprRelationType, - ) { - let (map, mut predicates, project) = mfp.as_map_filter_project(); - let typ_after_map = relation - .clone() - .map(map.clone()) - .typ_with_input_types(&[relation_type]); - canonicalize_predicates(&mut predicates, &typ_after_map.column_types); - // Rebuild the MFP with the new predicates. - *mfp = MapFilterProject::new(mfp.input_arity) - .map(map) - .filter(predicates) - .project(project); - } - - /// Distribute AND over OR + do flatten_and_or until fixed point. - /// This effectively converts to disjunctive normal form (DNF) (i.e., an OR of ANDs), because - /// [MirScalarExpr::reduce] did Demorgans and double-negation-elimination. So after - /// [MirScalarExpr::reduce], we get here a tree of AND/OR nodes. A distribution step lifts an OR - /// up the tree by 1 level, and a [MirScalarExpr::flatten_associative] merges two ORs that are at - /// adjacent levels, so eventually we'll end up with just one OR that is at the top of the tree, - /// with ANDs below it. - /// For example: - /// (a || b) && (c || d) - /// -> - /// ((a || b) && c) || ((a || b) && d) - /// -> - /// (a && c) || (b && c) || (a && d) || (b && d) - /// (This is a variadic OR with 4 arguments.) - /// - /// Example: - /// User wrote `WHERE (a,b) IN ((1,2), (1,4), (8,5))`, - /// from which [MirScalarExpr::undistribute_and_or] made this before us: - /// (#0 = 1 AND (#1 = 2 OR #1 = 4)) OR (#0 = 8 AND #1 = 5) - /// And now we distribute the first AND over the first OR in 2 steps: First to - /// ((#0 = 1 AND #1 = 2) OR (#0 = 1 AND #1 = 4)) OR (#0 = 8 AND #1 = 5) - /// then [MirScalarExpr::flatten_associative]: - /// (#0 = 1 AND #1 = 2) OR (#0 = 1 AND #1 = 4) OR (#0 = 8 AND #1 = 5) - /// - /// Note that [MirScalarExpr::undistribute_and_or] is not exactly an inverse to this because - /// 1) it can undistribute both AND over OR and OR over AND. - /// 2) it cannot always undo the distribution, because an expression might have multiple - /// overlapping undistribution opportunities, see comment there. - fn distribute_and_over_or(mfp: &mut MapFilterProject) -> Result<(), RecursionLimitError> { - mfp.predicates.iter_mut().try_for_each(|(_, p)| { - let mut old_p = MirScalarExpr::column(0); - while old_p != *p { - let size = p.size(); - // We might make the expression exponentially larger, so we should have some limit. - // Below 1000 (e.g., a single IN list of ~300 elements, or 3 IN lists of 4-5 - // elements each), we are <10 ms for a single IN list, and even less for multiple IN - // lists. - if size > 1000 { - break; - } - old_p = p.clone(); - p.visit_mut_post(&mut |e: &mut MirScalarExpr| { - if let MirScalarExpr::CallVariadic { - func: VariadicFunc::And(And), - exprs: and_args, - } = e - { - if let Some((i, _)) = and_args.iter().enumerate().find(|(_i, a)| { - matches!( - a, - MirScalarExpr::CallVariadic { - func: VariadicFunc::Or(Or), - .. - } - ) - }) { - // We found an AND whose ith argument is an OR. We'll distribute the other - // args of the AND over this OR. - let mut or = and_args.swap_remove(i); - let to_distribute = - MirScalarExpr::call_variadic(And, (*and_args).clone()); - if let MirScalarExpr::CallVariadic { - func: VariadicFunc::Or(Or), - exprs: ref mut or_args, - } = or - { - or_args.iter_mut().for_each(|a| { - *a = a.clone().and(to_distribute.clone()); - }); - } else { - unreachable!(); // because the `find` found a match already - } - *e = or; // The modified OR will be the new top-level expr. - } - } - }); - p.visit_mut_post(&mut |e: &mut MirScalarExpr| { - e.flatten_associative(); - }); - } - Ok(()) - }) - } - - /// For each of the arguments of the top-level OR (if no top-level OR, then interpret the whole - /// expression as a 1-arg OR, see [LiteralConstraints::get_or_args]), check if it's an AND, and - /// if not, then wrap it in a 1-arg AND. - fn unary_and(mfp: &mut MapFilterProject) { - let mut or_args = Self::get_or_args(mfp); - let mut changed = false; - or_args.iter_mut().for_each(|or_arg| { - if !matches!( - or_arg, - MirScalarExpr::CallVariadic { - func: VariadicFunc::And(And), - .. - } - ) { - *or_arg = MirScalarExpr::call_variadic(And, vec![or_arg.clone()]); - changed = true; - } - }); - if changed { - let new_predicates = vec![MirScalarExpr::call_variadic(Or, or_args)]; - let (map, _predicates, project) = mfp.as_map_filter_project(); - *mfp = MapFilterProject::new(mfp.input_arity) - .map(map) - .filter(new_predicates) - .project(project); - } - } - - fn predicates_size(mfp: &MapFilterProject) -> usize { - let mut sum = 0; - for (_, p) in mfp.predicates.iter() { - sum = sum + p.size(); - } - sum - } } /// Whether an index is usable to speed up a Filter with literal constraints. #[derive(Clone)] enum IndexMatch { - /// The index is usable, that is, each OR argument constrains each key field. + /// The index is usable, that is, the predicate bounds every key field to literal values. /// - /// The `Vec` has the constraining literal values, where each Row corresponds to one OR - /// argument, and each value in the Row corresponds to one key field. + /// The `Vec` has the key values to look up, one Row per value of the whole key. /// /// The `bool` indicates whether we needed to inverse cast equalities to match them up with key /// fields. The inverse cast enables index usage when an implicit cast is wrapping a key field. diff --git a/src/transform/src/literal_constraints/key_bounds.rs b/src/transform/src/literal_constraints/key_bounds.rs new file mode 100644 index 0000000000000..0285db6f3774a --- /dev/null +++ b/src/transform/src/literal_constraints/key_bounds.rs @@ -0,0 +1,459 @@ +// Copyright Materialize, Inc. and contributors. All rights reserved. +// +// Use of this software is governed by the Business Source License +// included in the LICENSE file. +// +// As of the Change Date specified in that file, in accordance with +// the Business Source License, use of this software will be governed +// by the Apache License, Version 2.0. + +//! Index-directed extraction of literal constraints from a filter predicate. +//! +//! The question this module answers is always asked about a specific list of key +//! expressions: "given that this predicate holds, which values can these expressions +//! take?" Everything in the predicate that says nothing about those expressions is +//! invisible to the answer, and costs a single visit of the node. +//! +//! The answer is a [`KeyBounds`]: a disjunction of conjunctive boxes, where a box bounds +//! each key field independently. Two shapes motivate the representation. +//! +//! * `a IN (1, 2) AND b IN (3, 4)` is one box, `{a: {1,2}, b: {3,4}}`. The four key values +//! are the cross product, formed over datums rather than over expression nodes. +//! * `(a, b) IN ((1, 3), (2, 4))` is two boxes, `{a: {1}, b: {3}}` and `{a: {2}, b: {4}}`. +//! Collapsing it to one box would admit `(1, 4)`, which the predicate rejects. +//! +//! `AND` intersects boxes pairwise and `OR` concatenates them, so the box count is bounded +//! by the number of distinct key tuples the predicate admits. It does not grow with +//! disjunctions over columns that the index does not cover. +//! +//! NOTE: A `KeyBounds` is only ever a *sound* bound. Over-approximating is always safe for +//! choosing lookup values, because the residual filter still runs and the constant +//! collection the lookups become has distinct rows, so the semi-join cannot duplicate. +//! Removing a constraint from the filter is a different claim, and requires +//! [`KeyBounds::exact`]. + +use std::collections::btree_map::Entry; +use std::collections::{BTreeMap, BTreeSet}; + +use itertools::Itertools; +use mz_expr::MirScalarExpr; +use mz_expr::VariadicFunc; +use mz_expr::func::variadic::{And, Or}; +use mz_repr::Row; + +/// Largest number of boxes we will carry. Beyond this we widen, trading exactness for a +/// bound on our own work. The cap is on the disjunction width, not on the number of key +/// values a box denotes, so a wide `IN` list over a single column costs one box. +const MAX_BOXES: usize = 1024; + +/// Largest number of key values we will ask an index to look up. Above this a full scan +/// plus filter is the better plan, and the constant collection would itself be a burden. +const MAX_LOOKUP_VALUES: usize = 100_000; + +/// The values a single key field may take. `None` means the predicate does not bound it. +/// +/// `Some` is never empty: a field bounded to no values makes its whole box unsatisfiable, +/// and such boxes are dropped rather than stored. +type FieldBound = Option>; + +/// One conjunctive bound on all key fields, entry `i` bounding key field `i`. +type KeyBox = Vec; + +/// What a predicate implies about a list of key expressions. +#[derive(Clone, Debug)] +pub struct KeyBounds { + /// The key can only take a value that falls inside at least one of these boxes. + /// + /// An empty list therefore means the predicate is never satisfied. + boxes: Vec, + /// Whether `boxes` characterizes the predicate exactly, so that the predicate is + /// equivalent to "the key falls in one of these boxes". + /// + /// False either because the predicate constrains something besides the key fields, or + /// because we widened to stay inside [`MAX_BOXES`]. Only an exact bound may be removed + /// from the filter. + exact: bool, + /// Whether matching a key field required inverting a cast on it. Reported so that the + /// caller can prefer an index whose key needs no inversion. + pub inv_cast: bool, + /// The number of key fields, which is the width of every box. + arity: usize, +} + +impl KeyBounds { + /// The bound of a predicate we cannot read: every key value is admissible, and the + /// predicate is not equivalent to that, so it must stay in the filter. + fn top(arity: usize) -> Self { + KeyBounds { + boxes: vec![vec![None; arity]], + exact: false, + inv_cast: false, + arity, + } + } + + /// The bound of a predicate that is always satisfied, which is the identity of `and`. + /// + /// NOTE: This differs from [`KeyBounds::top`] only in `exact`, and the difference + /// matters. `top` stands for "we could not read this predicate", so it must not be + /// removed from the filter. `unit` stands for "there is nothing here to read". + fn unit(arity: usize) -> Self { + KeyBounds { + boxes: vec![vec![None; arity]], + exact: true, + inv_cast: false, + arity, + } + } + + /// The bound of a predicate that is never satisfied. + fn bottom(arity: usize) -> Self { + KeyBounds { + boxes: Vec::new(), + exact: true, + inv_cast: false, + arity, + } + } + + /// Extracts what a conjunction of predicates jointly says about `key`. + pub fn conjunction<'a>( + predicates: impl IntoIterator, + key: &[MirScalarExpr], + ) -> Self { + Self::conjunction_of( + predicates.into_iter().map(|p| Self::extract(p, key)), + key.len(), + ) + } + + /// The bound implied by all of `args` holding. + fn conjunction_of(args: impl IntoIterator, arity: usize) -> Self { + args.into_iter().fold(Self::unit(arity), Self::and) + } + + /// Extracts what `predicate` says about `key`. + /// + /// Linear in the size of `predicate`, apart from the box arithmetic, which is bounded + /// by [`MAX_BOXES`]. + pub fn extract(predicate: &MirScalarExpr, key: &[MirScalarExpr]) -> Self { + mz_ore::stack::maybe_grow(|| match predicate { + MirScalarExpr::CallVariadic { + func: VariadicFunc::And(And), + exprs, + } => Self::conjunction_of(exprs.iter().map(|e| Self::extract(e, key)), key.len()), + MirScalarExpr::CallVariadic { + func: VariadicFunc::Or(Or), + exprs, + } => Self::disjunction(exprs.iter().map(|e| Self::extract(e, key)), key.len()), + _ => Self::leaf(predicate, key), + }) + } + + /// Extracts what a predicate with no `AND`/`OR` at its root says about `key`. + fn leaf(predicate: &MirScalarExpr, key: &[MirScalarExpr]) -> Self { + // NOTE: `null` counts as never satisfied because these are filter predicates, where + // a row that evaluates to `null` is dropped just as a `false` one is. A literal + // *error* is not: that row errors out rather than being filtered away, so it stays + // opaque. + if predicate.is_literal_false() || predicate.is_literal_null() { + return Self::bottom(key.len()); + } + // A literal equality whose cast cannot be inverted without erroring is never true. + if predicate.impossible_literal_equality_because_types() { + return Self::bottom(key.len()); + } + let mut result = Self::top(key.len()); + // A single leaf can pin more than one key field, if the key holds both an + // expression and a cast of it. Recording all of them is sound and no less precise. + for (i, key_field) in key.iter().enumerate() { + if let Some((literal, inv_cast)) = predicate.expr_eq_literal(key_field) { + result.boxes[0][i] = Some(BTreeSet::from([literal])); + result.exact = true; + result.inv_cast |= inv_cast; + } + } + result + } + + /// The bound implied by both `self` and `other` holding. + fn and(self, other: Self) -> Self { + debug_assert_eq!(self.arity, other.arity); + let arity = self.arity; + let inv_cast = self.inv_cast || other.inv_cast; + + // Widen before multiplying, so the product stays inside the budget. Only the wider + // operand is widened, because widening both would discard structure that + // `MAX_BOXES` can still afford to keep. + let (left, right, exact) = if self.boxes.len() * other.boxes.len() > MAX_BOXES { + if self.boxes.len() >= other.boxes.len() { + (Self::widen(&self.boxes, arity), other.boxes, false) + } else { + (self.boxes, Self::widen(&other.boxes, arity), false) + } + } else { + (self.boxes, other.boxes, self.exact && other.exact) + }; + + Self { + boxes: Self::product(&left, &right), + exact, + inv_cast, + arity, + } + } + + /// The bound implied by any one of `args` holding. + /// + /// NOTE: Taken n-ary rather than folded pairwise. Folding would normalize the + /// accumulator once per argument, which is quadratic in the width of an `IN` list, and + /// an `IN` list is the case that matters most here. + fn disjunction(args: impl IntoIterator, arity: usize) -> Self { + let mut boxes = Vec::new(); + // A disjunction with no arguments is `false`, which `bottom` already describes. + let mut result = Self::bottom(arity); + for arg in args { + debug_assert_eq!(arg.arity, arity); + result.exact &= arg.exact; + result.inv_cast |= arg.inv_cast; + boxes.extend(arg.boxes); + } + result.boxes = Self::normalize(boxes, arity); + if result.boxes.len() > MAX_BOXES { + result.boxes = Self::widen(&result.boxes, arity); + result.exact = false; + } + result + } + + /// Pairwise intersection of two box lists, dropping boxes that come out unsatisfiable. + fn product(left: &[KeyBox], right: &[KeyBox]) -> Vec { + let mut out = Vec::new(); + for l in left { + for r in right { + if let Some(b) = Self::intersect(l, r) { + out.push(b); + } + } + } + Self::normalize(out, left.first().map_or(0, |b| b.len())) + } + + /// Deduplicates a disjunction of boxes, and merges any two that differ in a single + /// field by unioning that field. + /// + /// The merge is what keeps `a IN ()` to one box instead of `n` of them, which + /// matters for both cost and exactness: `n` boxes would blow the [`MAX_BOXES`] budget + /// and force a widening, when the single merged box is an exact answer. + fn normalize(mut boxes: Vec, arity: usize) -> Vec { + boxes.sort(); + boxes.dedup(); + for i in 0..arity { + // Group by every field but `i`, then union field `i` within each group. + let mut groups: BTreeMap = BTreeMap::new(); + for mut b in boxes { + let field = b[i].take(); + match groups.entry(b) { + Entry::Vacant(e) => { + e.insert(field); + } + Entry::Occupied(mut e) => { + // An unbounded field stays unbounded in the union. + let merged = match (e.get_mut().take(), field) { + (Some(mut l), Some(r)) => { + l.extend(r); + Some(l) + } + _ => None, + }; + *e.get_mut() = merged; + } + } + } + boxes = groups + .into_iter() + .map(|(mut b, field)| { + b[i] = field; + b + }) + .collect(); + } + boxes + } + + /// Intersects two boxes, returning `None` if no key value satisfies both. + fn intersect(left: &KeyBox, right: &KeyBox) -> Option { + left.iter() + .zip_eq(right.iter()) + .map(|(l, r)| match (l, r) { + (None, None) => Some(None), + (None, Some(s)) | (Some(s), None) => Some(Some(s.clone())), + (Some(l), Some(r)) => { + let both: BTreeSet = l.intersection(r).cloned().collect(); + // An empty field bound makes the whole box unsatisfiable. + (!both.is_empty()).then_some(Some(both)) + } + }) + .collect() + } + + /// Collapses a disjunction of boxes into the single box that contains all of them. + /// + /// Sound but lossy: the result admits key values that no input box did, which is why + /// every caller clears `exact`. + fn widen(boxes: &[KeyBox], arity: usize) -> Vec { + if boxes.is_empty() { + // "Never satisfied" needs no widening, and widening it would be wrong. + return Vec::new(); + } + let widened = (0..arity) + .map(|i| { + // A field left unconstrained by any one box is unconstrained in the union. + boxes + .iter() + .map(|b| b[i].as_ref()) + .fold_options(BTreeSet::new(), |mut acc, s| { + acc.extend(s.iter().cloned()); + acc + }) + }) + .collect(); + vec![widened] + } + + /// The key values to look up. + /// + /// An empty result means the predicate is never satisfied. `None` means the value count + /// exceeds [`MAX_LOOKUP_VALUES`], for which there is no useful advice to give: a full + /// scan really is the better plan. + /// + /// Callers must establish that every key field is bounded, via + /// [`KeyBounds::bounds_every_field`], before calling this. + pub fn lookup_values(&self) -> Option> { + assert!(self.bounds_every_field(), "unbounded key field"); + let mut values = BTreeSet::new(); + for b in &self.boxes { + let sets = b + .iter() + .map(|f| f.as_ref().expect("checked by bounds_every_field")) + .collect_vec(); + for combination in sets.into_iter().multi_cartesian_product() { + values.insert(Row::pack(combination.iter().map(|r| r.unpack_first()))); + if values.len() > MAX_LOOKUP_VALUES { + return None; + } + } + } + Some(values.into_iter().collect()) + } + + /// Whether every key field is bounded in every box, which is what makes an index + /// usable at all. + pub fn bounds_every_field(&self) -> bool { + self.arity > 0 && self.boxes.iter().all(|b| b.iter().all(|f| f.is_some())) + } + + /// The key fields that every box bounds. + /// + /// When this is a strict, non-empty subset of the key, an index on just these fields + /// would have been usable, which is what the "index too wide" notice reports. + pub fn bounded_fields(&self) -> Vec { + (0..self.arity) + .filter(|i| self.boxes.iter().all(|b| b[*i].is_some())) + .collect() + } + + /// Whether no key value at all satisfies the predicate, which makes the whole relation + /// empty. + /// + /// Sound in the same way the lookup values are: every rule here only ever widens the + /// set of admissible key values, so an empty result really means empty. + pub fn is_unsatisfiable(&self) -> bool { + self.boxes.is_empty() + } + + /// Whether the bound characterizes the predicate exactly, so that replacing the + /// predicate with the corresponding key lookup preserves meaning. + pub fn exact(&self) -> bool { + self.exact + } +} + +/// Replaces every subexpression of `predicates` that can never be satisfied with `false`. +/// +/// `predicates` is an implicit conjunction, as an MFP's predicate list is. +/// +/// This is what turns `a IN (1, 2) AND a IN (2, 3, 4)` into `a = 2`, and it applies whether +/// or not any index is involved, so `key` should be the full list of literal-pinned +/// expressions from [`literal_constrained_exprs`] rather than an index key. +/// +/// Bottom-up and single-pass, carrying each node's bounds back up so that no subtree is +/// analyzed twice. A pruned child reports itself unsatisfiable, so a parent whose every +/// disjunct died is pruned in the same pass. The leftover `false` arguments are for +/// `MirScalarExpr::reduce` to clean up. +/// Returns whether anything was pruned, and whether the conjunction is unsatisfiable as a +/// whole. The latter covers contradictions that span two predicates, such as `c IN (1, 2)` +/// alongside `c IN (3, 4)`, which no amount of pruning inside either one would reveal. +pub fn prune_unsatisfiable<'a>( + predicates: impl IntoIterator, + key: &[MirScalarExpr], +) -> (bool, bool) { + let mut changed = false; + let bounds = predicates + .into_iter() + .map(|p| prune_inner(p, key, &mut changed)) + .collect_vec(); + let empty = KeyBounds::conjunction_of(bounds, key.len()).is_unsatisfiable(); + (changed, empty) +} + +fn prune_inner( + predicate: &mut MirScalarExpr, + key: &[MirScalarExpr], + changed: &mut bool, +) -> KeyBounds { + mz_ore::stack::maybe_grow(|| { + let bounds = match predicate { + MirScalarExpr::CallVariadic { + func: func @ (VariadicFunc::And(And) | VariadicFunc::Or(Or)), + exprs, + } => { + let is_and = matches!(func, VariadicFunc::And(_)); + let child_bounds = exprs + .iter_mut() + .map(|e| prune_inner(e, key, changed)) + .collect_vec(); + if is_and { + KeyBounds::conjunction_of(child_bounds, key.len()) + } else { + KeyBounds::disjunction(child_bounds, key.len()) + } + } + _ => KeyBounds::leaf(predicate, key), + }; + if bounds.is_unsatisfiable() && !predicate.is_literal_false() { + *predicate = MirScalarExpr::literal_false(); + *changed = true; + } + bounds + }) +} + +/// Every expression that the predicate constrains to literal values somewhere. +/// +/// Index-blind, and used for two things: cheaply rejecting an index whose key mentions an +/// expression the predicate never pins, and recommending a key to a user whose index was +/// too wide. +pub fn literal_constrained_exprs<'a>( + predicates: impl IntoIterator, +) -> Vec { + let mut found = BTreeSet::new(); + for predicate in predicates { + predicate.visit_pre(|e| { + if let Some(expr) = e.any_expr_eq_literal() { + found.insert(expr); + } + }); + } + found.into_iter().collect() +} diff --git a/test/sqllogictest/transform/literal_constraints.slt b/test/sqllogictest/transform/literal_constraints.slt index 3f6b96eb59daa..4c2d832880607 100644 --- a/test/sqllogictest/transform/literal_constraints.slt +++ b/test/sqllogictest/transform/literal_constraints.slt @@ -1490,3 +1490,179 @@ SELECT u FROM t_uint_cast WHERE u::int2 = 5::int2 query error "3000000000" uint2 out of range SELECT u FROM t_uint_cast WHERE u::uint2 = 5::uint2 +# An IN list large enough that a disjunctive-normal-form expansion would be impractical +# still uses the index, and the constraints still come out of the filter. The predicate is +# read once per candidate index, so the conjuncts that say nothing about the index key +# neither cost anything nor stand in the way. +# See https://github.com/MaterializeInc/database-issues/issues/1924 + +statement ok +CREATE TABLE wide (shop_id text, sku_code text, created_at int, rule text, flag bool) + +statement ok +CREATE INDEX wide_idx ON wide(shop_id, sku_code) + +query T multiline +EXPLAIN OPTIMIZED PLAN WITH(humanized expressions, join implementations) AS VERBOSE TEXT FOR +SELECT shop_id FROM wide +WHERE shop_id = 's1' AND sku_code IN ('sku0','sku1','sku2','sku3','sku4','sku5','sku6','sku7','sku8','sku9','sku10','sku11','sku12','sku13','sku14','sku15','sku16','sku17','sku18','sku19','sku20','sku21','sku22','sku23','sku24','sku25','sku26','sku27','sku28','sku29','sku30','sku31','sku32','sku33','sku34','sku35','sku36','sku37','sku38','sku39','sku40','sku41','sku42','sku43','sku44','sku45','sku46','sku47','sku48','sku49','sku50','sku51','sku52','sku53','sku54','sku55','sku56','sku57','sku58','sku59','sku60','sku61','sku62','sku63','sku64','sku65','sku66','sku67','sku68','sku69','sku70','sku71','sku72','sku73','sku74','sku75','sku76','sku77','sku78','sku79','sku80','sku81','sku82','sku83','sku84','sku85','sku86','sku87','sku88','sku89','sku90','sku91','sku92','sku93','sku94','sku95','sku96','sku97','sku98','sku99','sku100','sku101','sku102','sku103','sku104','sku105','sku106','sku107','sku108','sku109','sku110','sku111','sku112','sku113','sku114','sku115','sku116','sku117','sku118','sku119','sku120','sku121','sku122','sku123','sku124','sku125','sku126','sku127','sku128','sku129','sku130','sku131','sku132','sku133','sku134','sku135','sku136','sku137','sku138','sku139','sku140','sku141','sku142','sku143','sku144','sku145','sku146','sku147','sku148','sku149','sku150','sku151','sku152','sku153','sku154','sku155','sku156','sku157','sku158','sku159','sku160','sku161','sku162','sku163','sku164','sku165','sku166','sku167','sku168','sku169','sku170','sku171','sku172','sku173','sku174','sku175','sku176','sku177','sku178','sku179','sku180','sku181','sku182','sku183','sku184','sku185','sku186','sku187','sku188','sku189','sku190','sku191','sku192','sku193','sku194','sku195','sku196','sku197','sku198','sku199','sku200','sku201','sku202','sku203','sku204','sku205','sku206','sku207','sku208','sku209','sku210','sku211','sku212','sku213','sku214','sku215','sku216','sku217','sku218','sku219','sku220','sku221','sku222','sku223','sku224','sku225','sku226','sku227','sku228','sku229','sku230','sku231','sku232','sku233','sku234','sku235','sku236','sku237','sku238','sku239','sku240','sku241','sku242','sku243','sku244','sku245','sku246','sku247','sku248','sku249','sku250','sku251','sku252','sku253','sku254','sku255','sku256','sku257','sku258','sku259','sku260','sku261','sku262','sku263','sku264','sku265','sku266','sku267','sku268','sku269','sku270','sku271','sku272','sku273','sku274','sku275','sku276','sku277','sku278','sku279','sku280','sku281','sku282','sku283','sku284','sku285','sku286','sku287','sku288','sku289','sku290','sku291','sku292','sku293','sku294','sku295','sku296','sku297','sku298','sku299','sku300','sku301','sku302','sku303','sku304','sku305','sku306','sku307','sku308','sku309','sku310','sku311','sku312','sku313','sku314','sku315','sku316','sku317','sku318','sku319','sku320','sku321','sku322','sku323','sku324','sku325','sku326','sku327','sku328','sku329','sku330','sku331','sku332','sku333','sku334','sku335','sku336','sku337','sku338','sku339','sku340','sku341','sku342','sku343','sku344','sku345','sku346','sku347','sku348','sku349','sku350','sku351','sku352','sku353','sku354','sku355','sku356','sku357','sku358','sku359','sku360','sku361','sku362','sku363','sku364','sku365','sku366','sku367','sku368','sku369','sku370','sku371','sku372','sku373','sku374','sku375','sku376','sku377','sku378','sku379','sku380','sku381','sku382','sku383','sku384','sku385','sku386','sku387','sku388','sku389','sku390','sku391','sku392','sku393','sku394','sku395','sku396','sku397','sku398','sku399') + AND created_at < 100 AND (rule = 'median' OR rule = 'same_price') AND flag = false +---- +Explained Query (fast path): + Project (#0{shop_id}) + Filter (#2{created_at} < 100) AND ((#3{rule} = "median") OR (#3{rule} = "same_price")) AND (#4{flag} = false) + ReadIndex on=materialize.public.wide wide_idx=[lookup values=[("s1", "sku0"); ("s1", "sku1"); ("s1", "sku2"); ("s1", "sku3"); ("s1", "sku4"); ("s1", "sku5"); ("s1", "sku6"); ("s1", "sku7"); ("s1", "sku8"); ("s1", "sku9"); ("s1", "sku10"); ("s1", "sku11"); ("s1", "sku12"); ("s1", "sku13"); ("s1", "sku14"); ("s1", "sku15"); ("s1", "sku16"); ("s1", "sku17"); ("s1", "sku18"); ("s1", "sku19"); ("s1", "sku20"); ("s1", "sku21"); ("s1", "sku22"); ("s1", "sku23"); ("s1", "sku24"); ("s1", "sku25"); ("s1", "sku26"); ("s1", "sku27"); ("s1", "sku28"); ("s1", "sku29"); ("s1", "sku30"); ("s1", "sku31"); ("s1", "sku32"); ("s1", "sku33"); ("s1", "sku34"); ("s1", "sku35"); ("s1", "sku36"); ("s1", "sku37"); ("s1", "sku38"); ("s1", "sku39"); ("s1", "sku40"); ("s1", "sku41"); ("s1", "sku42"); ("s1", "sku43"); ("s1", "sku44"); ("s1", "sku45"); ("s1", "sku46"); ("s1", "sku47"); ("s1", "sku48"); ("s1", "sku49"); ("s1", "sku50"); ("s1", "sku51"); ("s1", "sku52"); ("s1", "sku53"); ("s1", "sku54"); ("s1", "sku55"); ("s1", "sku56"); ("s1", "sku57"); ("s1", "sku58"); ("s1", "sku59"); ("s1", "sku60"); ("s1", "sku61"); ("s1", "sku62"); ("s1", "sku63"); ("s1", "sku64"); ("s1", "sku65"); ("s1", "sku66"); ("s1", "sku67"); ("s1", "sku68"); ("s1", "sku69"); ("s1", "sku70"); ("s1", "sku71"); ("s1", "sku72"); ("s1", "sku73"); ("s1", "sku74"); ("s1", "sku75"); ("s1", "sku76"); ("s1", "sku77"); ("s1", "sku78"); ("s1", "sku79"); ("s1", "sku80"); ("s1", "sku81"); ("s1", "sku82"); ("s1", "sku83"); ("s1", "sku84"); ("s1", "sku85"); ("s1", "sku86"); ("s1", "sku87"); ("s1", "sku88"); ("s1", "sku89"); ("s1", "sku90"); ("s1", "sku91"); ("s1", "sku92"); ("s1", "sku93"); ("s1", "sku94"); ("s1", "sku95"); ("s1", "sku96"); ("s1", "sku97"); ("s1", "sku98"); ("s1", "sku99"); ("s1", "sku100"); ("s1", "sku101"); ("s1", "sku102"); ("s1", "sku103"); ("s1", "sku104"); ("s1", "sku105"); ("s1", "sku106"); ("s1", "sku107"); ("s1", "sku108"); ("s1", "sku109"); ("s1", "sku110"); ("s1", "sku111"); ("s1", "sku112"); ("s1", "sku113"); ("s1", "sku114"); ("s1", "sku115"); ("s1", "sku116"); ("s1", "sku117"); ("s1", "sku118"); ("s1", "sku119"); ("s1", "sku120"); ("s1", "sku121"); ("s1", "sku122"); ("s1", "sku123"); ("s1", "sku124"); ("s1", "sku125"); ("s1", "sku126"); ("s1", "sku127"); ("s1", "sku128"); ("s1", "sku129"); ("s1", "sku130"); ("s1", "sku131"); ("s1", "sku132"); ("s1", "sku133"); ("s1", "sku134"); ("s1", "sku135"); ("s1", "sku136"); ("s1", "sku137"); ("s1", "sku138"); ("s1", "sku139"); ("s1", "sku140"); ("s1", "sku141"); ("s1", "sku142"); ("s1", "sku143"); ("s1", "sku144"); ("s1", "sku145"); ("s1", "sku146"); ("s1", "sku147"); ("s1", "sku148"); ("s1", "sku149"); ("s1", "sku150"); ("s1", "sku151"); ("s1", "sku152"); ("s1", "sku153"); ("s1", "sku154"); ("s1", "sku155"); ("s1", "sku156"); ("s1", "sku157"); ("s1", "sku158"); ("s1", "sku159"); ("s1", "sku160"); ("s1", "sku161"); ("s1", "sku162"); ("s1", "sku163"); ("s1", "sku164"); ("s1", "sku165"); ("s1", "sku166"); ("s1", "sku167"); ("s1", "sku168"); ("s1", "sku169"); ("s1", "sku170"); ("s1", "sku171"); ("s1", "sku172"); ("s1", "sku173"); ("s1", "sku174"); ("s1", "sku175"); ("s1", "sku176"); ("s1", "sku177"); ("s1", "sku178"); ("s1", "sku179"); ("s1", "sku180"); ("s1", "sku181"); ("s1", "sku182"); ("s1", "sku183"); ("s1", "sku184"); ("s1", "sku185"); ("s1", "sku186"); ("s1", "sku187"); ("s1", "sku188"); ("s1", "sku189"); ("s1", "sku190"); ("s1", "sku191"); ("s1", "sku192"); ("s1", "sku193"); ("s1", "sku194"); ("s1", "sku195"); ("s1", "sku196"); ("s1", "sku197"); ("s1", "sku198"); ("s1", "sku199"); ("s1", "sku200"); ("s1", "sku201"); ("s1", "sku202"); ("s1", "sku203"); ("s1", "sku204"); ("s1", "sku205"); ("s1", "sku206"); ("s1", "sku207"); ("s1", "sku208"); ("s1", "sku209"); ("s1", "sku210"); ("s1", "sku211"); ("s1", "sku212"); ("s1", "sku213"); ("s1", "sku214"); ("s1", "sku215"); ("s1", "sku216"); ("s1", "sku217"); ("s1", "sku218"); ("s1", "sku219"); ("s1", "sku220"); ("s1", "sku221"); ("s1", "sku222"); ("s1", "sku223"); ("s1", "sku224"); ("s1", "sku225"); ("s1", "sku226"); ("s1", "sku227"); ("s1", "sku228"); ("s1", "sku229"); ("s1", "sku230"); ("s1", "sku231"); ("s1", "sku232"); ("s1", "sku233"); ("s1", "sku234"); ("s1", "sku235"); ("s1", "sku236"); ("s1", "sku237"); ("s1", "sku238"); ("s1", "sku239"); ("s1", "sku240"); ("s1", "sku241"); ("s1", "sku242"); ("s1", "sku243"); ("s1", "sku244"); ("s1", "sku245"); ("s1", "sku246"); ("s1", "sku247"); ("s1", "sku248"); ("s1", "sku249"); ("s1", "sku250"); ("s1", "sku251"); ("s1", "sku252"); ("s1", "sku253"); ("s1", "sku254"); ("s1", "sku255"); ("s1", "sku256"); ("s1", "sku257"); ("s1", "sku258"); ("s1", "sku259"); ("s1", "sku260"); ("s1", "sku261"); ("s1", "sku262"); ("s1", "sku263"); ("s1", "sku264"); ("s1", "sku265"); ("s1", "sku266"); ("s1", "sku267"); ("s1", "sku268"); ("s1", "sku269"); ("s1", "sku270"); ("s1", "sku271"); ("s1", "sku272"); ("s1", "sku273"); ("s1", "sku274"); ("s1", "sku275"); ("s1", "sku276"); ("s1", "sku277"); ("s1", "sku278"); ("s1", "sku279"); ("s1", "sku280"); ("s1", "sku281"); ("s1", "sku282"); ("s1", "sku283"); ("s1", "sku284"); ("s1", "sku285"); ("s1", "sku286"); ("s1", "sku287"); ("s1", "sku288"); ("s1", "sku289"); ("s1", "sku290"); ("s1", "sku291"); ("s1", "sku292"); ("s1", "sku293"); ("s1", "sku294"); ("s1", "sku295"); ("s1", "sku296"); ("s1", "sku297"); ("s1", "sku298"); ("s1", "sku299"); ("s1", "sku300"); ("s1", "sku301"); ("s1", "sku302"); ("s1", "sku303"); ("s1", "sku304"); ("s1", "sku305"); ("s1", "sku306"); ("s1", "sku307"); ("s1", "sku308"); ("s1", "sku309"); ("s1", "sku310"); ("s1", "sku311"); ("s1", "sku312"); ("s1", "sku313"); ("s1", "sku314"); ("s1", "sku315"); ("s1", "sku316"); ("s1", "sku317"); ("s1", "sku318"); ("s1", "sku319"); ("s1", "sku320"); ("s1", "sku321"); ("s1", "sku322"); ("s1", "sku323"); ("s1", "sku324"); ("s1", "sku325"); ("s1", "sku326"); ("s1", "sku327"); ("s1", "sku328"); ("s1", "sku329"); ("s1", "sku330"); ("s1", "sku331"); ("s1", "sku332"); ("s1", "sku333"); ("s1", "sku334"); ("s1", "sku335"); ("s1", "sku336"); ("s1", "sku337"); ("s1", "sku338"); ("s1", "sku339"); ("s1", "sku340"); ("s1", "sku341"); ("s1", "sku342"); ("s1", "sku343"); ("s1", "sku344"); ("s1", "sku345"); ("s1", "sku346"); ("s1", "sku347"); ("s1", "sku348"); ("s1", "sku349"); ("s1", "sku350"); ("s1", "sku351"); ("s1", "sku352"); ("s1", "sku353"); ("s1", "sku354"); ("s1", "sku355"); ("s1", "sku356"); ("s1", "sku357"); ("s1", "sku358"); ("s1", "sku359"); ("s1", "sku360"); ("s1", "sku361"); ("s1", "sku362"); ("s1", "sku363"); ("s1", "sku364"); ("s1", "sku365"); ("s1", "sku366"); ("s1", "sku367"); ("s1", "sku368"); ("s1", "sku369"); ("s1", "sku370"); ("s1", "sku371"); ("s1", "sku372"); ("s1", "sku373"); ("s1", "sku374"); ("s1", "sku375"); ("s1", "sku376"); ("s1", "sku377"); ("s1", "sku378"); ("s1", "sku379"); ("s1", "sku380"); ("s1", "sku381"); ("s1", "sku382"); ("s1", "sku383"); ("s1", "sku384"); ("s1", "sku385"); ("s1", "sku386"); ("s1", "sku387"); ("s1", "sku388"); ("s1", "sku389"); ("s1", "sku390"); ("s1", "sku391"); ("s1", "sku392"); ("s1", "sku393"); ("s1", "sku394"); ("s1", "sku395"); ("s1", "sku396"); ("s1", "sku397"); ("s1", "sku398"); ("s1", "sku399")]] + +Used Indexes: + - materialize.public.wide_idx (lookup) + +Target cluster: quickstart + +EOF + +# Unrelated disjunctions multiply a disjunctive normal form but say nothing about the key, +# so they are simply carried through to the residual filter. + +query T multiline +EXPLAIN OPTIMIZED PLAN WITH(humanized expressions, join implementations) AS VERBOSE TEXT FOR +SELECT shop_id FROM wide +WHERE shop_id = 's1' AND sku_code IN ('sku3','sku4') + AND (created_at > 0 OR rule = 'r0') AND (created_at > 1 OR rule = 'r1') + AND (created_at > 2 OR rule = 'r2') AND (created_at > 3 OR rule = 'r3') + AND (created_at > 4 OR rule = 'r4') AND (created_at > 5 OR rule = 'r5') + AND (created_at > 6 OR rule = 'r6') AND (created_at > 7 OR rule = 'r7') + AND (created_at > 8 OR rule = 'r8') AND (created_at > 9 OR rule = 'r9') +---- +Explained Query (fast path): + Project (#0{shop_id}) + Filter ((#3{rule} = "r0") OR (#2{created_at} > 0)) AND ((#3{rule} = "r1") OR (#2{created_at} > 1)) AND ((#3{rule} = "r2") OR (#2{created_at} > 2)) AND ((#3{rule} = "r3") OR (#2{created_at} > 3)) AND ((#3{rule} = "r4") OR (#2{created_at} > 4)) AND ((#3{rule} = "r5") OR (#2{created_at} > 5)) AND ((#3{rule} = "r6") OR (#2{created_at} > 6)) AND ((#3{rule} = "r7") OR (#2{created_at} > 7)) AND ((#3{rule} = "r8") OR (#2{created_at} > 8)) AND ((#3{rule} = "r9") OR (#2{created_at} > 9)) + ReadIndex on=materialize.public.wide wide_idx=[lookup values=[("s1", "sku3"); ("s1", "sku4")]] + +Used Indexes: + - materialize.public.wide_idx (lookup) + +Target cluster: quickstart + +EOF + +# An expression pinned to two different values makes the relation empty, whether or not the +# expression is part of an index key. + +query T multiline +EXPLAIN OPTIMIZED PLAN WITH(humanized expressions, join implementations) AS VERBOSE TEXT FOR +SELECT shop_id FROM wide +WHERE shop_id = 's1' AND sku_code = 'sku3' + AND (created_at = 1 OR created_at = 2) AND (created_at = 3 OR created_at = 4) +---- +Explained Query (fast path): + Constant + +Target cluster: quickstart + +EOF + +# An IN list on a column the index does not cover must not obstruct the index. Converting to +# disjunctive normal form would multiply the two lists together, so these are the cases that +# used to fall off a cliff. The lookup values come only from the covered column, and the +# uncovered list is carried through to the residual filter. + +statement ok +CREATE TABLE cover (foo int, bar int, qux int) + +statement ok +CREATE INDEX cover_foo ON cover(foo) + +statement ok +INSERT INTO cover VALUES (1, 1, 1), (2, 2, 2), (3, 3, 3) + +# Two IN lists, one covered. A DNF would have 144 disjuncts for 12 lookup values. + +query T multiline +EXPLAIN OPTIMIZED PLAN WITH(humanized expressions, join implementations) AS VERBOSE TEXT FOR +SELECT foo FROM cover WHERE foo IN (0,1,2,3,4,5,6,7,8,9,10,11) AND bar IN (0,1,2,3,4,5,6,7,8,9,10,11) +---- +Explained Query (fast path): + Project (#0{foo}) + Filter ((#1{bar} = 0) OR (#1{bar} = 1) OR (#1{bar} = 2) OR (#1{bar} = 3) OR (#1{bar} = 4) OR (#1{bar} = 5) OR (#1{bar} = 6) OR (#1{bar} = 7) OR (#1{bar} = 8) OR (#1{bar} = 9) OR (#1{bar} = 10) OR (#1{bar} = 11)) + ReadIndex on=materialize.public.cover cover_foo=[lookup values=[(0); (1); (2); (3); (4); (5); (6); (7); (8); (9); (10); (11)]] + +Used Indexes: + - materialize.public.cover_foo (lookup) + +Target cluster: quickstart + +EOF + +# Three IN lists, one covered. A DNF would have 12^3 = 1728 disjuncts for the same 12 values. + +query T multiline +EXPLAIN OPTIMIZED PLAN WITH(humanized expressions, join implementations) AS VERBOSE TEXT FOR +SELECT foo FROM cover WHERE foo IN (0,1,2,3,4,5,6,7,8,9,10,11) AND bar IN (0,1,2,3,4,5,6,7,8,9,10,11) AND qux IN (0,1,2,3,4,5,6,7,8,9,10,11) +---- +Explained Query (fast path): + Project (#0{foo}) + Filter ((#1{bar} = 0) OR (#1{bar} = 1) OR (#1{bar} = 2) OR (#1{bar} = 3) OR (#1{bar} = 4) OR (#1{bar} = 5) OR (#1{bar} = 6) OR (#1{bar} = 7) OR (#1{bar} = 8) OR (#1{bar} = 9) OR (#1{bar} = 10) OR (#1{bar} = 11)) AND ((#2{qux} = 0) OR (#2{qux} = 1) OR (#2{qux} = 2) OR (#2{qux} = 3) OR (#2{qux} = 4) OR (#2{qux} = 5) OR (#2{qux} = 6) OR (#2{qux} = 7) OR (#2{qux} = 8) OR (#2{qux} = 9) OR (#2{qux} = 10) OR (#2{qux} = 11)) + ReadIndex on=materialize.public.cover cover_foo=[lookup values=[(0); (1); (2); (3); (4); (5); (6); (7); (8); (9); (10); (11)]] + +Used Indexes: + - materialize.public.cover_foo (lookup) + +Target cluster: quickstart + +EOF + +# A covered list long enough that the whole predicate is past any workable size guard on a +# DNF, with an uncovered list alongside it. The long list is on the covered column here so +# that the plan prints one line of lookup values rather than a wall of residual predicates. + +query T multiline +EXPLAIN OPTIMIZED PLAN WITH(humanized expressions, join implementations) AS VERBOSE TEXT FOR +SELECT foo FROM cover WHERE foo IN (0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26,27,28,29,30,31,32,33,34,35,36,37,38,39,40,41,42,43,44,45,46,47,48,49,50,51,52,53,54,55,56,57,58,59,60,61,62,63,64,65,66,67,68,69,70,71,72,73,74,75,76,77,78,79,80,81,82,83,84,85,86,87,88,89,90,91,92,93,94,95,96,97,98,99,100,101,102,103,104,105,106,107,108,109,110,111,112,113,114,115,116,117,118,119,120,121,122,123,124,125,126,127,128,129,130,131,132,133,134,135,136,137,138,139,140,141,142,143,144,145,146,147,148,149,150,151,152,153,154,155,156,157,158,159,160,161,162,163,164,165,166,167,168,169,170,171,172,173,174,175,176,177,178,179,180,181,182,183,184,185,186,187,188,189,190,191,192,193,194,195,196,197,198,199,200,201,202,203,204,205,206,207,208,209,210,211,212,213,214,215,216,217,218,219,220,221,222,223,224,225,226,227,228,229,230,231,232,233,234,235,236,237,238,239,240,241,242,243,244,245,246,247,248,249,250,251,252,253,254,255,256,257,258,259,260,261,262,263,264,265,266,267,268,269,270,271,272,273,274,275,276,277,278,279,280,281,282,283,284,285,286,287,288,289,290,291,292,293,294,295,296,297,298,299,300,301,302,303,304,305,306,307,308,309,310,311,312,313,314,315,316,317,318,319,320,321,322,323,324,325,326,327,328,329,330,331,332,333,334,335,336,337,338,339) AND bar IN (1,2) +---- +Explained Query (fast path): + Project (#0{foo}) + Filter ((#1{bar} = 1) OR (#1{bar} = 2)) + ReadIndex on=materialize.public.cover cover_foo=[lookup values=[(0); (1); (2); (3); (4); (5); (6); (7); (8); (9); (10); (11); (12); (13); (14); (15); (16); (17); (18); (19); (20); (21); (22); (23); (24); (25); (26); (27); (28); (29); (30); (31); (32); (33); (34); (35); (36); (37); (38); (39); (40); (41); (42); (43); (44); (45); (46); (47); (48); (49); (50); (51); (52); (53); (54); (55); (56); (57); (58); (59); (60); (61); (62); (63); (64); (65); (66); (67); (68); (69); (70); (71); (72); (73); (74); (75); (76); (77); (78); (79); (80); (81); (82); (83); (84); (85); (86); (87); (88); (89); (90); (91); (92); (93); (94); (95); (96); (97); (98); (99); (100); (101); (102); (103); (104); (105); (106); (107); (108); (109); (110); (111); (112); (113); (114); (115); (116); (117); (118); (119); (120); (121); (122); (123); (124); (125); (126); (127); (128); (129); (130); (131); (132); (133); (134); (135); (136); (137); (138); (139); (140); (141); (142); (143); (144); (145); (146); (147); (148); (149); (150); (151); (152); (153); (154); (155); (156); (157); (158); (159); (160); (161); (162); (163); (164); (165); (166); (167); (168); (169); (170); (171); (172); (173); (174); (175); (176); (177); (178); (179); (180); (181); (182); (183); (184); (185); (186); (187); (188); (189); (190); (191); (192); (193); (194); (195); (196); (197); (198); (199); (200); (201); (202); (203); (204); (205); (206); (207); (208); (209); (210); (211); (212); (213); (214); (215); (216); (217); (218); (219); (220); (221); (222); (223); (224); (225); (226); (227); (228); (229); (230); (231); (232); (233); (234); (235); (236); (237); (238); (239); (240); (241); (242); (243); (244); (245); (246); (247); (248); (249); (250); (251); (252); (253); (254); (255); (256); (257); (258); (259); (260); (261); (262); (263); (264); (265); (266); (267); (268); (269); (270); (271); (272); (273); (274); (275); (276); (277); (278); (279); (280); (281); (282); (283); (284); (285); (286); (287); (288); (289); (290); (291); (292); (293); (294); (295); (296); (297); (298); (299); (300); (301); (302); (303); (304); (305); (306); (307); (308); (309); (310); (311); (312); (313); (314); (315); (316); (317); (318); (319); (320); (321); (322); (323); (324); (325); (326); (327); (328); (329); (330); (331); (332); (333); (334); (335); (336); (337); (338); (339)]] + +Used Indexes: + - materialize.public.cover_foo (lookup) + +Target cluster: quickstart + +EOF + +query III rowsort +SELECT * FROM cover WHERE foo IN (0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26,27,28,29,30,31,32,33,34,35,36,37,38,39,40,41,42,43,44,45,46,47,48,49,50,51,52,53,54,55,56,57,58,59,60,61,62,63,64,65,66,67,68,69,70,71,72,73,74,75,76,77,78,79,80,81,82,83,84,85,86,87,88,89,90,91,92,93,94,95,96,97,98,99,100,101,102,103,104,105,106,107,108,109,110,111,112,113,114,115,116,117,118,119,120,121,122,123,124,125,126,127,128,129,130,131,132,133,134,135,136,137,138,139,140,141,142,143,144,145,146,147,148,149,150,151,152,153,154,155,156,157,158,159,160,161,162,163,164,165,166,167,168,169,170,171,172,173,174,175,176,177,178,179,180,181,182,183,184,185,186,187,188,189,190,191,192,193,194,195,196,197,198,199,200,201,202,203,204,205,206,207,208,209,210,211,212,213,214,215,216,217,218,219,220,221,222,223,224,225,226,227,228,229,230,231,232,233,234,235,236,237,238,239,240,241,242,243,244,245,246,247,248,249,250,251,252,253,254,255,256,257,258,259,260,261,262,263,264,265,266,267,268,269,270,271,272,273,274,275,276,277,278,279,280,281,282,283,284,285,286,287,288,289,290,291,292,293,294,295,296,297,298,299,300,301,302,303,304,305,306,307,308,309,310,311,312,313,314,315,316,317,318,319,320,321,322,323,324,325,326,327,328,329,330,331,332,333,334,335,336,337,338,339) AND bar IN (1,2) +---- +1 1 1 +2 2 2 + +# With both `foo` and `qux` covered, the cross product of their two lists is the real answer, +# so the nine lookup values here are inherent rather than incidental. `bar` still contributes +# nothing but a residual predicate. + +statement ok +CREATE INDEX cover_foo_qux ON cover(foo, qux) + +query T multiline +EXPLAIN OPTIMIZED PLAN WITH(humanized expressions, join implementations) AS VERBOSE TEXT FOR +SELECT foo FROM cover WHERE foo IN (1,2,3) AND bar IN (0,1,2,3,4,5,6,7,8,9,10,11) AND qux IN (1,2,3) +---- +Explained Query (fast path): + Project (#0{foo}) + Filter ((#2{bar} = 0) OR (#2{bar} = 1) OR (#2{bar} = 2) OR (#2{bar} = 3) OR (#2{bar} = 4) OR (#2{bar} = 5) OR (#2{bar} = 6) OR (#2{bar} = 7) OR (#2{bar} = 8) OR (#2{bar} = 9) OR (#2{bar} = 10) OR (#2{bar} = 11)) + ReadIndex on=materialize.public.cover cover_foo_qux=[lookup values=[(1, 1); (1, 2); (1, 3); (2, 1); (2, 2); (2, 3); (3, 1); (3, 2); (3, 3)]] + +Used Indexes: + - materialize.public.cover_foo_qux (lookup) + +Target cluster: quickstart + +EOF + +query III rowsort +SELECT * FROM cover WHERE foo IN (1,2,3) AND bar IN (0,1,2,3,4,5,6,7,8,9,10,11) AND qux IN (1,2,3) +---- +1 1 1 +2 2 2 +3 3 3 From 729d3d0e05493a39c4d7ebeae9186a6c85809886 Mon Sep 17 00:00:00 2001 From: Frank McSherry Date: Thu, 27 Aug 2026 11:14:34 -0400 Subject: [PATCH 2/4] transform: update expectations for index-directed literal constraints Two plans improve. `WHERE a = NULL OR a = 2` reduces to a `null` literal disjoined with a literal equality. A filter drops a `null` row just as it drops a `false` one, so the predicate is equivalent to `a = 2` and the equality can be served by a lookup rather than a full scan with a residual filter. The paired data query is unchanged, so the `null` row in the table is still not returned, which is what the case was written to check. The join equivalence `{#0, #1, #2}` over a single input lowers to the null-safe pairwise equalities with one mapped expression, rather than the shape the old normal-form round trip left behind. There is no index on that source at all, so the previous expectation recorded only the distortion: the expansion ran, found nothing to use, and the node-count heuristic then preferred the tangled result over the predicate it started from. --- .../tests/test_transforms/join_implementation.spec | 8 ++++---- test/sqllogictest/transform/literal_constraints.slt | 11 +++++++---- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/src/transform/tests/test_transforms/join_implementation.spec b/src/transform/tests/test_transforms/join_implementation.spec index d1d14f092f832..99ca1b7d78183 100644 --- a/src/transform/tests/test_transforms/join_implementation.spec +++ b/src/transform/tests/test_transforms/join_implementation.spec @@ -40,8 +40,8 @@ Join on=(#0 = #1 = #2) Get x ---- Project (#0..=#2) - Filter ((#4 AND #5) OR ((#0) IS NULL AND ((#3 AND #5) OR ((#1) IS NULL AND (#3 OR #4))))) - Map ((#2) IS NULL, (#0 = #2), (#0 = #1)) + Filter ((#0 = #1) OR (#3 AND (#1) IS NULL)) AND ((#0 = #2) OR (#3 AND (#2) IS NULL)) + Map ((#0) IS NULL) Get x apply pipeline=optimize @@ -49,8 +49,8 @@ Join on=(#0 = #2 = #1 = #2) Get x ---- Project (#0..=#2) - Filter ((#4 AND #5) OR ((#0) IS NULL AND ((#3 AND #5) OR ((#1) IS NULL AND (#3 OR #4))))) - Map ((#2) IS NULL, (#0 = #2), (#0 = #1)) + Filter ((#0 = #1) OR (#3 AND (#1) IS NULL)) AND ((#0 = #2) OR (#3 AND (#2) IS NULL)) + Map ((#0) IS NULL) Get x apply pipeline=optimize diff --git a/test/sqllogictest/transform/literal_constraints.slt b/test/sqllogictest/transform/literal_constraints.slt index 4c2d832880607..3ae7c30425255 100644 --- a/test/sqllogictest/transform/literal_constraints.slt +++ b/test/sqllogictest/transform/literal_constraints.slt @@ -211,18 +211,20 @@ Target cluster: quickstart EOF -# `a = NULL` should NOT find the NULL in the table. +# `a = NULL` should NOT find the NULL in the table. Note that `a = NULL` reduces to a `null` +# literal, and a filter drops a `null` row just as it drops a `false` one, so the predicate +# is equivalent to just `a = 2` and the lookup is exact. query T multiline EXPLAIN OPTIMIZED PLAN WITH(humanized expressions, arity, join implementations) AS VERBOSE TEXT FOR SELECT * FROM t1 WHERE a = NULL OR a = 2 ---- Explained Query (fast path): - Filter (null OR (#0{a} = 2)) - ReadIndex on=materialize.public.t1 idx_t1_a_b=[*** full scan ***] + Project (#0{a}, #1{b}) + ReadIndex on=materialize.public.t1 idx_t1_a=[lookup value=(2)] Used Indexes: - - materialize.public.idx_t1_a_b (*** full scan ***) + - materialize.public.idx_t1_a (lookup) Target cluster: quickstart @@ -1490,6 +1492,7 @@ SELECT u FROM t_uint_cast WHERE u::int2 = 5::int2 query error "3000000000" uint2 out of range SELECT u FROM t_uint_cast WHERE u::uint2 = 5::uint2 + # An IN list large enough that a disjunctive-normal-form expansion would be impractical # still uses the index, and the constraints still come out of the filter. The predicate is # read once per candidate index, so the conjuncts that say nothing about the index key From 1a83451f92d17edd8141d2bea970ed38d7a0bce8 Mon Sep 17 00:00:00 2001 From: Frank McSherry Date: Fri, 28 Aug 2026 15:05:16 -0400 Subject: [PATCH 3/4] transform: fix doc lint fallout in literal constraints `bin/doc` runs rustdoc with `-D warnings`, which rejects an intra-doc link from a public module to a private one even under `--document-private-items`, since the link would break without that flag. Refer to the `key_bounds` submodule in prose instead. Also drop a reference to a closed issue from a comment on a new test. `bin/ci-closed-issues-detect --changed-lines-only` flags those, and the case the reference belonged to already cites it a few lines up. --- src/transform/src/literal_constraints.rs | 4 ++-- test/sqllogictest/transform/literal_constraints.slt | 1 - 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/transform/src/literal_constraints.rs b/src/transform/src/literal_constraints.rs index e571a650aa100..67aad1fa8fd35 100644 --- a/src/transform/src/literal_constraints.rs +++ b/src/transform/src/literal_constraints.rs @@ -13,8 +13,8 @@ //! a constant collection. //! //! The detection is index-directed: for each candidate index we ask what the predicate says -//! about that index's key expressions, and read the answer off in a single pass. See -//! [`key_bounds`]. +//! about that index's key expressions, and read the answer off in a single pass. The +//! `key_bounds` submodule holds that analysis. //! //! E.g.: Logically, we go from something like //! `SELECT f1, f2, f3 FROM t WHERE t.f1 = lit1 AND t.f2 = lit2` diff --git a/test/sqllogictest/transform/literal_constraints.slt b/test/sqllogictest/transform/literal_constraints.slt index 3ae7c30425255..889ffbbcf8dcd 100644 --- a/test/sqllogictest/transform/literal_constraints.slt +++ b/test/sqllogictest/transform/literal_constraints.slt @@ -1497,7 +1497,6 @@ SELECT u FROM t_uint_cast WHERE u::uint2 = 5::uint2 # still uses the index, and the constraints still come out of the filter. The predicate is # read once per candidate index, so the conjuncts that say nothing about the index key # neither cost anything nor stand in the way. -# See https://github.com/MaterializeInc/database-issues/issues/1924 statement ok CREATE TABLE wide (shop_id text, sku_code text, created_at int, rule text, flag bool) From 6e72aa56c32516751c9036d38a43ae6256b91bca Mon Sep 17 00:00:00 2001 From: Frank McSherry Date: Fri, 28 Aug 2026 15:34:20 -0400 Subject: [PATCH 4/4] transform: refuse constraint removal when the key bound widened Removing a predicate that exactly bounds the index key is sound only because the lookup values are the *intersection* of what every predicate implies, so they fall inside the removed predicate's own bounds. `KeyBounds::and` breaks that when a box product exceeds its budget: it widens one operand to the box containing it, which yields a superset of the intersection. The operands keep their own `exact` flags, and removal consulted only those, so both predicates of a two-list conjunction could be dropped against lookup values that neither of them admits. With an index on `(a, b)` and two disjoint 40-element pair lists, the plan looked up 39 keys with no residual filter and returned rows for a predicate whose answer is empty. Track widening as its own bit rather than folding it into `exact`. `exact` has to stay per-predicate for removal to work at all, since a conjunction containing one opaque predicate is not exact yet its other conjuncts are still removable. Widening is a property of the conjunction, so removal now refuses outright when the bound it is working against widened anywhere. Keeping the filter is always correct, as the lookup then merely over-approximates. The other two consumers were never affected: lookup values and `is_unsatisfiable` only ever over-approximate, which widening preserves. Adds a sqllogictest asserting rows rather than a plan, since over-approximating the lookup is allowed and returning a row is not, and unit tests on `KeyBounds` for the widened and non-widened conjunctions. Also skips the no-op passes in `normalize`. Merging on a field that every box agrees on cannot combine anything, because two boxes sharing a group would then agree everywhere and have been deduplicated already. Wide keys are mostly pinned to single values, so this is nearly all of them: for a 1000-value tuple list conjoined with 48 equalities, `LiteralConstraints` drops from 245ms to 49ms, against 23ms before this branch, where no index was selected at all. Gates the "index too wide" recommendation on some index actually being too wide. It runs a bounds pass over the widest key in play, and a Get with no indexes should not pay for it. Replaces an assertion in `lookup_values` with a `None`, since a panic is a poor failure mode for an invariant that spans calls. --- src/transform/src/literal_constraints.rs | 19 +- .../src/literal_constraints/key_bounds.rs | 169 +++++++++++++++--- .../transform/literal_constraints.slt | 25 +++ 3 files changed, 188 insertions(+), 25 deletions(-) diff --git a/src/transform/src/literal_constraints.rs b/src/transform/src/literal_constraints.rs index 67aad1fa8fd35..d8c223012f20d 100644 --- a/src/transform/src/literal_constraints.rs +++ b/src/transform/src/literal_constraints.rs @@ -246,9 +246,14 @@ impl LiteralConstraints { .max_by_key(|(_idx_id, key, _vals, inv_cast)| (key.len(), *inv_cast)) .map(|(idx_id, key, vals, _inv_cast)| (idx_id, key, vals)); - if result.is_none() { - // Let's see if we can give a hint to the user. - // + // A hint is only worth computing when some index is close enough to receive one. + // `constrained` is the widest key in play, so the pass below is the most expensive + // one we do, and a Get with no indexes at all must not pay for it. + let advisable = index_matches + .iter() + .any(|(_, _, m)| matches!(m, IndexMatch::UnusableTooWide(_))); + + if result.is_none() && advisable { // The recommendation is index-blind: gather every expression the predicate // pins to literal values anywhere, then keep those it pins in all cases. An // index on exactly those would have been usable. @@ -332,8 +337,14 @@ impl LiteralConstraints { /// NOTE: This is sound only because the lookup values are the intersection of what /// *every* predicate implies, including the ones we keep. So the retained predicates /// can only narrow the key further, never widen it past what a removed predicate - /// allowed. + /// allowed. A widened bound is a superset of that intersection rather than the + /// intersection itself, which breaks exactly that step, so removal refuses outright when + /// the conjunction widened anywhere. Keeping the filter is always correct, since the + /// lookup then merely over-approximates. fn remove_literal_constraints(mfp: &mut MapFilterProject, key: &[MirScalarExpr]) -> bool { + if Self::key_bounds(mfp, key).widened() { + return false; + } let (map, predicates, project) = mfp.as_map_filter_project(); let kept = predicates .into_iter() diff --git a/src/transform/src/literal_constraints/key_bounds.rs b/src/transform/src/literal_constraints/key_bounds.rs index 0285db6f3774a..ac3730e986a25 100644 --- a/src/transform/src/literal_constraints/key_bounds.rs +++ b/src/transform/src/literal_constraints/key_bounds.rs @@ -29,8 +29,16 @@ //! NOTE: A `KeyBounds` is only ever a *sound* bound. Over-approximating is always safe for //! choosing lookup values, because the residual filter still runs and the constant //! collection the lookups become has distinct rows, so the semi-join cannot duplicate. -//! Removing a constraint from the filter is a different claim, and requires -//! [`KeyBounds::exact`]. +//! Removing a constraint from the filter is a different claim, and requires both +//! [`KeyBounds::exact`] and the absence of [`KeyBounds::widened`]. +//! +//! NOTE: Literal values are compared as `Row`s, and `RowRef`'s `Ord` orders by the packed +//! byte representation rather than by `Datum::cmp`. Two literals that compare equal in SQL +//! can therefore land in different set elements: `munge_numeric` normalizes `-0` but not +//! scale, so `n = 1.0 AND n = 1.00` intersects to the empty set even though `1.0 = 1.00`. +//! The verdict is the same one the surrounding transform has always reached, but the set +//! arithmetic here leans on it much harder, so treat byte identity as the definition of +//! literal equality for these purposes. use std::collections::btree_map::Entry; use std::collections::{BTreeMap, BTreeSet}; @@ -67,12 +75,24 @@ pub struct KeyBounds { /// An empty list therefore means the predicate is never satisfied. boxes: Vec, /// Whether `boxes` characterizes the predicate exactly, so that the predicate is - /// equivalent to "the key falls in one of these boxes". + /// equivalent, *as a filter*, to "the key falls in one of these boxes". + /// + /// "As a filter" is the operative part, and is what lets a literal `null` count as + /// unsatisfiable: a filter drops a `null` row exactly as it drops a `false` one. See + /// [`KeyBounds::leaf`]. /// /// False either because the predicate constrains something besides the key fields, or /// because we widened to stay inside [`MAX_BOXES`]. Only an exact bound may be removed /// from the filter. exact: bool, + /// Whether any step of this bound's derivation widened, that is, replaced a disjunction + /// of boxes by a single box containing them. + /// + /// Widening keeps the bound sound, because it only ever admits more key values, so the + /// lookup values and [`KeyBounds::is_unsatisfiable`] stay correct. It breaks a stronger + /// property that only constraint removal needs. See + /// [`KeyBounds::widened`]. + widened: bool, /// Whether matching a key field required inverting a cast on it. Reported so that the /// caller can prefer an index whose key needs no inversion. pub inv_cast: bool, @@ -85,6 +105,7 @@ impl KeyBounds { /// predicate is not equivalent to that, so it must stay in the filter. fn top(arity: usize) -> Self { KeyBounds { + widened: false, boxes: vec![vec![None; arity]], exact: false, inv_cast: false, @@ -99,6 +120,7 @@ impl KeyBounds { /// removed from the filter. `unit` stands for "there is nothing here to read". fn unit(arity: usize) -> Self { KeyBounds { + widened: false, boxes: vec![vec![None; arity]], exact: true, inv_cast: false, @@ -109,6 +131,7 @@ impl KeyBounds { /// The bound of a predicate that is never satisfied. fn bottom(arity: usize) -> Self { KeyBounds { + widened: false, boxes: Vec::new(), exact: true, inv_cast: false, @@ -185,19 +208,25 @@ impl KeyBounds { // Widen before multiplying, so the product stays inside the budget. Only the wider // operand is widened, because widening both would discard structure that // `MAX_BOXES` can still afford to keep. - let (left, right, exact) = if self.boxes.len() * other.boxes.len() > MAX_BOXES { + let (left, right, exact, widened) = if self.boxes.len() * other.boxes.len() > MAX_BOXES { if self.boxes.len() >= other.boxes.len() { - (Self::widen(&self.boxes, arity), other.boxes, false) + (Self::widen(&self.boxes, arity), other.boxes, false, true) } else { - (self.boxes, Self::widen(&other.boxes, arity), false) + (self.boxes, Self::widen(&other.boxes, arity), false, true) } } else { - (self.boxes, other.boxes, self.exact && other.exact) + ( + self.boxes, + other.boxes, + self.exact && other.exact, + self.widened || other.widened, + ) }; Self { boxes: Self::product(&left, &right), exact, + widened: widened || self.widened || other.widened, inv_cast, arity, } @@ -215,6 +244,7 @@ impl KeyBounds { for arg in args { debug_assert_eq!(arg.arity, arity); result.exact &= arg.exact; + result.widened |= arg.widened; result.inv_cast |= arg.inv_cast; boxes.extend(arg.boxes); } @@ -222,6 +252,7 @@ impl KeyBounds { if result.boxes.len() > MAX_BOXES { result.boxes = Self::widen(&result.boxes, arity); result.exact = false; + result.widened = true; } result } @@ -248,7 +279,18 @@ impl KeyBounds { fn normalize(mut boxes: Vec, arity: usize) -> Vec { boxes.sort(); boxes.dedup(); + if boxes.len() < 2 { + return boxes; + } for i in 0..arity { + // Merging on a field that every box agrees on is a no-op: two boxes sharing a + // group would then agree on every field and have been deduplicated already. + // Skipping those keeps the cost proportional to the fields that actually vary, + // which is what makes a wide key affordable when most of it is pinned to single + // values. + if boxes.iter().all(|b| b[i] == boxes[0][i]) { + continue; + } // Group by every field but `i`, then union field `i` within each group. let mut groups: BTreeMap = BTreeMap::new(); for mut b in boxes { @@ -323,20 +365,17 @@ impl KeyBounds { /// The key values to look up. /// - /// An empty result means the predicate is never satisfied. `None` means the value count - /// exceeds [`MAX_LOOKUP_VALUES`], for which there is no useful advice to give: a full - /// scan really is the better plan. - /// - /// Callers must establish that every key field is bounded, via - /// [`KeyBounds::bounds_every_field`], before calling this. + /// An empty result means the predicate is never satisfied. `None` means there is nothing + /// worth looking up: either a key field is unbounded, or the value count exceeds + /// [`MAX_LOOKUP_VALUES`] and a full scan is the better plan. Callers that need to tell + /// those apart should consult [`KeyBounds::bounds_every_field`] first. pub fn lookup_values(&self) -> Option> { - assert!(self.bounds_every_field(), "unbounded key field"); + if !self.bounds_every_field() { + return None; + } let mut values = BTreeSet::new(); for b in &self.boxes { - let sets = b - .iter() - .map(|f| f.as_ref().expect("checked by bounds_every_field")) - .collect_vec(); + let sets = b.iter().map(|f| f.as_ref()).collect::>>()?; for combination in sets.into_iter().multi_cartesian_product() { values.insert(Row::pack(combination.iter().map(|r| r.unpack_first()))); if values.len() > MAX_LOOKUP_VALUES { @@ -363,6 +402,19 @@ impl KeyBounds { .collect() } + /// Whether the derivation widened anywhere, which makes the bound a strict + /// over-approximation of what the predicate implies. + /// + /// Removal reasons about *containment*, not just exactness: dropping a predicate that is + /// exactly a bound on the key is sound only because the lookup values are the + /// intersection of what every predicate implies, so they fall inside the dropped + /// predicate's own bounds. Widening produces a superset of that intersection instead, so + /// a lookup value need no longer satisfy a dropped predicate, and the rows it finds would + /// go unfiltered. Callers that remove must refuse when this is set. + pub fn widened(&self) -> bool { + self.widened + } + /// Whether no key value at all satisfies the predicate, which makes the whole relation /// empty. /// @@ -391,6 +443,12 @@ impl KeyBounds { /// analyzed twice. A pruned child reports itself unsatisfiable, so a parent whose every /// disjunct died is pruned in the same pass. The leftover `false` arguments are for /// `MirScalarExpr::reduce` to clean up. +/// +/// NOTE: Pruning suppresses errors that the pruned subtree would have raised, since a +/// predicate that cannot be satisfied is replaced rather than evaluated. `a IN (1,2) AND +/// a IN (3,4) AND 1/x > 0` returns empty instead of dividing by zero. That matches how +/// contradictory disjuncts have always been dropped here, but the reach is wider, because a +/// contradiction spanning two separate predicates is now visible too. /// Returns whether anything was pruned, and whether the conjunction is unsatisfiable as a /// whole. The latter covers contradictions that span two predicates, such as `c IN (1, 2)` /// alongside `c IN (3, 4)`, which no amount of pruning inside either one would reveal. @@ -441,9 +499,9 @@ fn prune_inner( /// Every expression that the predicate constrains to literal values somewhere. /// -/// Index-blind, and used for two things: cheaply rejecting an index whose key mentions an -/// expression the predicate never pins, and recommending a key to a user whose index was -/// too wide. +/// Index-blind. Treating the result as a key asks what the predicate says about all of them +/// at once, which is what drives contradiction pruning and what recommends a key to a user +/// whose index was too wide. pub fn literal_constrained_exprs<'a>( predicates: impl IntoIterator, ) -> Vec { @@ -457,3 +515,72 @@ pub fn literal_constrained_exprs<'a>( } found.into_iter().collect() } + +#[cfg(test)] +mod tests { + use mz_expr::func; + use mz_repr::{Datum, ReprScalarType}; + + use super::*; + + /// `(#0, #1) IN [(f(i), g(i)) for i in 0..n]`, as a disjunction of two-field conjunctions. + fn pair_list(n: i32, f: impl Fn(i32) -> i32, g: impl Fn(i32) -> i32) -> MirScalarExpr { + let eq = |col: usize, v: i32| { + MirScalarExpr::column(col).call_binary( + MirScalarExpr::literal_ok(Datum::Int32(v), ReprScalarType::Int32), + func::Eq, + ) + }; + MirScalarExpr::call_variadic( + Or, + (0..n) + .map(|i| MirScalarExpr::call_variadic(And, vec![eq(0, f(i)), eq(1, g(i))])) + .collect(), + ) + } + + fn key() -> Vec { + vec![MirScalarExpr::column(0), MirScalarExpr::column(1)] + } + + /// Removal drops a predicate that exactly bounds the key, which is sound only while the + /// lookup values stay inside that predicate's own bounds. A widening in the conjunction + /// produces a superset of the intersection instead, so the bound must report it: two + /// disjoint pair lists whose product exceeds `MAX_BOXES` would otherwise both be dropped + /// and the lookup would find rows the predicate rejects. + #[mz_ore::test] + fn widening_in_a_conjunction_is_reported() { + let key = key(); + let diagonal = pair_list(40, |i| i, |i| i); + let shifted = pair_list(40, |i| i, |i| i + 1); + + // Each list on its own is an exact, unwidened bound, so each is removable alone. + for p in [&diagonal, &shifted] { + let bounds = KeyBounds::extract(p, &key); + assert!(bounds.exact(), "a pair list exactly bounds the key"); + assert!(!bounds.widened(), "40 boxes is inside the budget"); + } + + // Together they exceed the budget, so the conjunction widens and says so. + let both = KeyBounds::conjunction([&diagonal, &shifted], &key); + assert!(both.widened(), "40 * 40 boxes must widen"); + assert!( + !both.lookup_values().expect("bounded").is_empty(), + "widening leaves lookup values the predicate rejects, which is why \ + removal has to consult `widened` and not just `exact`" + ); + } + + /// The budget is not reached, so the conjunction is a true intersection and removal stays + /// available. Two disjoint lists then leave nothing to look up at all. + #[mz_ore::test] + fn small_conjunctions_intersect_exactly() { + let key = key(); + let both = KeyBounds::conjunction( + [&pair_list(4, |i| i, |i| i), &pair_list(4, |i| i, |i| i + 1)], + &key, + ); + assert!(!both.widened()); + assert!(both.is_unsatisfiable(), "the two lists are disjoint"); + } +} diff --git a/test/sqllogictest/transform/literal_constraints.slt b/test/sqllogictest/transform/literal_constraints.slt index 889ffbbcf8dcd..e53f03010b6e5 100644 --- a/test/sqllogictest/transform/literal_constraints.slt +++ b/test/sqllogictest/transform/literal_constraints.slt @@ -1497,6 +1497,7 @@ SELECT u FROM t_uint_cast WHERE u::uint2 = 5::uint2 # still uses the index, and the constraints still come out of the filter. The predicate is # read once per candidate index, so the conjuncts that say nothing about the index key # neither cost anything nor stand in the way. +# See https://github.com/MaterializeInc/database-issues/issues/1924 statement ok CREATE TABLE wide (shop_id text, sku_code text, created_at int, rule text, flag bool) @@ -1668,3 +1669,27 @@ SELECT * FROM cover WHERE foo IN (1,2,3) AND bar IN (0,1,2,3,4,5,6,7,8,9,10,11) 1 1 1 2 2 2 3 3 3 + +# Two disjoint pair lists whose box product exceeds the analysis budget. The bound widens to +# stay inside it, which makes the lookup values a superset of what the predicate admits, so +# the constraints must stay in the filter rather than being removed. Asserted on rows rather +# than on the plan: over-approximating the lookup is allowed, returning a row is not. + +statement ok +CREATE TABLE pairs (a int, b int) + +statement ok +CREATE INDEX pairs_idx ON pairs(a, b) + +statement ok +INSERT INTO pairs VALUES (5, 5), (5, 6), (7, 8) + +query II rowsort +SELECT * FROM pairs WHERE (a,b) IN ((0,0),(1,1),(2,2),(3,3),(4,4),(5,5),(6,6),(7,7),(8,8),(9,9),(10,10),(11,11),(12,12),(13,13),(14,14),(15,15),(16,16),(17,17),(18,18),(19,19),(20,20),(21,21),(22,22),(23,23),(24,24),(25,25),(26,26),(27,27),(28,28),(29,29),(30,30),(31,31),(32,32),(33,33),(34,34),(35,35),(36,36),(37,37),(38,38),(39,39)) AND (a,b) IN ((0,1),(1,2),(2,3),(3,4),(4,5),(5,6),(6,7),(7,8),(8,9),(9,10),(10,11),(11,12),(12,13),(13,14),(14,15),(15,16),(16,17),(17,18),(18,19),(19,20),(20,21),(21,22),(22,23),(23,24),(24,25),(25,26),(26,27),(27,28),(28,29),(29,30),(30,31),(31,32),(32,33),(33,34),(34,35),(35,36),(36,37),(37,38),(38,39),(39,40)) +---- + +# The same shape under the budget, where the intersection is exact and stays empty. + +query II rowsort +SELECT * FROM pairs WHERE (a,b) IN ((0,0),(5,5)) AND (a,b) IN ((0,1),(5,6)) +----