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 2018/04/02 16:42:40 UTC

[GitHub] trafodion pull request #1502: various fixes, details in pull request

GitHub user anoopsharma00 opened a pull request:

    https://github.com/apache/trafodion/pull/1502

    various fixes, details in pull request

    -- JIRA 2980 support for INSTR function.
       INSTR(source_string, pattern, startPos, occurrence)
       Search for pattern in source_string.
       Start at startPos'th character. Optional, default 1
       Return occurrence'th occurrence. Optional, default 1
       (exp/exp_function.cpp/h, exp/ExpPCodeClauseGen.cpp,
        generator/GenItemFunc.cpp, optimizer/BindItemExpr.cpp, ItemExpr.cpp,
        optimizer/ItemFunc.h, SynthType.cpp, parser/sqlparser.y)
    
    -- hivemd returns precision, scale, display_datatype
      (comexe/ComTdbExeUtil.h, common/BaseTypes.cpp,NAType.cpp/h,
       executor/ExExeUtilGet.cpp, ExExeUtil.h)
    
    -- hive decimal type is treated as sql NUMERIC for performance.
      (common/NAType.cpp)
    
    -- hive select from hivemd with prepare and multiple executes work
      (executor/ExExeUtilGet.cpp)
    
    -- fix for crash in sql buffer pool when allocating large size rows.
       (executor/sql_buffer.cpp)
    
    -- SIGN(op) now returns error if op is not numeric
       (BindItemExpr.cpp)
    
    -- strings of length zero are no longer cached. Caching them causes problem
       during backpatching.
       (optimizer/ItemCache.cpp)
    
    --- compile time predicates on constants were not being created correctly
        in some cases. That has been fixed.
       (ValueDesc.cpp)
    
    -- trim now supported on scaled numeric, float and bignum datatypes
       (BindItemExpr.cpp)
    
    -- CASE is now supported on scaled numerics, float, bignum
       (BindItemExpr.cpp)
    
    -- char_length is now supported on numerics
       (ItemFunc.h, BindItemExpr.cpp)
    
    -- DAYOFMONTH now allowed only on datetime datatypes
       (common/OperTypeEnum.h, sqlparser.y, BindItemExpr.cpp, ItemExpr.cpp)
    
    -- TO_TIME now allowed only on 'datetime with time' or character datatypes.
       (BindItemExpr.cpp, bin/SqlciErrors.txt)
    
    -- updated expected file for compGeneral/test042

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

    $ git pull https://github.com/anoopsharma00/trafodion ansharma_trafixes_deliver2_br

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

    https://github.com/apache/trafodion/pull/1502.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 #1502
    
----
commit 43984c46cc9ea2cfc0e816080e93e0dc7e61369a
Author: Anoop Sharma <an...@...>
Date:   2018-04-02T16:40:16Z

    various fixes
    
    -- JIRA 2980 support for INSTR function.
       INSTR(source_string, pattern, startPos, occurrence)
       Search for pattern in source_string.
       Start at startPos'th character. Optional, default 1
       Return occurrence'th occurrence. Optional, default 1
       (exp/exp_function.cpp/h, exp/ExpPCodeClauseGen.cpp,
        generator/GenItemFunc.cpp, optimizer/BindItemExpr.cpp, ItemExpr.cpp,
        optimizer/ItemFunc.h, SynthType.cpp, parser/sqlparser.y)
    
    -- hivemd returns precision, scale, display_datatype
      (comexe/ComTdbExeUtil.h, common/BaseTypes.cpp,NAType.cpp/h,
       executor/ExExeUtilGet.cpp, ExExeUtil.h)
    
    -- hive decimal type is treated as sql NUMERIC for performance.
      (common/NAType.cpp)
    
    -- hive select from hivemd with prepare and multiple executes work
      (executor/ExExeUtilGet.cpp)
    
    -- fix for crash in sql buffer pool when allocating large size rows.
       (executor/sql_buffer.cpp)
    
    -- SIGN(op) now returns error if op is not numeric
       (BindItemExpr.cpp)
    
    -- strings of length zero are no longer cached. Caching them causes problem
       during backpatching.
       (optimizer/ItemCache.cpp)
    
    --- compile time predicates on constants were not being created correctly
        in some cases. That has been fixed.
       (ValueDesc.cpp)
    
    -- trim now supported on scaled numeric, float and bignum datatypes
       (BindItemExpr.cpp)
    
    -- CASE is now supported on scaled numerics, float, bignum
       (BindItemExpr.cpp)
    
    -- char_length is now supported on numerics
       (ItemFunc.h, BindItemExpr.cpp)
    
    -- DAYOFMONTH now allowed only on datetime datatypes
       (common/OperTypeEnum.h, sqlparser.y, BindItemExpr.cpp, ItemExpr.cpp)
    
    -- TO_TIME now allowed only on 'datetime with time' or character datatypes.
       (BindItemExpr.cpp, bin/SqlciErrors.txt)
    
    -- updated expected file for compGeneral/test042

