You are viewing a plain text version of this content. The canonical link for it is here.
Posted to codereview@trafodion.apache.org by selvaganesang <gi...@git.apache.org> on 2016/03/18 13:52:12 UTC

[GitHub] incubator-trafodion pull request: [TRAFODION-1896] CQD TRAF_UPSERT...

GitHub user selvaganesang opened a pull request:

    https://github.com/apache/incubator-trafodion/pull/393

    [TRAFODION-1896] CQD TRAF_UPSERT_WITH_INSERT_DEFAULT_SEMANTICS for no…

    …n-aligned format
    
    This CQD is now honored for non-aligned format by replacing the column
    value with default values for omitted columns and put into hbase table.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/selvaganesang/incubator-trafodion trafodion-1896

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/incubator-trafodion/pull/393.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #393
    
----
commit 53dcf926625f645fa4c5a3ee1df6ce7ba7569717
Author: selvaganesang <se...@esgyn.com>
Date:   2016-03-18T12:24:02Z

    [TRAFODION-1896] CQD TRAF_UPSERT_WITH_INSERT_DEFAULT_SEMANTICS for non-aligned format
    
    This CQD is now honored for non-aligned format by replacing the column
    value with default values for omitted columns and put into hbase table.

----


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request: [TRAFODION-1896] CQD TRAF_UPSERT...

