Hi Dave,
>From logic point, I think the code below does the right thing, because
there is no predicates present.
if ((allChosenPredsAreEqualPreds) &&
(allBeginKeysMissing) &&
(allEndKeysMissing))
allChosenPredsAreEqualPreds = FALSE;
To fix the problem you found, I wonder if we add some additional checks
shown below.
if ((! skey->isUnique()) &&
(skey->areAllChosenPredsEqualPreds() ||
skey->areAllBeginKeysMissing() || skey->areAllEndKeysMissing()) &&
(NOT bef->noCheck()) &&
(CmpCommon::getDefault(HBASE_SQL_IUD_SEMANTICS) == DF_ON) &&
(CmpCommon::getDefault(HBASE_UPDEL_CURSOR_OPT) == DF_ON) &&
((ActiveSchemaDB()->getDefaults()).getAsLong(COMP_INT_74) == 0))
result = NULL;
thanks --Qifan
On Thu, Oct 1, 2015 at 1:00 PM, Dave Birdsall <[email protected]>
wrote:
> Hi,
>
>
>
> I’m working on code to cost DELETEs in Trafodion.
>
>
>
> One of the rules in the Optimizer (see ImplRule.cpp) is the
> HbaseDeleteRule. This rule decides whether to consider a delete plan
> consisting of just a “trafodion_delete” node. By experimentation, it seems
> to fire when all of the keys are covered with equality predicates. If only
> a leading subset of the keys are covered, it does not fire. But if none of
> the keys are covered by any predicates (example: delete from t;), it fires!
>
>
>
> I can see considering a “trafodion_delete” only plan if all the keys are
> covered. We are deleting a unique row. This plan uses the Hbase delete API,
> deleting one row at a time. When there is only one row, this makes perfect
> sense.
>
>
>
> The alternative plan is a Scan + a Tuple_flow + a Trafodion_delete or
> Trafodion_delete_vsbb, the latter uses the Hbase deleteRows API to delete
> several rows in one shot. The alternative plan is likely more efficient
> whenever we are deleting multiple rows.
>
>
>
> So it makes sense not to consider a “trafodion_delete” only plan when not
> all the keys are covered. But what seems odd is the case where there are no
> key predicates at all. Why would we consider such a plan then?
>
>
>
> Drilling down into the code: In HbaseDeleteRule::nextSubstitute is the
> following:
>
>
>
> if ((! skey->isUnique()) &&
>
> (skey->areAllChosenPredsEqualPreds()) &&
>
> (NOT bef->noCheck()) &&
>
> (CmpCommon::getDefault(HBASE_SQL_IUD_SEMANTICS) == DF_ON) &&
>
> (CmpCommon::getDefault(HBASE_UPDEL_CURSOR_OPT) == DF_ON) &&
>
> ((ActiveSchemaDB()->getDefaults()).getAsLong(COMP_INT_74) == 0))
>
> result = NULL;
>
>
>
> When this ‘if’ is taken, the “trafodion_delete” only plan is not
> considered.
>
>
>
> Here, “skey” is a SearchKey object which represents a begin/end key pair,
> that is, a subset of rows within the clustering key range to be processed.
> For the unique row case, “skey->isUnique()” returns true, so the “if” is
> not taken. So, the “trafodion_delete” only plan IS considered.
>
>
>
> For the non-unique case, “skey->isUnique()” returns false, and we look at
> the other conditions in the ‘if’. If the key is partially covered with
> equality predicates, “skey->areAllChosenPredsEqualPreds()” returns true.
> The other conditions also return true, and the ‘if’ is taken. So the
> “trafodion_delete” only plan IS NOT considered. But if there are no key
> predicates at all (the “delete from t;” example),
> “skey->areAllChosenPredsEqualPreds()” returns false.
>
>
>
> I think this is a bug. I think “skey->areAllChosenPredsEqualPreds()” should
> return true in this case also. It’s an example of the old
> not-handling-the-empty-set-case-properly bug.
>
>
>
> I investigated how this particular flag is set. At the end of
> SearchKeyWorkSpace::computeBeginKeyAndEndKeyValues (in file SearchKey.cpp)
> is the following code:
>
>
>
> if ((allChosenPredsAreEqualPreds) &&
>
> (allBeginKeysMissing) &&
>
> (allEndKeysMissing))
>
> allChosenPredsAreEqualPreds = FALSE;
>
>
>
> So, there is explicit code there to treat the empty set case differently.
> So, someone, somewhere, thought this was the right thing to do.
>
>
>
> But, I’m guessing the right thing to do is to simply delete this code.
>
>
>
> Does anyone have an opinion?
>
>
>
> Thanks,
>
>
>
> Dave
>
--
Regards, --Qifan