----


---

[GitHub] trafodion pull request #1502: various fixes, details in pull request

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

    https://github.com/apache/trafodion/pull/1502#discussion_r179541863
  
    --- Diff: core/sql/executor/ExExeUtilGet.cpp ---
    @@ -5795,40 +5833,51 @@ short ExExeUtilHiveMDaccessTcb::work()
                 str_cpy(infoCol->colName, 
                         (hcd ? hcd->name_ : hpd->name_), 256, ' ');
     
    -            infoCol->fsDatatype = 
    -              getFSTypeFromHiveColType(hcd ? hcd->type_ : hpd->type_);
    -
    -	    if (infoCol->fsDatatype < 0)
    +            Lng32 fstype = -1;
    +            Lng32 length = -1;
    +            Lng32 precision = -1;
    +            Lng32 scale = -1;
    +            char sqlType[50];
    +            char displayType[100];
    --- End diff --
    
    I get a little worried when I see hard-coded lengths. Elsewhere for example the displayType seems to be 96. Seems like if we are not checking buffer lengths at run time we should be using some enum or const ints for these items or risk buffer overrun problems down the line when someone else is too casual about these lengths.


---

[GitHub] trafodion pull request #1502: various fixes, details in pull request

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

    https://github.com/apache/trafodion/pull/1502#discussion_r179617202
  
    --- Diff: core/sql/executor/ExExeUtilGet.cpp ---
    @@ -5795,40 +5833,51 @@ short ExExeUtilHiveMDaccessTcb::work()
                 str_cpy(infoCol->colName, 
                         (hcd ? hcd->name_ : hpd->name_), 256, ' ');
     
    -            infoCol->fsDatatype = 
    -              getFSTypeFromHiveColType(hcd ? hcd->type_ : hpd->type_);
    -
    -	    if (infoCol->fsDatatype < 0)
    +            Lng32 fstype = -1;
    +            Lng32 length = -1;
    +            Lng32 precision = -1;
    +            Lng32 scale = -1;
    +            char sqlType[50];
    +            char displayType[100];
    +            char charset[50];
    +            retcode = 
    +              getTypeAttrsFromHiveColType(hcd ? hcd->type_ : hpd->type_,
    +                                          htd->getSDs()->isOrcFile(),
    +                                          fstype, length, precision, scale,
    +                                          sqlType, displayType, charset);
    +                 
    +	    if (retcode < 0)
     	      {
    +                // add a warning and continue.
     		char strP[300];
    -		sprintf(strP, "Datatype %s is not supported.", 
    -                        (hcd ? hcd->type_ : hpd->type_));
    -		*diags << DgSqlCode(-CLI_GET_METADATA_INFO_ERROR)
    +		sprintf(strP, "Datatype %s for column '%s' in table %s.%s.%s is not supported. This table will be ignored.", 
    --- End diff --
    
    snsprintf is a good idea. It will protect against buffer overruns.


---

[GitHub] trafodion pull request #1502: various fixes, details in pull request

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

    https://github.com/apache/trafodion/pull/1502#discussion_r179546844
  
    --- Diff: core/sql/optimizer/ValueDesc.cpp ---
    @@ -6381,35 +6380,47 @@ void ValueIdList::convertToTextKey(const ValueIdList& keyList, NAString& result)
     	 }
            else
     	 {
    -	   short vLen = val.length();
    +           short vLen = val.length();
     
     	   if ((constType->getTypeQualifier()  == NA_NUMERIC_TYPE) &&
     	       (((NumericType*)constType)->isExact()) &&
    -               (NOT ((NumericType&)type).isBigNum()) &&
    +               (NOT ((NumericType*)constType)->isBigNum()) &&
     	       (constType->getScale() > 0))
     	     {
    -	       NAString newVal;
    -	       if (vLen <= constType->getScale())
    -		 {
    -		   newVal = "0.";
    -		   for (Lng32 i = 0; i < (constType->getScale() - vLen); i++)
    -		     {
    -		       newVal += "0";
    -		     }
    -		   newVal += val;
    -		 }
    -	       else
    -		 {
    -                   // get digits to the left of scale
    -		   newVal = val(0, vLen - constType->getScale() );
    -
    -		   newVal += ".";
    -		   newVal += val(vLen - constType->getScale(), constType->getScale());
    -		 }
    +               // See how many positions the result will take in the display
    +               Lng32 t = constType->getDisplayLength(constType->getFSDatatype(),
    +                                               constType->getNominalSize(),
    +                                               constType->getPrecision(),
    +                                               constType->getScale(),
    +                                               0);
    +
    +               char strval[100];
    --- End diff --
    
    If you make this "char strval[t+1]" you will be guaranteed that you will never have a buffer overrun here.


---

[GitHub] trafodion pull request #1502: various fixes, details in pull request

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

    https://github.com/apache/trafodion/pull/1502#discussion_r179617647
  
    --- Diff: core/sql/optimizer/ValueDesc.cpp ---
    @@ -6381,35 +6380,47 @@ void ValueIdList::convertToTextKey(const ValueIdList& keyList, NAString& result)
     	 }
            else
     	 {
    -	   short vLen = val.length();
    +           short vLen = val.length();
     
     	   if ((constType->getTypeQualifier()  == NA_NUMERIC_TYPE) &&
     	       (((NumericType*)constType)->isExact()) &&
    -               (NOT ((NumericType&)type).isBigNum()) &&
    +               (NOT ((NumericType*)constType)->isBigNum()) &&
     	       (constType->getScale() > 0))
     	     {
    -	       NAString newVal;
    -	       if (vLen <= constType->getScale())
    -		 {
    -		   newVal = "0.";
    -		   for (Lng32 i = 0; i < (constType->getScale() - vLen); i++)
    -		     {
    -		       newVal += "0";
    -		     }
    -		   newVal += val;
    -		 }
    -	       else
    -		 {
    -                   // get digits to the left of scale
    -		   newVal = val(0, vLen - constType->getScale() );
    -
    -		   newVal += ".";
    -		   newVal += val(vLen - constType->getScale(), constType->getScale());
    -		 }
    +               // See how many positions the result will take in the display
    +               Lng32 t = constType->getDisplayLength(constType->getFSDatatype(),
    +                                               constType->getNominalSize(),
    +                                               constType->getPrecision(),
    +                                               constType->getScale(),
    +                                               0);
    +
    +               char strval[100];
    --- End diff --
    
    good idea.


---

[GitHub] trafodion pull request #1502: various fixes, details in pull request

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

    https://github.com/apache/trafodion/pull/1502#discussion_r179560631
  
    --- Diff: core/sql/exp/exp_function.cpp ---
    @@ -2224,12 +2232,51 @@ ex_function_char_length_doublebyte::eval(char *op_data[],
       return ex_expr::EXPR_OK;
     };
     
    +Lng32 ex_function_position::errorChecks(Lng32 startPos, Lng32 occurrence,
    +                                        CollHeap* heap, ComDiagsArea** diagsArea)
    +{
    +  // startPos is 1 based. Cannot be <= 0
    +  if (startPos < 0)
    +    {
    +      ExRaiseSqlError(heap, diagsArea, (ExeErrorCode)1572);
    --- End diff --
    
    1572 can't be cast to ExeErrorCode unless this error code is added to ExpErrorEnum.h


---

[GitHub] trafodion pull request #1502: various fixes, details in pull request

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

    https://github.com/apache/trafodion/pull/1502#discussion_r179617591
  
    --- Diff: core/sql/executor/ExExeUtilGet.cpp ---
    @@ -5795,40 +5833,51 @@ short ExExeUtilHiveMDaccessTcb::work()
                 str_cpy(infoCol->colName, 
                         (hcd ? hcd->name_ : hpd->name_), 256, ' ');
     
    -            infoCol->fsDatatype = 
    -              getFSTypeFromHiveColType(hcd ? hcd->type_ : hpd->type_);
    -
    -	    if (infoCol->fsDatatype < 0)
    +            Lng32 fstype = -1;
    +            Lng32 length = -1;
    +            Lng32 precision = -1;
    +            Lng32 scale = -1;
    +            char sqlType[50];
    +            char displayType[100];
    +            char charset[50];
    +            retcode = 
    +              getTypeAttrsFromHiveColType(hcd ? hcd->type_ : hpd->type_,
    +                                          htd->getSDs()->isOrcFile(),
    +                                          fstype, length, precision, scale,
    +                                          sqlType, displayType, charset);
    +                 
    +	    if (retcode < 0)
     	      {
    +                // add a warning and continue.
     		char strP[300];
    -		sprintf(strP, "Datatype %s is not supported.", 
    -                        (hcd ? hcd->type_ : hpd->type_));
    -		*diags << DgSqlCode(-CLI_GET_METADATA_INFO_ERROR)
    +		sprintf(strP, "Datatype %s for column '%s' in table %s.%s.%s is not supported. This table will be ignored.", 
    +                        (hcd ? hcd->type_ : hpd->type_),
    +                        (hcd ? hcd->name_ : hpd->name_),
    +                        hiveCat_, hiveSch_, htd->tblName_);
    +		*diags << DgSqlCode(CLI_GET_METADATA_INFO_ERROR)
     		       << DgString0(strP);
    -		
    -		step_ = HANDLE_ERROR_;
    -		break;
    +
    +                step_ = ADVANCE_ROW_;
    --- End diff --
    
    If that error condition is reached, we dont return that row. It is skipped  and we move
    to the next row through step ADVANCE_ROW_.
    The warning is maintained in the diags area which is returned when the next row
    or EOD is returned.


---

[GitHub] trafodion pull request #1502: various fixes, details in pull request

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

    https://github.com/apache/trafodion/pull/1502#discussion_r179540660
  
    --- Diff: core/sql/common/NAType.h ---
    @@ -562,7 +562,8 @@ class NAType : public NABasicObject
     				 short displayCaseSpecific = 0);
     
       short getMyTypeAsText(NAString * outputStr, 
    -			NABoolean addNullability = TRUE) const; // output
    +			NABoolean addNullability = TRUE,
    +                        NABoolean addCollation = TRUE) const; // output
    --- End diff --
    
    Looks like the "// output" comment migrated to the wrong place. Maybe it belongs on "NAString * outputStr"? Or we could just delete it.


---

[GitHub] trafodion pull request #1502: various fixes, details in pull request

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

    https://github.com/apache/trafodion/pull/1502#discussion_r179559224
  
    --- Diff: core/sql/executor/ExExeUtilGet.cpp ---
    @@ -5795,40 +5833,51 @@ short ExExeUtilHiveMDaccessTcb::work()
                 str_cpy(infoCol->colName, 
                         (hcd ? hcd->name_ : hpd->name_), 256, ' ');
     
    -            infoCol->fsDatatype = 
    -              getFSTypeFromHiveColType(hcd ? hcd->type_ : hpd->type_);
    -
    -	    if (infoCol->fsDatatype < 0)
    +            Lng32 fstype = -1;
    +            Lng32 length = -1;
    +            Lng32 precision = -1;
    +            Lng32 scale = -1;
    +            char sqlType[50];
    +            char displayType[100];
    +            char charset[50];
    +            retcode = 
    +              getTypeAttrsFromHiveColType(hcd ? hcd->type_ : hpd->type_,
    +                                          htd->getSDs()->isOrcFile(),
    +                                          fstype, length, precision, scale,
    +                                          sqlType, displayType, charset);
    +                 
    +	    if (retcode < 0)
     	      {
    +                // add a warning and continue.
     		char strP[300];
    -		sprintf(strP, "Datatype %s is not supported.", 
    -                        (hcd ? hcd->type_ : hpd->type_));
    -		*diags << DgSqlCode(-CLI_GET_METADATA_INFO_ERROR)
    +		sprintf(strP, "Datatype %s for column '%s' in table %s.%s.%s is not supported. This table will be ignored.", 
    --- End diff --
    
    In #1504 I am changing this type of populating error to use ExRaiseSqlError. This needs to be changed to use ExRaiseSqlError because diags could be null


---

[GitHub] trafodion pull request #1502: various fixes, details in pull request

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

    https://github.com/apache/trafodion/pull/1502#discussion_r179543357
  
    --- Diff: core/sql/executor/ExExeUtilGet.cpp ---
    @@ -5795,40 +5833,51 @@ short ExExeUtilHiveMDaccessTcb::work()
                 str_cpy(infoCol->colName, 
                         (hcd ? hcd->name_ : hpd->name_), 256, ' ');
     
    -            infoCol->fsDatatype = 
    -              getFSTypeFromHiveColType(hcd ? hcd->type_ : hpd->type_);
    -
    -	    if (infoCol->fsDatatype < 0)
    +            Lng32 fstype = -1;
    +            Lng32 length = -1;
    +            Lng32 precision = -1;
    +            Lng32 scale = -1;
    +            char sqlType[50];
    +            char displayType[100];
    +            char charset[50];
    +            retcode = 
    +              getTypeAttrsFromHiveColType(hcd ? hcd->type_ : hpd->type_,
    +                                          htd->getSDs()->isOrcFile(),
    +                                          fstype, length, precision, scale,
    +                                          sqlType, displayType, charset);
    +                 
    +	    if (retcode < 0)
     	      {
    +                // add a warning and continue.
     		char strP[300];
    -		sprintf(strP, "Datatype %s is not supported.", 
    -                        (hcd ? hcd->type_ : hpd->type_));
    -		*diags << DgSqlCode(-CLI_GET_METADATA_INFO_ERROR)
    +		sprintf(strP, "Datatype %s for column '%s' in table %s.%s.%s is not supported. This table will be ignored.", 
    +                        (hcd ? hcd->type_ : hpd->type_),
    +                        (hcd ? hcd->name_ : hpd->name_),
    +                        hiveCat_, hiveSch_, htd->tblName_);
    +		*diags << DgSqlCode(CLI_GET_METADATA_INFO_ERROR)
     		       << DgString0(strP);
    -		
    -		step_ = HANDLE_ERROR_;
    -		break;
    +
    +                step_ = ADVANCE_ROW_;
    --- End diff --
    
    getTypeAttrsFromHiveColType only initializes some of the output fields when it returns an error. Since we are converting the error to a warning now, it seems like it should initialize all of the output fields, or we should initialize them here.


---

[GitHub] trafodion pull request #1502: various fixes, details in pull request

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

    https://github.com/apache/trafodion/pull/1502#discussion_r179561304
  
    --- Diff: core/sql/executor/ExExeUtilGet.cpp ---
    @@ -5795,40 +5833,51 @@ short ExExeUtilHiveMDaccessTcb::work()
                 str_cpy(infoCol->colName, 
                         (hcd ? hcd->name_ : hpd->name_), 256, ' ');
     
    -            infoCol->fsDatatype = 
    -              getFSTypeFromHiveColType(hcd ? hcd->type_ : hpd->type_);
    -
    -	    if (infoCol->fsDatatype < 0)
    +            Lng32 fstype = -1;
    +            Lng32 length = -1;
    +            Lng32 precision = -1;
    +            Lng32 scale = -1;
    +            char sqlType[50];
    +            char displayType[100];
    +            char charset[50];
    +            retcode = 
    +              getTypeAttrsFromHiveColType(hcd ? hcd->type_ : hpd->type_,
    +                                          htd->getSDs()->isOrcFile(),
    +                                          fstype, length, precision, scale,
    +                                          sqlType, displayType, charset);
    +                 
    +	    if (retcode < 0)
     	      {
    +                // add a warning and continue.
     		char strP[300];
    -		sprintf(strP, "Datatype %s is not supported.", 
    -                        (hcd ? hcd->type_ : hpd->type_));
    -		*diags << DgSqlCode(-CLI_GET_METADATA_INFO_ERROR)
    +		sprintf(strP, "Datatype %s for column '%s' in table %s.%s.%s is not supported. This table will be ignored.", 
    +                        (hcd ? hcd->type_ : hpd->type_),
    +                        (hcd ? hcd->name_ : hpd->name_),
    +                        hiveCat_, hiveSch_, htd->tblName_);
    +		*diags << DgSqlCode(CLI_GET_METADATA_INFO_ERROR)
     		       << DgString0(strP);
    -		
    -		step_ = HANDLE_ERROR_;
    -		break;
    +
    +                step_ = ADVANCE_ROW_;
    --- End diff --
    
    Need to change it into ExRaiseSqlError because diags could be null


---

[GitHub] trafodion pull request #1502: various fixes, details in pull request

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

    https://github.com/apache/trafodion/pull/1502#discussion_r179550540
  
    --- Diff: core/sql/executor/ExExeUtilGet.cpp ---
    @@ -5795,40 +5833,51 @@ short ExExeUtilHiveMDaccessTcb::work()
                 str_cpy(infoCol->colName, 
                         (hcd ? hcd->name_ : hpd->name_), 256, ' ');
     
    -            infoCol->fsDatatype = 
    -              getFSTypeFromHiveColType(hcd ? hcd->type_ : hpd->type_);
    -
    -	    if (infoCol->fsDatatype < 0)
    +            Lng32 fstype = -1;
    +            Lng32 length = -1;
    +            Lng32 precision = -1;
    +            Lng32 scale = -1;
    +            char sqlType[50];
    +            char displayType[100];
    +            char charset[50];
    +            retcode = 
    +              getTypeAttrsFromHiveColType(hcd ? hcd->type_ : hpd->type_,
    +                                          htd->getSDs()->isOrcFile(),
    +                                          fstype, length, precision, scale,
    +                                          sqlType, displayType, charset);
    +                 
    +	    if (retcode < 0)
     	      {
    +                // add a warning and continue.
     		char strP[300];
    -		sprintf(strP, "Datatype %s is not supported.", 
    -                        (hcd ? hcd->type_ : hpd->type_));
    -		*diags << DgSqlCode(-CLI_GET_METADATA_INFO_ERROR)
    +		sprintf(strP, "Datatype %s for column '%s' in table %s.%s.%s is not supported. This table will be ignored.", 
    --- End diff --
    
    Why not use snprintf(strP, sizeof(strP), ...)?


---

[GitHub] trafodion pull request #1502: various fixes, details in pull request

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

    https://github.com/apache/trafodion/pull/1502#discussion_r179617033
  
    --- Diff: core/sql/common/NAType.h ---
    @@ -562,7 +562,8 @@ class NAType : public NABasicObject
     				 short displayCaseSpecific = 0);
     
       short getMyTypeAsText(NAString * outputStr, 
    -			NABoolean addNullability = TRUE) const; // output
    +			NABoolean addNullability = TRUE,
    +                        NABoolean addCollation = TRUE) const; // output
    --- End diff --
    
    thanks, will fix that.


---

[GitHub] trafodion pull request #1502: various fixes, details in pull request

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

    https://github.com/apache/trafodion/pull/1502


---

[GitHub] trafodion pull request #1502: various fixes, details in pull request

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

    https://github.com/apache/trafodion/pull/1502#discussion_r179617106
  
    --- Diff: core/sql/executor/ExExeUtilGet.cpp ---
    @@ -5795,40 +5833,51 @@ short ExExeUtilHiveMDaccessTcb::work()
                 str_cpy(infoCol->colName, 
                         (hcd ? hcd->name_ : hpd->name_), 256, ' ');
     
    -            infoCol->fsDatatype = 
    -              getFSTypeFromHiveColType(hcd ? hcd->type_ : hpd->type_);
    -
    -	    if (infoCol->fsDatatype < 0)
    +            Lng32 fstype = -1;
    +            Lng32 length = -1;
    +            Lng32 precision = -1;
    +            Lng32 scale = -1;
    +            char sqlType[50];
    +            char displayType[100];
    --- End diff --
    
    will fix that by creating enums for these lengths.


---

[GitHub] trafodion pull request #1502: various fixes, details in pull request

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

    https://github.com/apache/trafodion/pull/1502#discussion_r179542589
  
    --- Diff: core/sql/executor/ExExeUtilGet.cpp ---
    @@ -5795,40 +5833,51 @@ short ExExeUtilHiveMDaccessTcb::work()
                 str_cpy(infoCol->colName, 
                         (hcd ? hcd->name_ : hpd->name_), 256, ' ');
     
    -            infoCol->fsDatatype = 
    -              getFSTypeFromHiveColType(hcd ? hcd->type_ : hpd->type_);
    -
    -	    if (infoCol->fsDatatype < 0)
    +            Lng32 fstype = -1;
    +            Lng32 length = -1;
    +            Lng32 precision = -1;
    +            Lng32 scale = -1;
    +            char sqlType[50];
    +            char displayType[100];
    +            char charset[50];
    +            retcode = 
    +              getTypeAttrsFromHiveColType(hcd ? hcd->type_ : hpd->type_,
    +                                          htd->getSDs()->isOrcFile(),
    +                                          fstype, length, precision, scale,
    +                                          sqlType, displayType, charset);
    +                 
    +	    if (retcode < 0)
     	      {
    +                // add a warning and continue.
     		char strP[300];
    -		sprintf(strP, "Datatype %s is not supported.", 
    -                        (hcd ? hcd->type_ : hpd->type_));
    -		*diags << DgSqlCode(-CLI_GET_METADATA_INFO_ERROR)
    +		sprintf(strP, "Datatype %s for column '%s' in table %s.%s.%s is not supported. This table will be ignored.", 
    --- End diff --
    
    300 is not long enough. The column name, schema name and table name can each be up to 256 characters.


---