Github user zellerh commented on a diff in the pull request:

    
https://github.com/apache/incubator-trafodion/pull/1051#discussion_r111208355
  
    --- Diff: core/sql/optimizer/RelExpr.cpp ---
    @@ -9055,19 +9056,33 @@ void Scan::setTableAttributes(CANodeId nodeId)
       setBaseCardinality(MIN_ONE (tableDesc->getNATable()->getEstRowCount())) ;
     }
     
    -NABoolean Scan::updateableIndex(IndexDesc *idx)
    +NABoolean Scan::updateableIndex(IndexDesc *idx, ValueIdSet& preds, 
    +                                NABoolean & needsHalloweenProtection)
     {
       //
    -  // Returns TRUE if the index (idx) can be used for a scan during 
    -  // an UPDATE. Halloween problem is protected with a sort using
    -  // Scan::requiresHalloweenForUpdateUsingIndexScan(). 
    -  // Returns FALSE only for certain embedded updates now.
    +  // Returns TRUE if the index (idx) can be used for a scan during an 
UPDATE.
    +  // Returns TRUE with needsHalloweenProtection also set to TRUE, if the 
index
    +  // needs a blocking sort for Halloween protection
    +  // Otherwise, returns FALSE to prevent use of this index. 
    +  // Using the index in this case requires Halloween protection, but is 
likely 
    +  // inefficient since there are no useful preds on the index key columns, 
so
    +  // we don't add this index to list of candidates.
       //
     
    -  if (!getGroupAttr()->isEmbeddedUpdate())
    -    return TRUE ;
    -
    +  // The conditions of when this index returns TRUE can also be expressed 
as
    +  // returns true for an index if it is 
    +  // a) a unique/clustering index or 
    +  // b) has a unique predicate on its key or 
    +  // c) has an equals or range predicate on all the index columns that 
    --- End diff --
    
    I would have thought that we look for any key predicates for this  index, 
not just on the updated columns.  We are forming a SearchKey below and check 
for uniqueness. Couldn't we just examine that SearchKey instead, for example by 
using methods areAllBeginKeysMissing() and areAllEndKeysMissing() or by calling 
getKeyPredicates() and checking whether the returned set is empty?
    
    Two examples, let's assume we have a base table with two indexes, one on 
column A and one on column B:
    
    ```
    -- example 1:
    update t set b=b+1 where a > 10; 
    -- We would like to use index on a (no Halloween protection needed)
    -- and we would like to exclude the index on b because it is not useful
    -- and it requires Halloween protection.
    
    -- example 2:
    update t set b=b+1 where b > 10;
    -- Here the index on a is not useful, but there is also no need to exclude 
it
    -- since it does not require Halloween protection. Index on b is useful but
    -- requires Halloween protection.
    ```
    
    Would this work? Sorry for providing so much detail, this is just an 
attempt to provide detailed feedback, and not meant to say that this is the 
only solution:
    
    * Rename the method to "indexIsUsefulForUpdateOrDelete"
    * Let _ixcols_ be the set of columns the index is defined on, excluding the 
clustering key columns added to non-unique indexes, if necessary.
    * Return TRUE if the predicates can be used to build a SearchKey for 
_ixcols_. Note that this excludes MDAM - in a future change we could check for 
MDAM predicates as well. Note also that it includes poor search key predicates 
- cost-based optimization should take care of that. Return FALSE otherwise (we 
would have to do a full index scan).
    * Return FALSE for the needsHalloweenProtection parameter only if a) 
_ixcols_ do not contain any columns that are updated or b) the index is unique 
(that includes the clustering index) or c) it has a unique predicate on the 
columns it is defined on (excluding the clustering key columns added to the end 
of the key) or d) we have an equals predicate on all the columns that get 
updated. Return TRUE otherwise - the index needs Halloween protection.
    * CQD UPDATE_USE_HALLOWEEN_INDEXES (Note: Shouldn't we use a separate CQD 
here, since this logic is very different from that for updating clustering 
keys?) be used to override the condition for Halloween protection:
      * OFF: Always return FALSE for indexes that require Halloween protection, 
so we never add a sort node
      * ON (the default): Follow the logic above
      * AGGRESSIVE: Always return FALSE for needsHalloweenProtection, meaning 
that we never force a sort and that we could encounter the Halloween problem - 
this would mean data corruption!



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---

Reply via email to