You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Zihao Ye (Code Review)" <ge...@cloudera.org> on 2024/08/22 07:33:44 UTC

[Impala-ASF-CR] IMPALA-12957: Support reading Inf and NaN from JSON

Zihao Ye has uploaded this change for review. ( http://gerrit.cloudera.org:8080/21701


Change subject: IMPALA-12957: Support reading Inf and NaN from JSON
......................................................................

IMPALA-12957: Support reading Inf and NaN from JSON

Despite the fact that special values such as Inf and NaN are not
supported in standard JSON (they are considered invalid values),
rapidjson does support them. However, it requires the parsing flag
'kParseNanAndInfFlag' to be enabled.
This patch enables the flag to allows JsonParser to parse special
numbers like Inf and NaN. Corresponding modifications have also been
made to JsonSkipper to maintain consistent behavior when zero slots
scans.

TODO: Due to a bug in rapidjson when parsing NaN and Inf with fractional
or exponent parts (https://github.com/Tencent/rapidjson/issues/2299),
JsonSkipper has "inherited" this bug to maintain consistent behavior.
After upgrading rapidjson to resolve this issue, JsonSkipper's code
should also be modified accordingly.

Testing:
  - Added and passed relevant test cases (BE, E2E).

Change-Id: I05ee7c7c7fb7e78fff9570f659ce2d13c94a4e10
---
M be/src/exec/json/json-parser-test.cc
M be/src/exec/json/json-parser.cc
M be/src/exec/json/json-parser.h
M testdata/data/json_test/overflow.json
M testdata/workloads/functional-query/queries/QueryTest/overflow_json.test
5 files changed, 43 insertions(+), 18 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/01/21701/1
-- 
To view, visit http://gerrit.cloudera.org:8080/21701
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I05ee7c7c7fb7e78fff9570f659ce2d13c94a4e10
Gerrit-Change-Number: 21701
Gerrit-PatchSet: 1
Gerrit-Owner: Zihao Ye <ey...@163.com>

[Impala-ASF-CR] IMPALA-12957: Support reading Inf and NaN from JSON

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21701 )

Change subject: IMPALA-12957: Support reading Inf and NaN from JSON
......................................................................


Patch Set 4:

Build started: https://jenkins.impala.io/job/gerrit-docs-submit/390/ DRY_RUN=false


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I05ee7c7c7fb7e78fff9570f659ce2d13c94a4e10
Gerrit-Change-Number: 21701
Gerrit-PatchSet: 4
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Comment-Date: Mon, 11 Nov 2024 00:02:24 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12957: Support reading Inf and NaN from JSON

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21701 )

Change subject: IMPALA-12957: Support reading Inf and NaN from JSON
......................................................................


Patch Set 2:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/17320/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I05ee7c7c7fb7e78fff9570f659ce2d13c94a4e10
Gerrit-Change-Number: 21701
Gerrit-PatchSet: 2
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Comment-Date: Thu, 31 Oct 2024 07:13:17 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12957: Support reading Inf and NaN from JSON

Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/21701 )

Change subject: IMPALA-12957: Support reading Inf and NaN from JSON
......................................................................


Patch Set 3: Code-Review+1

(1 comment)

http://gerrit.cloudera.org:8080/#/c/21701/3/be/src/exec/json/json-parser-test.cc
File be/src/exec/json/json-parser-test.cc:

http://gerrit.cloudera.org:8080/#/c/21701/3/be/src/exec/json/json-parser-test.cc@191
PS3, Line 191:   TestSkip("Inf", TYPE_NUMBER);
             :   TestSkip("-Infinity", TYPE_NUMBER);
             :   TestSkip("NaN", TYPE_NUMBER);
nit: Is case matter or not (nan, inf, infinity)?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I05ee7c7c7fb7e78fff9570f659ce2d13c94a4e10
Gerrit-Change-Number: 21701
Gerrit-PatchSet: 3
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Comment-Date: Tue, 05 Nov 2024 15:46:35 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12957: Support reading Inf and NaN from JSON

