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

[GitHub] incubator-trafodion pull request #1264: [TRAFODION-2772] - retrieve a value ...

GitHub user andyyangcn opened a pull request:

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

    [TRAFODION-2772] - retrieve a value from Json string got an error: Json value is invalid

    [TRAFODION-2772] - retrieve a value from Json string got an error: Json value is invalid

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

    $ git pull https://github.com/andyyangcn/incubator-trafodion fix2

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

    https://github.com/apache/incubator-trafodion/pull/1264.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 #1264
    
----
commit 5e334d6b321859faaf00cee29c94b1fcbb9f8e03
Author: Andy Yang <yo...@esgyn.cn>
Date:   2017-10-12T03:49:21Z

    [TRAFODION-2772] - retrieve a value from Json string got an error: Json value is invalid

commit 5d9938015bbe86053ee034bc93033e7d40f05208
Author: Andy Yang <yo...@esgyn.cn>
Date:   2017-10-12T03:59:59Z

    [TRAFODION-2772] - retrieve a value from Json string got an error: Json value is invalid - add NULL check

----


---

[GitHub] incubator-trafodion pull request #1264: [TRAFODION-2772] - retrieve a value ...

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/1264#discussion_r146280638
  
    --- Diff: core/sql/exp/exp_function.cpp ---
    @@ -6503,8 +6503,19 @@ ex_expr::exp_return_type ex_function_json_object_field_text::eval(char *op_data[
             Int32 prec2 = ((SimpleType *)getOperand(2))->getPrecision();
             len2 = Attributes::trimFillerSpaces( op_data[2], prec2, len2, cs );
         }
    +
         char *rltStr = NULL;
    -    JsonReturnType ret = json_extract_path_text(&rltStr, op_data[1], 1, op_data[2]);
    +    char *jsonStr = new(heap) char[len1+1];
    +    char *jsonAttr = new(heap) char[len2+1];
    +    if (jsonStr == NULL || jsonAttr == NULL)
    +    {
    +        return ex_expr::EXPR_ERROR;
    +    }
    +    memset(jsonStr, 0, len1+1);
    +    memset(jsonAttr, 0, len2+1);
    --- End diff --
    
    Consider setting the null character after strncpy instead


---

[GitHub] incubator-trafodion pull request #1264: [TRAFODION-2772] - retrieve a value ...

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/1264#discussion_r144664852
  
    --- Diff: core/sql/exp/exp_function.cpp ---
    @@ -6503,8 +6503,19 @@ ex_expr::exp_return_type ex_function_json_object_field_text::eval(char *op_data[
             Int32 prec2 = ((SimpleType *)getOperand(2))->getPrecision();
             len2 = Attributes::trimFillerSpaces( op_data[2], prec2, len2, cs );
         }
    +
         char *rltStr = NULL;
    -    JsonReturnType ret = json_extract_path_text(&rltStr, op_data[1], 1, op_data[2]);
    +    char *jsonStr = new(heap) char[len1+1];
    +    char *jsonAttr = new(heap) char[len2+1];
    +    if (jsonStr == NULL || jsonAttr == NULL)
    +    {
    +        return ex_expr::EXPR_ERROR;
    --- End diff --
    
    We should add something to the diagnostic area when making this return. Though I expect that might fail with a memory error as well. Might be worthwhile to see how other functions behave when memory allocations fail.


---