Yifan Zhang has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/23475 )

Change subject: IMPALA-14467: Support quoted keys with dots in get_json_object 
path strings
......................................................................


Patch Set 4:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/23475/2/be/src/exprs/string-functions.cc
File be/src/exprs/string-functions.cc:

http://gerrit.cloudera.org:8080/#/c/23475/2/be/src/exprs/string-functions.cc@363
PS2, Line 363:
> Should we also support single quotes?
I think supporting double quotes is enough.
Because JSON string should always be quoted with double quotes, and MySQL also 
only supports double quotes.


http://gerrit.cloudera.org:8080/#/c/23475/2/be/src/exprs/string-functions.cc@366
PS2, Line 366:
> nit: please align these
Done


http://gerrit.cloudera.org:8080/#/c/23475/2/be/src/util/string-util.cc
File be/src/util/string-util.cc:

http://gerrit.cloudera.org:8080/#/c/23475/2/be/src/util/string-util.cc@118
PS2, Line 118: else {
> Can you please explain about the function in a comment, what it returns and
Comment was added in .h file.


http://gerrit.cloudera.org:8080/#/c/23475/2/testdata/workloads/functional-query/queries/QueryTest/exprs.test
File testdata/workloads/functional-query/queries/QueryTest/exprs.test:

http://gerrit.cloudera.org:8080/#/c/23475/2/testdata/workloads/functional-query/queries/QueryTest/exprs.test@3144
PS2, Line 3144: "a.com"
> Can we add some negative tests like using "a."com where not all parts of th
Added some negative tests.
Yes, dot and other special characters can be quoted as a key.


http://gerrit.cloudera.org:8080/#/c/23475/2/testdata/workloads/functional-query/queries/QueryTest/exprs.test@3150
PS2, Line 3150: ---- QUERY
> Please add more tests and try to look at different combinations/cases.
Done


http://gerrit.cloudera.org:8080/#/c/23475/2/testdata/workloads/functional-query/queries/QueryTest/exprs.test@3149
PS2, Line 3149:
              : ---- QUERY
> Add a test of an empty string within "", do we want to treat that as a vali
Yes, empty string is also a valid key.



--
To view, visit http://gerrit.cloudera.org:8080/23475
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia3618d9e029501f9ac0e23190feb8263968b1b9f
Gerrit-Change-Number: 23475
Gerrit-PatchSet: 4
Gerrit-Owner: Yifan Zhang <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Pranav Lodha <[email protected]>
Gerrit-Reviewer: Quanlong Huang <[email protected]>
Gerrit-Reviewer: Yifan Zhang <[email protected]>
Gerrit-Comment-Date: Thu, 13 Nov 2025 11:20:22 +0000
Gerrit-HasComments: Yes

Reply via email to