Posted by "Zihao Ye (Code Review)" <ge...@cloudera.org>.
Zihao Ye has posted comments on this change. ( http://gerrit.cloudera.org:8080/21701 )

Change subject: IMPALA-12957: Support reading Inf and NaN from JSON
......................................................................


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/21701/3/be/src/exec/json/json-parser-test.cc
File be/src/exec/json/json-parser-test.cc:

http://gerrit.cloudera.org:8080/#/c/21701/3/be/src/exec/json/json-parser-test.cc@191
PS3, Line 191:   TestSkip("Inf", TYPE_NUMBER);
             :   TestSkip("-Infinity", TYPE_NUMBER);
             :   TestSkip("NaN", TYPE_NUMBER);
> nit: Is case matter or not (nan, inf, infinity)?
Yes, rapidjson only supports NaN, Inf, and Infinity. Other case variations are considered invalid values. See https://github.com/Tencent/rapidjson/blob/858451e5b7d1c56cf8f6d58f88cf958351837e53/include/rapidjson/reader.h#L1524



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I05ee7c7c7fb7e78fff9570f659ce2d13c94a4e10
Gerrit-Change-Number: 21701
Gerrit-PatchSet: 3
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Comment-Date: Thu, 07 Nov 2024 02:11:00 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12957: Support reading Inf and NaN from JSON

Posted by "Zihao Ye (Code Review)" <ge...@cloudera.org>.
Zihao Ye has posted comments on this change. ( http://gerrit.cloudera.org:8080/21701 )

Change subject: IMPALA-12957: Support reading Inf and NaN from JSON
......................................................................


Patch Set 1:

(1 comment)

Thank you for the review!

http://gerrit.cloudera.org:8080/#/c/21701/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/21701/1//COMMIT_MSG@19
PS1, Line 19: https://github.com/Tencent/rapidjson/issues/2299
> I see this is resolved. Do you know if rapidjson will have a new release to
It seems that there are no plans for a new release of rapidjson in the near future, so we might need to handle this ourselves. I have submitted a commit to native-toolchain to include the fix:
https://gerrit.cloudera.org/#/c/21980/
Once it is merged, we can revisit this issue.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I05ee7c7c7fb7e78fff9570f659ce2d13c94a4e10
Gerrit-Change-Number: 21701
Gerrit-PatchSet: 1
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Comment-Date: Mon, 28 Oct 2024 08:10:30 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12957: Support reading Inf and NaN from JSON

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21701 )

Change subject: IMPALA-12957: Support reading Inf and NaN from JSON
......................................................................


Patch Set 4: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I05ee7c7c7fb7e78fff9570f659ce2d13c94a4e10
Gerrit-Change-Number: 21701
Gerrit-PatchSet: 4
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Comment-Date: Mon, 11 Nov 2024 00:02:23 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12957: Support reading Inf and NaN from JSON

Posted by "Zihao Ye (Code Review)" <ge...@cloudera.org>.
Zihao Ye has posted comments on this change. ( http://gerrit.cloudera.org:8080/21701 )

Change subject: IMPALA-12957: Support reading Inf and NaN from JSON
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/21701/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/21701/1//COMMIT_MSG@19
PS1, Line 19: d relevant test cases (BE, E2E).
> I see this is resolved. Do you know if rapidjson will have a new release to
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I05ee7c7c7fb7e78fff9570f659ce2d13c94a4e10
Gerrit-Change-Number: 21701
Gerrit-PatchSet: 2
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Comment-Date: Thu, 31 Oct 2024 07:25:35 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12957: Support reading Inf and NaN from JSON

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21701 )

Change subject: IMPALA-12957: Support reading Inf and NaN from JSON
......................................................................


Patch Set 3:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/17366/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I05ee7c7c7fb7e78fff9570f659ce2d13c94a4e10
Gerrit-Change-Number: 21701
Gerrit-PatchSet: 3
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Comment-Date: Tue, 05 Nov 2024 07:46:02 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12957: Support reading Inf and NaN from JSON

Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/21701 )

Change subject: IMPALA-12957: Support reading Inf and NaN from JSON
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/21701/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/21701/1//COMMIT_MSG@19
PS1, Line 19: https://github.com/Tencent/rapidjson/issues/2299
I see this is resolved. Do you know if rapidjson will have a new release to contain this? We are currently using rapidjson-1.1.0 which is released in 2016 (but still the latest version)..

If no new releases, we can bump our rapidjson version in native-toolchain to add that fix. Just like what we did for the ORC lib:
https://github.com/cloudera/native-toolchain/tree/810d0f47572d9ac4036c34eb2ad6342d51d510cd/source/orc/orc-1.7.9-patches



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I05ee7c7c7fb7e78fff9570f659ce2d13c94a4e10
Gerrit-Change-Number: 21701
Gerrit-PatchSet: 1
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Tue, 15 Oct 2024 02:16:47 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12957: Support reading Inf and NaN from JSON

Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/21701 )

