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 2018/02/05 08:35:21 UTC
[GitHub] trafodion pull request #1439: [TRAFODION-2772] - retrieve a value from Json ...
GitHub user andyyangcn opened a pull request:
https://github.com/apache/trafodion/pull/1439
[TRAFODION-2772] - retrieve a value from Json string got an error
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/andyyangcn/incubator-trafodion fix3
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/trafodion/pull/1439.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 #1439
----
commit b2984e1ff96423d82ebd86d4e9a7e2beb6fb40d1
Author: Andy Yang <yo...@...>
Date: 2018-02-05T08:32:19Z
[TRAFODION-2772] - retrieve a value from Json string got an error
----
---
[GitHub] trafodion pull request #1439: [TRAFODION-2772] - retrieve a value from Json ...
Posted by andyyangcn <gi...@git.apache.org>.
Github user andyyangcn commented on a diff in the pull request:
https://github.com/apache/trafodion/pull/1439#discussion_r166245892
--- Diff: core/sql/exp/exp_function.cpp ---
@@ -6503,8 +6503,15 @@ 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];
--- End diff --
Since the value of len1 can only be determined when executing the query, we cannot define jsonStr as an Array.
Will release the memory manually.
---
[GitHub] trafodion pull request #1439: [TRAFODION-2772] - retrieve a value from Json ...
Posted by DaveBirdsall <gi...@git.apache.org>.
Github user DaveBirdsall commented on a diff in the pull request:
https://github.com/apache/trafodion/pull/1439#discussion_r166060055
--- Diff: core/sql/exp/exp_function.cpp ---
@@ -6503,8 +6503,15 @@ 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];
--- End diff --
I'm wondering if we need to delete jsonStr and jsonAttr after the json_extract_path_text call to avoid unnecessary heap pressure. Though if json_extract_path_text itself does new's on the same heap, we'd get heap fragmentation.
Another approach would be to allocate these on the stack instead, avoiding both concerns:
char jsonStr[len1+1];
char jsonAttr[len2+1];
...
---
[GitHub] trafodion pull request #1439: [TRAFODION-2772] - retrieve a value from Json ...
Posted by DaveBirdsall <gi...@git.apache.org>.
Github user DaveBirdsall commented on a diff in the pull request:
https://github.com/apache/trafodion/pull/1439#discussion_r166401395
--- Diff: core/sql/exp/exp_function.cpp ---
@@ -6503,8 +6503,15 @@ 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];
--- End diff --
Here's an example. Compile this program:
// Demonstrate dynamically-sized arrays on C++ stack
#include <iostream>
using namespace std;
int main(int argc, char * argv[])
{
for (size_t i = 5; i < 10; i++)
{
char temp[i+1];
cout << "sizeof(temp) = " << sizeof(temp) << endl;
}
return 0;
}
When you run it, you'll get this output:
[birdsall@edev08 dynChar]$ ./dynChar.exe
sizeof(temp) = 6
sizeof(temp) = 7
sizeof(temp) = 8
sizeof(temp) = 9
sizeof(temp) = 10
[birdsall@edev08 dynChar]$
---
[GitHub] trafodion pull request #1439: [TRAFODION-2772] - retrieve a value from Json ...
Posted by DaveBirdsall <gi...@git.apache.org>.
Github user DaveBirdsall commented on a diff in the pull request:
https://github.com/apache/trafodion/pull/1439#discussion_r166398401
--- Diff: core/sql/exp/exp_function.cpp ---
@@ -6503,8 +6503,15 @@ 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];
--- End diff --
But we can! C++ allows dynamically-sized arrays on the stack. Try it!
---
[GitHub] trafodion pull request #1439: [TRAFODION-2772] - retrieve a value from Json ...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/trafodion/pull/1439
---