Posted by selvaganesang <gi...@git.apache.org>.
Github user selvaganesang commented on a diff in the pull request:

    https://github.com/apache/incubator-trafodion/pull/393#discussion_r57943451
  
    --- Diff: core/sql/optimizer/BindRelExpr.cpp ---
    @@ -10165,22 +10165,35 @@ base table then the old version of the row will have to be deleted from
     indexes, and a new version inserted. Upsert is being transformed to merge
     so that we can delete the old version of an updated row from the index.
     
    -Upsert is also converted into merge when there are omitted cols with default values and 
    -TRAF_UPSERT_WITH_INSERT_DEFAULT_SEMANTICS is set to  OFF in case of aligned format table or 
    +Upsert is also converted into merge when TRAF_UPSERT_MODE is set to MERGE and 
    +there are omitted cols with default values in case of aligned format table or 
     omitted current timestamp cols in case of non-aligned row format
     */
     NABoolean Insert::isUpsertThatNeedsMerge(NABoolean isAlignedRowFormat, NABoolean omittedDefaultCols,
                                        NABoolean omittedCurrentDefaultClassCols) const
     {
    +  // The necessary conditions to convert upsert to merge and
       if (isUpsert() && 
           (NOT getIsTrafLoadPrep()) && 
           (NOT (getTableDesc()->isIdentityColumnGeneratedAlways() && getTableDesc()->hasIdentityColumnInClusteringKey())) && 
           (NOT (getTableDesc()->getClusteringIndex()->getNAFileSet()->hasSyskey())) && 
    -       ((getTableDesc()->hasSecondaryIndexes()) ||
    -         (( NOT isAlignedRowFormat) && omittedCurrentDefaultClassCols) ||
    -             ((isAlignedRowFormat && omittedDefaultCols
    -              && (CmpCommon::getDefault(TRAF_UPSERT_WITH_INSERT_DEFAULT_SEMANTICS) == DF_OFF)))
    -       ))
    +        // table has secondary indexes or
    +        (getTableDesc()->hasSecondaryIndexes() ||
    +          // CQD is set to MERGE  
    +          ((CmpCommon::getDefault(TRAF_UPSERT_MODE) == DF_MERGE) &&
    +            // omitted current default columns with non-aligned row format tables
    +            // or omitted default columns with aligned row format tables 
    +            (((NOT isAlignedRowFormat) && omittedCurrentDefaultClassCols) ||
    +            (isAlignedRowFormat && omittedDefaultCols))) ||
    +          // CQD is set to Optimal, for non-aligned row format with omitted 
    +          // current columns, it is converted into merge though it is not
    +          // optimal for performance - This is done to ensure that when the 
    +          // CQD is set to optimal, non-aligned format would behave like 
    +          // merge when any column is  omitted 
    --- End diff --
    
    What I intended to say is this:
    Non-aligned format table usually behaves like merge though it is not changed into MERGE statement because Trafodion engine doesn't store the omitted default column in the raw hbase table.
    When the upsert statement has omitted current default columns, it is changed into MERGE statement though it is not optimal because we want it to behave like above for this case also. Otherwise, it might be confusing to the application.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request: [TRAFODION-1896] CQD TRAF_UPSERT...

Posted by sureshsubbiah <gi...@git.apache.org>.
Github user sureshsubbiah commented on a diff in the pull request:

    https://github.com/apache/incubator-trafodion/pull/393#discussion_r57931410
  
    --- Diff: core/sql/optimizer/BindRelExpr.cpp ---
    @@ -10165,22 +10165,35 @@ base table then the old version of the row will have to be deleted from
     indexes, and a new version inserted. Upsert is being transformed to merge
     so that we can delete the old version of an updated row from the index.
     
    -Upsert is also converted into merge when there are omitted cols with default values and 
    -TRAF_UPSERT_WITH_INSERT_DEFAULT_SEMANTICS is set to  OFF in case of aligned format table or 
    +Upsert is also converted into merge when TRAF_UPSERT_MODE is set to MERGE and 
    +there are omitted cols with default values in case of aligned format table or 
     omitted current timestamp cols in case of non-aligned row format
     */
     NABoolean Insert::isUpsertThatNeedsMerge(NABoolean isAlignedRowFormat, NABoolean omittedDefaultCols,
                                        NABoolean omittedCurrentDefaultClassCols) const
     {
    +  // The necessary conditions to convert upsert to merge and
       if (isUpsert() && 
           (NOT getIsTrafLoadPrep()) && 
           (NOT (getTableDesc()->isIdentityColumnGeneratedAlways() && getTableDesc()->hasIdentityColumnInClusteringKey())) && 
           (NOT (getTableDesc()->getClusteringIndex()->getNAFileSet()->hasSyskey())) && 
    -       ((getTableDesc()->hasSecondaryIndexes()) ||
    -         (( NOT isAlignedRowFormat) && omittedCurrentDefaultClassCols) ||
    -             ((isAlignedRowFormat && omittedDefaultCols
    -              && (CmpCommon::getDefault(TRAF_UPSERT_WITH_INSERT_DEFAULT_SEMANTICS) == DF_OFF)))
    -       ))
    +        // table has secondary indexes or
    +        (getTableDesc()->hasSecondaryIndexes() ||
    +          // CQD is set to MERGE  
    +          ((CmpCommon::getDefault(TRAF_UPSERT_MODE) == DF_MERGE) &&
    +            // omitted current default columns with non-aligned row format tables
    +            // or omitted default columns with aligned row format tables 
    +            (((NOT isAlignedRowFormat) && omittedCurrentDefaultClassCols) ||
    +            (isAlignedRowFormat && omittedDefaultCols))) ||
    +          // CQD is set to Optimal, for non-aligned row format with omitted 
    +          // current columns, it is converted into merge though it is not
    +          // optimal for performance - This is done to ensure that when the 
    +          // CQD is set to optimal, non-aligned format would behave like 
    +          // merge when any column is  omitted 
    --- End diff --
    
    I suppose the comment in line 10192 should say "when any current default column is omitted" ?
    
    A table some place in code that shows what the behaviour is with 3 cqd settings for aligned omitted default, aligned omitted default current non-aligned omitted default, non-aligned omitted current default will be helpful.  


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request: [TRAFODION-1896] CQD TRAF_UPSERT...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/incubator-trafodion/pull/393


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request: [TRAFODION-1896] CQD TRAF_UPSERT...

Posted by selvaganesang <gi...@git.apache.org>.
Github user selvaganesang commented on a diff in the pull request:

    https://github.com/apache/incubator-trafodion/pull/393#discussion_r57943651
  
    --- Diff: core/sql/optimizer/BindRelExpr.cpp ---
    @@ -10165,22 +10165,35 @@ base table then the old version of the row will have to be deleted from
     indexes, and a new version inserted. Upsert is being transformed to merge
     so that we can delete the old version of an updated row from the index.
     
    -Upsert is also converted into merge when there are omitted cols with default values and 
    -TRAF_UPSERT_WITH_INSERT_DEFAULT_SEMANTICS is set to  OFF in case of aligned format table or 
    +Upsert is also converted into merge when TRAF_UPSERT_MODE is set to MERGE and 
    +there are omitted cols with default values in case of aligned format table or 
     omitted current timestamp cols in case of non-aligned row format
     */
     NABoolean Insert::isUpsertThatNeedsMerge(NABoolean isAlignedRowFormat, NABoolean omittedDefaultCols,
                                        NABoolean omittedCurrentDefaultClassCols) const
     {
    +  // The necessary conditions to convert upsert to merge and
       if (isUpsert() && 
           (NOT getIsTrafLoadPrep()) && 
           (NOT (getTableDesc()->isIdentityColumnGeneratedAlways() && getTableDesc()->hasIdentityColumnInClusteringKey())) && 
           (NOT (getTableDesc()->getClusteringIndex()->getNAFileSet()->hasSyskey())) && 
    -       ((getTableDesc()->hasSecondaryIndexes()) ||
    -         (( NOT isAlignedRowFormat) && omittedCurrentDefaultClassCols) ||
    -             ((isAlignedRowFormat && omittedDefaultCols
    -              && (CmpCommon::getDefault(TRAF_UPSERT_WITH_INSERT_DEFAULT_SEMANTICS) == DF_OFF)))
    -       ))
    +        // table has secondary indexes or
    +        (getTableDesc()->hasSecondaryIndexes() ||
    +          // CQD is set to MERGE  
    +          ((CmpCommon::getDefault(TRAF_UPSERT_MODE) == DF_MERGE) &&
    +            // omitted current default columns with non-aligned row format tables
    +            // or omitted default columns with aligned row format tables 
    +            (((NOT isAlignedRowFormat) && omittedCurrentDefaultClassCols) ||
    +            (isAlignedRowFormat && omittedDefaultCols))) ||
    +          // CQD is set to Optimal, for non-aligned row format with omitted 
    +          // current columns, it is converted into merge though it is not
    +          // optimal for performance - This is done to ensure that when the 
    +          // CQD is set to optimal, non-aligned format would behave like 
    +          // merge when any column is  omitted 
    --- End diff --
    
    I will add the table in the comments section from the message in user dlist when I get a chance to update this file again


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---