Change subject: IMPALA-12957: Support reading Inf and NaN from JSON
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/21701/2/be/src/exec/json/json-parser.cc
File be/src/exec/json/json-parser.cc:

http://gerrit.cloudera.org:8080/#/c/21701/2/be/src/exec/json/json-parser.cc@a340
PS2, Line 340: 
             : 
> nit: can we keep some comments about Inf and NaN, and mention we have enabl
Agree, L340 and L341 can stay.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I05ee7c7c7fb7e78fff9570f659ce2d13c94a4e10
Gerrit-Change-Number: 21701
Gerrit-PatchSet: 2
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Comment-Date: Mon, 04 Nov 2024 18:01:53 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12957: Support reading Inf and NaN from JSON

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21701 )

Change subject: IMPALA-12957: Support reading Inf and NaN from JSON
......................................................................


Patch Set 4: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I05ee7c7c7fb7e78fff9570f659ce2d13c94a4e10
Gerrit-Change-Number: 21701
Gerrit-PatchSet: 4
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Comment-Date: Mon, 11 Nov 2024 00:09:08 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12957: Support reading Inf and NaN from JSON

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21701 )

Change subject: IMPALA-12957: Support reading Inf and NaN from JSON
......................................................................


Patch Set 1:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/16758/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I05ee7c7c7fb7e78fff9570f659ce2d13c94a4e10
Gerrit-Change-Number: 21701
Gerrit-PatchSet: 1
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 22 Aug 2024 08:12:37 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12957: Support reading Inf and NaN from JSON

Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/21701 )

Change subject: IMPALA-12957: Support reading Inf and NaN from JSON
......................................................................


Patch Set 2: Code-Review+1

(1 comment)

http://gerrit.cloudera.org:8080/#/c/21701/2/be/src/exec/json/json-parser.cc
File be/src/exec/json/json-parser.cc:

http://gerrit.cloudera.org:8080/#/c/21701/2/be/src/exec/json/json-parser.cc@a340
PS2, Line 340: 
             : 
nit: can we keep some comments about Inf and NaN, and mention we have enabled kParseNanAndInfFlag?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I05ee7c7c7fb7e78fff9570f659ce2d13c94a4e10
Gerrit-Change-Number: 21701
Gerrit-PatchSet: 2
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Comment-Date: Mon, 04 Nov 2024 01:33:47 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12957: Support reading Inf and NaN from JSON

