trafodion-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Qifan Chen <qifan.c...@esgyn.com>
Subject Re: Optimizer question
Date Thu, 01 Oct 2015 20:12:42 GMT
Hi Dave,

I guess the debating point is about the meaning of "areAllChosenPreds" in
method areAllChosenPredsEqualPreds().

Since the subject is a searchKey instance, areAllChosenPreds is well
defined and should not be empty (at least my interpretation).

I am afraid that if we remove those 4 lines of code, later if the same
method is used, we still forget to test the empty-ness of the key which may
be important.

Thanks --Qifan

On Thu, Oct 1, 2015 at 2:31 PM, Qifan Chen <qifan.chen@esgyn.com> wrote:

> Hi Dave,
>
> You may be right with the mathematician's interpretation :-).
>
> My thinking of that method is that it tests two things:
> 1. existing predicates on keys
> 2. all these predicates are equal predicates
>
> If there is no predicate on keys (which is this case is about), then the
> 1st test above fail, and therefore the method should return false.
>
> Thanks --Qifan
>
> On Thu, Oct 1, 2015 at 1:44 PM, Dave Birdsall <dave.birdsall@esgyn.com>
> wrote:
>
>> Hi,
>>
>> Hmmm... I would have thought the logical thing would be: every universally
>> quantified predicate is true on an empty set. At least that's the way a
>> mathematician or logician would look at it.
>>
>> I could add the extra checks, but this and one other place (similar code
>> in
>> an update rule) are the only places where this particular function
>> areAllChosenPredsEqualPreds() are called. It would seem simpler to delete
>> 4
>> lines of code than to add 2 more tests in 2 places?
>>
>> Dave
>>
>> -----Original Message-----
>> From: Qifan Chen [mailto:qifan.chen@esgyn.com]
>> Sent: Thursday, October 1, 2015 11:39 AM
>> To: dev <dev@trafodion.incubator.apache.org>
>> Subject: Re: Optimizer question
>>
>> 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 <dave.birdsall@esgyn.com>
>> 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
>>
>
>
>
> --
> Regards, --Qifan
>
>


-- 
Regards, --Qifan

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message