Skip to content

Commit

Permalink
WL#7636 post-push patch: Fix a too strict restriction on join-pushabi…
Browse files Browse the repository at this point in the history
…lity

Fix pushability restriction introduced by the previous patch:

...............................
commit 1eb9f4635b9799b763902225a422bb22668ad14e
Author: Ole John Aske <[email protected]>
Date:   Thu Feb 13 17:38:58 2020 +0100

    WL#7636 Post-push fix, table coverage of trigger conditions

    Fix an issue related to how the set of 'used_tables' of a trigger
    condition is calculated. Such triggers are attached to a table
    (last table in join-nest) but covers a wider set of tables.
    .... more ....
....................

That patch ^^^ caused both a FOUND_MATCH- and a IS_NOT_NULL_COMPL-trigger
to be handled as trigger-condition-filter, which would limit the
pushability of the tables covered by their set of 'inner_tables'.

That is too restrictive as only FOUND_MATCH-triggers, possibly embedded
withing IS_NOT_NULL_COMP-triggers, restricts pushability.

It turned out that this too strict check, unintentionally disabled
the pushability of Q13 in the TPC-H test suite (among others). That
query has the trigger condition on the orders table:

<if>(is_not_null_compl(orders),
  (not((`orders`.`o_comment` like '%special%requests%'))), true)

As there is no FOUND_MATCH-trigger in this condition, it should not
contribute to restrict pushability. After the post-push patch pasted
above, it *did*.

This patch partly revert that incorrect patch, and ignore any
IS_NOT_NULL_COMPL-trigger as long as a FOUND_MATCH-trigger is
not embedded within it.

Reviewed by: Mikael Ronstrom <[email protected]>
  • Loading branch information
Ole John authored and dahlerlend committed Mar 23, 2020
1 parent 8942610 commit 22058fd
Showing 1 changed file with 37 additions and 13 deletions.
50 changes: 37 additions & 13 deletions storage/ndb/plugin/ha_ndbcluster_push.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1423,12 +1423,12 @@ void ndb_pushed_builder_ctx::validate_join_nest(ndb_table_access_map inner_nest,
ndb_table_access_map filter_cond;

/**
* Check conditions inside nest(s) for possible trigger conditions.
* Check conditions inside nest(s) for possible FOUND_MATCH-triggers.
* These are effectively evaluated 'higher up' in the nest structure
* when we have found a join-match, or created a null-extension
* for all 'used_tables()' in the trigger condition.
* So we collect the aggregated map of tables possibly affected by
* such trigger conditions in 'filter_cond'
* these MATCH-filters in 'filter_cond'
*
* Example: select straight_join *
* from
Expand All @@ -1446,8 +1446,8 @@ void ndb_pushed_builder_ctx::validate_join_nest(ndb_table_access_map inner_nest,
* For some (legacy?) reason the optimizer will attach the trigger condition
* to table t2 in the query plan 't1,t2,t3', as all referred tables(t1,t2)
* are available at this point.
* However, this ignores the encapsulating (t2,t3) trigger, which require
* the condition to be evaluated after t2 & t3 rows are produced. The
* However, this ignores the encapsulating FOUND_MATCH(t2,t3) trigger,
* which require the condition to also have a matching t3 row. The
* WalkItem below will identify such triggers and calculate the real table
* coverage of them.
*
Expand All @@ -1469,28 +1469,52 @@ void ndb_pushed_builder_ctx::validate_join_nest(ndb_table_access_map inner_nest,
AQP::Table_access *table = m_plan.get_table_access(tab_no);
const Item *cond = table->get_condition();
if (cond != nullptr) {
// Check 'cond' for match trigger / filters
table_map filtered_tables = 0;
struct {
table_map nest_scope; // Aggregated 'inner_tables' scope of triggers
table_map found_match; // FOUND_MATCH-trigger scope
} trig_cond = {0, 0};

// Check 'cond' for match trigger / filters
WalkItem(const_cast<Item *>(cond), enum_walk::PREFIX,
[&filtered_tables](Item *item) {
[&trig_cond](Item *item) {
if (item->type() == Item::FUNC_ITEM) {
const Item_func *func_item =
static_cast<const Item_func *>(item);
if (func_item->functype() == Item_func::TRIG_COND_FUNC) {
const Item_func_trig_cond *func_trig =
static_cast<const Item_func_trig_cond *>(func_item);
// is a FOUND_MATCH or IS_NOT_NULL_COMPL trigger
// Trigger is evaluated 'on top' of the inner_tables
filtered_tables |= func_trig->get_inner_tables();

/**
* The FOUND_MATCH-trigger may be encapsulated inside
* multiple IS_NOT_NULL_COMPL-triggers, which defines
* the scope of the triggers. Aggregate these
* 'inner_tables' scopes.
*/
trig_cond.nest_scope |= func_trig->get_inner_tables();

if (func_trig->get_trig_type() ==
Item_func_trig_cond::FOUND_MATCH) {
// The FOUND_MATCH-trigger is evaluated on top of
// the collected trigger nest_scope.
trig_cond.found_match |= trig_cond.nest_scope;
return true; // break out of this cond-branch
}
}
}
return false; // continue WalkItem
}); // End WalkItem' and lambda func

if (filtered_tables != 0) {
ndb_table_access_map map = get_table_map(filtered_tables);
filter_cond.add(map);
if (trig_cond.found_match != 0) {
const ndb_table_access_map map = get_table_map(trig_cond.found_match);

/**
* Only FOUND_MATCH-triggers partly overlapping join_scope will
* restrict push. (Else it is completely evaluated either before
* or after the pushed_join, thus does not affect it.
*/
if (map.is_overlapping(m_join_scope) && !map.contain(m_join_scope)) {
filter_cond.add(map);
}
}
}
}
Expand Down

0 comments on commit 22058fd

Please sign in to comment.