Posted by "Zihao Ye (Code Review)" <ge...@cloudera.org>.
Zihao Ye has posted comments on this change. ( http://gerrit.cloudera.org:8080/21701 )

Change subject: IMPALA-12957: Support reading Inf and NaN from JSON
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/21701/2/be/src/exec/json/json-parser.cc
File be/src/exec/json/json-parser.cc:

http://gerrit.cloudera.org:8080/#/c/21701/2/be/src/exec/json/json-parser.cc@a340
PS2, Line 340: 
             : 
> Agree, L340 and L341 can stay.
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I05ee7c7c7fb7e78fff9570f659ce2d13c94a4e10
Gerrit-Change-Number: 21701
Gerrit-PatchSet: 2
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Comment-Date: Tue, 05 Nov 2024 07:22:14 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12957: Support reading Inf and NaN from JSON

Posted by "Zihao Ye (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Riza Suminto, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/21701

to look at the new patch set (#3).

Change subject: IMPALA-12957: Support reading Inf and NaN from JSON
......................................................................

IMPALA-12957: Support reading Inf and NaN from JSON

Despite the fact that special values such as Inf and NaN are not
supported in standard JSON (they are considered invalid values),
rapidjson does support them. However, it requires the parsing flag
'kParseNanAndInfFlag' to be enabled.
This patch enables the flag to allows JsonParser to parse special
numbers like Inf and NaN. Corresponding modifications have also been
made to JsonSkipper to maintain consistent behavior when zero slots
scans.
Additionally, due to a minor issue in rapidjson v1.1.0 when parsing Inf
and NaN, this patch also updates the rapidjson version to include the
corresponding fix (see https://gerrit.cloudera.org/#/c/21980/).

Testing:
  - Added and passed relevant test cases (BE, E2E).

Change-Id: I05ee7c7c7fb7e78fff9570f659ce2d13c94a4e10
---
M be/src/exec/json/json-parser-test.cc
M be/src/exec/json/json-parser.cc
M be/src/exec/json/json-parser.h
M bin/impala-config.sh
M testdata/data/json_test/malformed.json
M testdata/data/json_test/overflow.json
M testdata/workloads/functional-query/queries/QueryTest/malformed_json.test
M testdata/workloads/functional-query/queries/QueryTest/overflow_json.test
8 files changed, 50 insertions(+), 21 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/01/21701/3
-- 
To view, visit http://gerrit.cloudera.org:8080/21701
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I05ee7c7c7fb7e78fff9570f659ce2d13c94a4e10
Gerrit-Change-Number: 21701
Gerrit-PatchSet: 3
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>

[Impala-ASF-CR] IMPALA-12957: Support reading Inf and NaN from JSON

Posted by "Zihao Ye (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Riza Suminto, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/21701

to look at the new patch set (#2).

Change subject: IMPALA-12957: Support reading Inf and NaN from JSON
......................................................................

IMPALA-12957: Support reading Inf and NaN from JSON

Despite the fact that special values such as Inf and NaN are not
supported in standard JSON (they are considered invalid values),
rapidjson does support them. However, it requires the parsing flag
'kParseNanAndInfFlag' to be enabled.
This patch enables the flag to allows JsonParser to parse special
numbers like Inf and NaN. Corresponding modifications have also been
made to JsonSkipper to maintain consistent behavior when zero slots
scans.

Testing:
  - Added and passed relevant test cases (BE, E2E).

Change-Id: I05ee7c7c7fb7e78fff9570f659ce2d13c94a4e10
---
M be/src/exec/json/json-parser-test.cc
M be/src/exec/json/json-parser.cc
M be/src/exec/json/json-parser.h
M bin/impala-config.sh
M testdata/data/json_test/malformed.json
M testdata/data/json_test/overflow.json
M testdata/workloads/functional-query/queries/QueryTest/malformed_json.test
M testdata/workloads/functional-query/queries/QueryTest/overflow_json.test
8 files changed, 47 insertions(+), 23 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/01/21701/2
-- 
To view, visit http://gerrit.cloudera.org:8080/21701
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I05ee7c7c7fb7e78fff9570f659ce2d13c94a4e10
Gerrit-Change-Number: 21701
Gerrit-PatchSet: 2
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>

[Impala-ASF-CR] IMPALA-12957: Support reading Inf and NaN from JSON

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/21701 )

Change subject: IMPALA-12957: Support reading Inf and NaN from JSON
......................................................................

IMPALA-12957: Support reading Inf and NaN from JSON

Despite the fact that special values such as Inf and NaN are not
supported in standard JSON (they are considered invalid values),
rapidjson does support them. However, it requires the parsing flag
'kParseNanAndInfFlag' to be enabled.
This patch enables the flag to allows JsonParser to parse special
numbers like Inf and NaN. Corresponding modifications have also been
made to JsonSkipper to maintain consistent behavior when zero slots
scans.
Additionally, due to a minor issue in rapidjson v1.1.0 when parsing Inf
and NaN, this patch also updates the rapidjson version to include the
corresponding fix (see https://gerrit.cloudera.org/#/c/21980/).

Testing:
  - Added and passed relevant test cases (BE, E2E).

Change-Id: I05ee7c7c7fb7e78fff9570f659ce2d13c94a4e10
Reviewed-on: http://gerrit.cloudera.org:8080/21701
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/src/exec/json/json-parser-test.cc
M be/src/exec/json/json-parser.cc
M be/src/exec/json/json-parser.h
M bin/impala-config.sh
M testdata/data/json_test/malformed.json
M testdata/data/json_test/overflow.json
M testdata/workloads/functional-query/queries/QueryTest/malformed_json.test
M testdata/workloads/functional-query/queries/QueryTest/overflow_json.test
8 files changed, 50 insertions(+), 21 deletions(-)

Approvals:
  Impala Public Jenkins: Looks good to me, approved; Verified

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I05ee7c7c7fb7e78fff9570f659ce2d13c94a4e10
Gerrit-Change-Number: 21701
Gerrit-PatchSet: 5
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>

[Impala-ASF-CR] IMPALA-12957: Support reading Inf and NaN from JSON

Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/21701 )

Change subject: IMPALA-12957: Support reading Inf and NaN from JSON
......................................................................


Patch Set 3: Code-Review+2

Carry Riza's +1.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I05ee7c7c7fb7e78fff9570f659ce2d13c94a4e10
Gerrit-Change-Number: 21701
Gerrit-PatchSet: 3
Gerrit-Owner: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Comment-Date: Mon, 11 Nov 2024 00:01:55 +0000
Gerrit-HasComments: No