You are viewing a plain text version of this content. The canonical link for it is here.
Posted to codereview@trafodion.apache.org by anoopsharma00 <gi...@git.apache.org> on 2016/04/22 00:42:05 UTC

[GitHub] incubator-trafodion pull request: Various fixes (details below)

GitHub user anoopsharma00 opened a pull request:

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

    Various fixes (details below)

    -- ddl_transactions cqd is now on by default.
       All ddl, where allowed and supported, will run 
       within one transaction
    
    -- drop schema has been disabled to run within one transaction.
       When jira 1948 is fixed, it will be enabled.
    
    -- hdfs scan handles hive col values > 32K
    
    -- pcode handles varchar with len > 32K
    
    -- errors (gaps, unsorted order) for cqd values in nadefaults are
       detected without crashing arkcmp.
    
    -- volatile and regular schema drop returns the reason if an error occured
       (for ex: name of objects that could not be dropped)
    
    -- dateformat displays european format correctly with a blank
       seperator between date and time values
    
    -- dateformat timestamp in usa format displayes time with AM/PM
    
    -- dateformat inside a cast function returns correct values.


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

    $ git pull https://github.com/anoopsharma00/incubator-trafodion ddlxns2_br

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

    https://github.com/apache/incubator-trafodion/pull/444.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 #444
    
----
commit e1e727cbfb102a26d6fd0f06c8ad746f7fc64fa0
Author: Cloud User <ce...@ansharma-3.novalocal>
Date:   2016-04-11T17:53:34Z

    ddl xns default plus handling of errors with default constants placement

commit 6b45e1773d8be613431116c9b021e7dd03025b3d
Author: Cloud User <ce...@ansharma-3.novalocal>
Date:   2016-04-12T01:08:27Z

    Merge remote branch 'origin/master' into ddlxns2_br

commit 54ff2911c29bb0a0500cebefdfb158dbad989937
Author: Cloud User <ce...@ansharma-3.novalocal>
Date:   2016-04-12T14:19:22Z

    Merge remote branch 'origin/master' into ddlxns2_br

commit eded5a15397aab8d118cf4c246a48e9e7d027874
Author: Cloud User <ce...@ansharma-3.novalocal>
Date:   2016-04-20T00:27:41Z

    Merge remote branch 'origin/master' into ddlxns2_br

commit b7a6e54d8ea4043f0d736dcd43e8f45ef8a14b30
Author: Cloud User <ce...@ansharma-3.novalocal>
Date:   2016-04-21T15:19:00Z

    Merge remote branch 'origin/master' into ddlxns2_br

commit 1c52a1b4ef645c8c8be87b8dab132ae07bc621b6
Author: Anoop Sharma <ce...@ansharma-3.novalocal>
Date:   2016-04-21T22:21:59Z

    Various fixes (details below)
    
    -- ddl_transactions cqd is now on by default.
       All ddl, where allowed and supported, will run
       within one transaction
    
    -- drop schema has been disabled to run within one transaction.
       When jira 1948 is fixed, it will be enabled.
    
    -- hdfs scan handles hive col values > 32K
    
    -- pcode handles varchar with len > 32K
    
    -- errors (gaps, unsorted order) for cqd values in nadefaults are
       detected without crashing arkcmp.
    
    -- volatile and regular schema drop returns the reason if an error occured
       (for ex: name of objects that could not be dropped)
    
    -- dateformat displays european format correctly with a blank
       seperator between date and time values
    
    -- dateformat timestamp in usa format displayes time with AM/PM
    
    -- dateformat inside a cast function returns correct values.

----


---
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: Various fixes (details below)

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

    https://github.com/apache/incubator-trafodion/pull/444#discussion_r61010867
  
    --- Diff: core/sql/regress/compGeneral/TEST004 ---
    @@ -857,3 +857,8 @@ explain options 'f' st1;
     log;
     
     obey test004(clean_up);
    +--set parserflags 131072;
    +--select object_name from "_MD_".objects where schema_name like 'VOLATILE_SCHEMA%';
    +--drop implicit volatile schema cleanup cascade;
    --- End diff --
    
    For later?


---
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: Various fixes (details below)

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

    https://github.com/apache/incubator-trafodion/pull/444#discussion_r61009125
  
    --- Diff: core/sql/executor/ExExeUtilVolTab.cpp ---
    @@ -537,15 +539,23 @@ short ExExeUtilCleanupVolatileTablesTcb::work()
     	    char * schemaName = vi->get(0);
     	    retcode =
     	      dropVolatileSchema(masterGlob->getStatement()->getContext(),
    -				 schemaName, getHeap(), getGlobals());
    +				 schemaName, getHeap(), getGlobals(),
    +                                 getDiagsArea());
     	    if (retcode < 0)
     	      {
    +                // changes errors to warnings and move on to next schema.
    +                if (getDiagsArea())
    +                  getDiagsArea()->negateAllErrors();
    +
     		// clear diags and move on to next schema.
     		// Remember that an error was returned, we will
     		// return a warning at the end.
     		SQL_EXEC_ClearDiagnostics(NULL);
     		retcode = 0;
     
    +                if ((strlen(errorSchemas_) + strlen(schemaName)) < 1000)
    +                  strcat(errorSchemas_, schemaName);
    --- End diff --
    
    Do you want to stick any delimiter (say a comma or a space) between schema names? Maybe that is already present in schemaName? Also, if they don't all fit, do you want to put some indicator (such as "...") at the end?


---
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: Various fixes (details below)

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

    https://github.com/apache/incubator-trafodion/pull/444#discussion_r61008392
  
    --- Diff: core/sqf/sql/scripts/get_libhdfs_files ---
    @@ -42,8 +42,14 @@ fi
     LOGFILE=${LIBHDFS_TEMP_DIR}/build.log
     
     # Hadoop source tar file to build libhdfs from
    +#  Narendra
    +#HADOOP_DEP_VER=2.6.0-cdh5.4.4
     HADOOP_ID=hadoop-${HADOOP_DEP_VER}
    -HADOOP_MIRROR_URL=https://archive.apache.org/dist/hadoop/common/${HADOOP_ID}
    +if [ -z $HADOOP_MIRROR_BASE ]; then
    +   HADOOP_MIRROR_BASE=archive.apache.org
    +fi
    +echo "Here 1"
    +HADOOP_MIRROR_URL=http://${HADOOP_MIRROR_BASE}/apache/hadoop/common/${HADOOP_ID}
    --- End diff --
    
    Looks like the /dist/ part of the path name went away. Is that intended?


---
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: Various fixes (details below)

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

    https://github.com/apache/incubator-trafodion/pull/444#discussion_r61008633
  
    --- Diff: core/sql/bin/SqlciErrors.txt ---
    @@ -592,6 +592,7 @@
     2076 21000 99999 BEGINNER MAJOR DBADMIN HIST_NO_STATS_UEC should always be less than or equal to CQD HIST_NO_STATS_ROWCOUNT. Present Value of HIST_NO_STATS_ROWCOUNT is $0~string0.
     2077 ZZZZZ 99999 BEGINNER MINOR DBADMIN The max size $0~int0 must be greater than the initial size $1~int1.
     2078 ZZZZZ 99999 BEGINNER MAJOR DBADMIN Assertion failure during semantic query optimization ($0~string0) in file $1~string1 at line $2~int0. Attempting to recover and produce a plan.
    +2079 ZZZZZ 99999 BEGINNER MAJOR DBADMIN Error occurred during initialization of NADefaults structure. Make sure that default constants specified in enum DefaultConstants are set up correctly in defaultDefaults[] array.
    --- End diff --
    
    A user should never see this error, right? Only a developer who is fiddling with NADefaults?


---
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: Various fixes (details below)

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

    https://github.com/apache/incubator-trafodion/pull/444#discussion_r61010530
  
    --- Diff: core/sql/generator/GenRelScan.cpp ---
    @@ -216,41 +217,56 @@ int HbaseAccess::createAsciiColAndCastExpr(Generator * generator,
         }
     
       if (newGivenType->getTypeQualifier() == NA_CHARACTER_TYPE &&
    -      (CmpCommon::getDefaultString(HIVE_FILE_CHARSET) == "GBK" || CmpCommon::getDefaultString(HIVE_FILE_CHARSET) == "gbk") && CmpCommon::getDefaultString(HIVE_DEFAULT_CHARSET) == "UTF8" )
    -        needTranslate = TRUE;
    -
    +      (CmpCommon::getDefaultString(HIVE_FILE_CHARSET) == "GBK" || 
    --- End diff --
    
    Don't we case-shift these things before we get this far? Otherwise we have to worry about variants such as Gbk, gBK, etc.


---
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: Various fixes (details below)

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

    https://github.com/apache/incubator-trafodion/pull/444#discussion_r61008320
  
    --- Diff: core/sqf/sql/scripts/get_libhdfs_files ---
    @@ -42,8 +42,14 @@ fi
     LOGFILE=${LIBHDFS_TEMP_DIR}/build.log
     
     # Hadoop source tar file to build libhdfs from
    +#  Narendra
    +#HADOOP_DEP_VER=2.6.0-cdh5.4.4
     HADOOP_ID=hadoop-${HADOOP_DEP_VER}
    -HADOOP_MIRROR_URL=https://archive.apache.org/dist/hadoop/common/${HADOOP_ID}
    +if [ -z $HADOOP_MIRROR_BASE ]; then
    +   HADOOP_MIRROR_BASE=archive.apache.org
    +fi
    +echo "Here 1"
    --- End diff --
    
    Looks like a debug statement was left in


---
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: Various fixes (details below)

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

    https://github.com/apache/incubator-trafodion/pull/444#discussion_r61112170
  
    --- Diff: core/sql/executor/ExExeUtilVolTab.cpp ---
    @@ -554,7 +554,12 @@ short ExExeUtilCleanupVolatileTablesTcb::work()
     		retcode = 0;
     
                     if ((strlen(errorSchemas_) + strlen(schemaName)) < 1000)
    -                  strcat(errorSchemas_, schemaName);
    +                  {
    +                    strcat(errorSchemas_, schemaName);
    +                    strcat(errorSchemas_, " ");
    +                  }
    +                else
    +                  strcat(errorSchemas_, "..."); // could not fit
    --- End diff --
    
    We would want to do this at most once. As written we could keep appending dots and have a buffer overflow. (Apologies; I know this change was in response to one of my earlier comments.)


---
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: Various fixes (details below)

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

    https://github.com/apache/incubator-trafodion/pull/444#discussion_r61009421
  
    --- Diff: core/sql/exp/exp_datetime.cpp ---
    @@ -2835,7 +2836,8 @@ ExpDatetime::convAsciiToDate(char *srcData,
         };  
         break;
     
    -  case DATETIME_FORMAT_TS2: // DD.MM.YYYY:HH24:MI:SS
    +  case DATETIME_FORMAT_TS2:  // DD.MM.YYYY:HH24:MI:SS
    +  case DATETIME_FORMAT_TS10: // DD.MM.YYYY:HH24:MI:SS
    --- End diff --
    
    Comment is wrong? No colon after YYYY for TS10?


---
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: Various fixes (details below)

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

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


---
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.
---