You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Yifan Zhang (Code Review)" <ge...@cloudera.org> on 2024/05/17 16:10:26 UTC

[Impala-ASF-CR] IMPALA-12562: Cast double and float to string with exact presicion

Yifan Zhang has uploaded this change for review. ( http://gerrit.cloudera.org:8080/21441


Change subject: IMPALA-12562: Cast double and float to string with exact presicion
......................................................................

IMPALA-12562: Cast double and float to string with exact presicion

The builtin functions casttostring(DOUBLE) and casttostring(FLOAT)
printed more digits when converting double and float values to
string values. This patch fixes this by switching to use the existing
methods SimpleDtoa and SimpleFtoa, which are simple and fast
implementations to print necessary digits.

Testing:
  - Add end-to-end tests to query_test/test_exprs.py

Change-Id: Icd79c55dd57dc0fa13e4ec11c2284ef2800e8b1a
---
M be/src/exprs/cast-functions-ir.cc
M testdata/workloads/functional-query/queries/QueryTest/exprs.test
2 files changed, 43 insertions(+), 25 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Icd79c55dd57dc0fa13e4ec11c2284ef2800e8b1a
Gerrit-Change-Number: 21441
Gerrit-PatchSet: 1
Gerrit-Owner: Yifan Zhang <ch...@163.com>

[Impala-ASF-CR] IMPALA-12562: Cast double and float to string with exact presicion

Posted by "Yifan Zhang (Code Review)" <ge...@cloudera.org>.
Hello Daniel Becker, Gabor Kaszab, Csaba Ringhofer, Michael Smith, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-12562: Cast double and float to string with exact presicion
......................................................................

IMPALA-12562: Cast double and float to string with exact presicion

The builtin functions casttostring(DOUBLE) and casttostring(FLOAT)
printed more digits when converting double and float values to
string values. This patch fixes this by switching to use the existing
methods DoubleToBuffer and FloatToBuffer, which are simple and fast
implementations to print necessary digits.

Testing:
  - Add end-to-end tests to verify the fixes
  - Add benchmarks for modified functions

Change-Id: Icd79c55dd57dc0fa13e4ec11c2284ef2800e8b1a
---
M be/src/benchmarks/expr-benchmark.cc
M be/src/exprs/cast-functions-ir.cc
M testdata/workloads/functional-query/queries/QueryTest/exprs.test
3 files changed, 135 insertions(+), 47 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Icd79c55dd57dc0fa13e4ec11c2284ef2800e8b1a
Gerrit-Change-Number: 21441
Gerrit-PatchSet: 3
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>

[Impala-ASF-CR] IMPALA-12562: Cast double and float to string with exact presicion

Posted by "Yifan Zhang (Code Review)" <ge...@cloudera.org>.
Hello Daniel Becker, Gabor Kaszab, Csaba Ringhofer, Michael Smith, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-12562: Cast double and float to string with exact presicion
......................................................................

IMPALA-12562: Cast double and float to string with exact presicion

The builtin functions casttostring(DOUBLE) and casttostring(FLOAT)
printed more digits when converting double and float values to
string values. This patch fixes this by switching to use the existing
methods SimpleDtoa and SimpleFtoa, which are simple and fast
implementations to print necessary digits.

Testing:
  - Add end-to-end tests to query_test/test_exprs.py

Change-Id: Icd79c55dd57dc0fa13e4ec11c2284ef2800e8b1a
---
M be/src/exprs/cast-functions-ir.cc
M testdata/workloads/functional-query/queries/QueryTest/exprs.test
2 files changed, 104 insertions(+), 29 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Icd79c55dd57dc0fa13e4ec11c2284ef2800e8b1a
Gerrit-Change-Number: 21441
Gerrit-PatchSet: 2
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>

[Impala-ASF-CR] IMPALA-12562: Cast double and float to string with exact presicion

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

Change subject: IMPALA-12562: Cast double and float to string with exact presicion
......................................................................


Patch Set 2:

(3 comments)

Looks great! Generally I have very good experience with gutil/numbers both in terms of speed and correctness

http://gerrit.cloudera.org:8080/#/c/21441/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/21441/2//COMMIT_MSG@12
PS2, Line 12: fast
Can you provide some benchmarks? I expect it to be an improvement, but it would be nice to verify this.


http://gerrit.cloudera.org:8080/#/c/21441/2/be/src/exprs/cast-functions-ir.cc
File be/src/exprs/cast-functions-ir.cc:

http://gerrit.cloudera.org:8080/#/c/21441/2/be/src/exprs/cast-functions-ir.cc@344
PS2, Line 344:     if (UNLIKELY(sv.is_null)) {                                 \
At this point no allocation happened yet so this check is not useful.


http://gerrit.cloudera.org:8080/#/c/21441/2/be/src/exprs/cast-functions-ir.cc@355
PS2, Line 355: SimpleFtoa
I would prefer using DoubleToBuffer / FloatToBuffer as it avoids the extra heap allocation needed for the std::string.
https://github.com/apache/impala/blob/bcff4df6194b2f192d937bb9c031721feccb69df/be/src/gutil/strings/numbers.h#L451



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icd79c55dd57dc0fa13e4ec11c2284ef2800e8b1a
Gerrit-Change-Number: 21441
Gerrit-PatchSet: 2
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Comment-Date: Sat, 18 May 2024 07:35:19 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12562: Cast double and float to string with exact presicion

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

Change subject: IMPALA-12562: Cast double and float to string with exact presicion
......................................................................


Patch Set 4: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icd79c55dd57dc0fa13e4ec11c2284ef2800e8b1a
Gerrit-Change-Number: 21441
Gerrit-PatchSet: 4
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Comment-Date: Thu, 23 May 2024 17:31:27 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12562: Cast double and float to string with exact presicion

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

Change subject: IMPALA-12562: Cast double and float to string with exact presicion
......................................................................


Patch Set 4:

> Patch Set 4: Verified-1
> 
> Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/10665/

Tests show some real failures: https://jenkins.impala.io/job/ubuntu-20.04-from-scratch/2665/testReport/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icd79c55dd57dc0fa13e4ec11c2284ef2800e8b1a
Gerrit-Change-Number: 21441
Gerrit-PatchSet: 4
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Comment-Date: Thu, 23 May 2024 23:01:16 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12562: Cast double and float to string with exact presicion

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

Change subject: IMPALA-12562: Cast double and float to string with exact presicion
......................................................................


Patch Set 5: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icd79c55dd57dc0fa13e4ec11c2284ef2800e8b1a
Gerrit-Change-Number: 21441
Gerrit-PatchSet: 5
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Comment-Date: Fri, 24 May 2024 20:31:31 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12562: Cast double and float to string with exact presicion

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

Change subject: IMPALA-12562: Cast double and float to string with exact presicion
......................................................................


Patch Set 3: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icd79c55dd57dc0fa13e4ec11c2284ef2800e8b1a
Gerrit-Change-Number: 21441
Gerrit-PatchSet: 3
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Comment-Date: Thu, 23 May 2024 17:30:48 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12562: Cast double and float to string with exact presicion

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

Change subject: IMPALA-12562: Cast double and float to string with exact presicion
......................................................................


Patch Set 4: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/10665/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icd79c55dd57dc0fa13e4ec11c2284ef2800e8b1a
Gerrit-Change-Number: 21441
Gerrit-PatchSet: 4
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Comment-Date: Thu, 23 May 2024 22:35:43 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12562: Cast double and float to string with exact presicion

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

Change subject: IMPALA-12562: Cast double and float to string with exact presicion
......................................................................


Patch Set 5:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/16219/ : 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/21441
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icd79c55dd57dc0fa13e4ec11c2284ef2800e8b1a
Gerrit-Change-Number: 21441
Gerrit-PatchSet: 5
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Comment-Date: Fri, 24 May 2024 15:14:23 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12562: Cast double and float to string with exact presicion

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

Change subject: IMPALA-12562: Cast double and float to string with exact presicion
......................................................................


Patch Set 5:

> Patch Set 4:
> 
> > Patch Set 4: Verified-1
> > 
> > Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/10665/
> 
> Tests show some real failures: https://jenkins.impala.io/job/ubuntu-20.04-from-scratch/2665/testReport/

Related tests have been updated in PS5. We use boost::lexical_cast to cast double/float to string in TestCast, which also loses precisions. So I use string values directly for comparison.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icd79c55dd57dc0fa13e4ec11c2284ef2800e8b1a
Gerrit-Change-Number: 21441
Gerrit-PatchSet: 5
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Comment-Date: Fri, 24 May 2024 14:59:18 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12562: Cast double and float to string with exact presicion

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

Change subject: IMPALA-12562: Cast double and float to string with exact presicion
......................................................................


Patch Set 1:

Build Failed 

https://jenkins.impala.io/job/gerrit-code-review-checks/16181/ : Initial code review checks failed. See linked job for details on the failure.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icd79c55dd57dc0fa13e4ec11c2284ef2800e8b1a
Gerrit-Change-Number: 21441
Gerrit-PatchSet: 1
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Comment-Date: Fri, 17 May 2024 20:00:23 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12562: Cast double and float to string with exact presicion

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

Change subject: IMPALA-12562: Cast double and float to string with exact presicion
......................................................................


Patch Set 3:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/16203/ : 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/21441
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icd79c55dd57dc0fa13e4ec11c2284ef2800e8b1a
Gerrit-Change-Number: 21441
Gerrit-PatchSet: 3
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Comment-Date: Wed, 22 May 2024 10:20:11 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12562: Cast double and float to string with exact presicion

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

Change subject: IMPALA-12562: Cast double and float to string with exact presicion
......................................................................


Patch Set 5:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/10670/ DRY_RUN=true


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icd79c55dd57dc0fa13e4ec11c2284ef2800e8b1a
Gerrit-Change-Number: 21441
Gerrit-PatchSet: 5
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Comment-Date: Fri, 24 May 2024 15:21:14 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12562: Cast double and float to string with exact presicion

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

Change subject: IMPALA-12562: Cast double and float to string with exact presicion
......................................................................


Patch Set 6:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/10707/ DRY_RUN=false


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icd79c55dd57dc0fa13e4ec11c2284ef2800e8b1a
Gerrit-Change-Number: 21441
Gerrit-PatchSet: 6
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Comment-Date: Wed, 12 Jun 2024 12:18:35 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12562: Cast double and float to string with exact presicion

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

Change subject: IMPALA-12562: Cast double and float to string with exact presicion
......................................................................


Patch Set 3: Code-Review+1

(1 comment)

http://gerrit.cloudera.org:8080/#/c/21441/3/be/src/exprs/cast-functions-ir.cc
File be/src/exprs/cast-functions-ir.cc:

http://gerrit.cloudera.org:8080/#/c/21441/3/be/src/exprs/cast-functions-ir.cc@352
PS3, Line 352: strlen
> It seems that the returned pointer doesn't point to the terminating null by
You are right, sorry, I was confused by the way FastInt32ToBufferLeft and friends work and thought that FloatToBuffer/DoubleToBuffer doesn't point to the beginning of the buffer.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icd79c55dd57dc0fa13e4ec11c2284ef2800e8b1a
Gerrit-Change-Number: 21441
Gerrit-PatchSet: 3
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Comment-Date: Thu, 23 May 2024 12:53:28 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12562: Cast double and float to string with exact presicion

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

Change subject: IMPALA-12562: Cast double and float to string with exact presicion
......................................................................


Patch Set 2:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/16186/ : 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/21441
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icd79c55dd57dc0fa13e4ec11c2284ef2800e8b1a
Gerrit-Change-Number: 21441
Gerrit-PatchSet: 2
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Comment-Date: Sat, 18 May 2024 01:37:44 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12562: Cast double and float to string with exact presicion

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

Change subject: IMPALA-12562: Cast double and float to string with exact presicion
......................................................................


Patch Set 3:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/21441/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/21441/2//COMMIT_MSG@12
PS2, Line 12: le a
> Can you provide some benchmarks? I expect it to be an improvement, but it w
I added some benchmarks, I guess this could bring a slight performance degradation since it does more work to print more accurate results. 

According to comments in gutil/strings/numbers.cc, these implementations are about as fast as other strategies and do not need to introduce a new library.


http://gerrit.cloudera.org:8080/#/c/21441/2/be/src/exprs/cast-functions-ir.cc
File be/src/exprs/cast-functions-ir.cc:

http://gerrit.cloudera.org:8080/#/c/21441/2/be/src/exprs/cast-functions-ir.cc@344
PS2, Line 344:     if (val.is_null) return StringVal::null();                                 \
> At this point no allocation happened yet so this check is not useful.
Done


http://gerrit.cloudera.org:8080/#/c/21441/2/be/src/exprs/cast-functions-ir.cc@355
PS2, Line 355: sary(ctx->
> I would prefer using DoubleToBuffer / FloatToBuffer as it avoids the extra 
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icd79c55dd57dc0fa13e4ec11c2284ef2800e8b1a
Gerrit-Change-Number: 21441
Gerrit-PatchSet: 3
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Comment-Date: Wed, 22 May 2024 09:56:21 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12562: Cast double and float to string with exact presicion

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

Change subject: IMPALA-12562: Cast double and float to string with exact presicion
......................................................................


Patch Set 3:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/21441/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/21441/2//COMMIT_MSG@12
PS2, Line 12: le a
> I added some benchmarks, I guess this could bring a slight performance degr
looked at the implementation and it uses snprintf + some additional logic, so the regression is expected


http://gerrit.cloudera.org:8080/#/c/21441/3/be/src/exprs/cast-functions-ir.cc
File be/src/exprs/cast-functions-ir.cc:

http://gerrit.cloudera.org:8080/#/c/21441/3/be/src/exprs/cast-functions-ir.cc@352
PS3, Line 352: strlen
strlen could be avoid by saving the original pointer and comparing the difference with the returned pointer



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icd79c55dd57dc0fa13e4ec11c2284ef2800e8b1a
Gerrit-Change-Number: 21441
Gerrit-PatchSet: 3
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Comment-Date: Wed, 22 May 2024 12:33:09 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12562: Cast double and float to string with exact presicion

Posted by "Yifan Zhang (Code Review)" <ge...@cloudera.org>.
Hello Daniel Becker, Gabor Kaszab, Csaba Ringhofer, Michael Smith, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-12562: Cast double and float to string with exact presicion
......................................................................

IMPALA-12562: Cast double and float to string with exact presicion

The builtin functions casttostring(DOUBLE) and casttostring(FLOAT)
printed more digits when converting double and float values to
string values. This patch fixes this by switching to use the existing
methods DoubleToBuffer and FloatToBuffer, which are simple and fast
implementations to print necessary digits.

Testing:
  - Add end-to-end tests to verify the fixes
  - Add benchmarks for modified functions
  - Update tests in expr-test

Change-Id: Icd79c55dd57dc0fa13e4ec11c2284ef2800e8b1a
---
M be/src/benchmarks/expr-benchmark.cc
M be/src/exprs/cast-functions-ir.cc
M be/src/exprs/expr-test.cc
M testdata/workloads/functional-query/queries/QueryTest/exprs.test
4 files changed, 173 insertions(+), 71 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/41/21441/5
-- 
To view, visit http://gerrit.cloudera.org:8080/21441
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Icd79c55dd57dc0fa13e4ec11c2284ef2800e8b1a
Gerrit-Change-Number: 21441
Gerrit-PatchSet: 5
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>

[Impala-ASF-CR] IMPALA-12562: Cast double and float to string with exact presicion

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

Change subject: IMPALA-12562: Cast double and float to string with exact presicion
......................................................................


Patch Set 6: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icd79c55dd57dc0fa13e4ec11c2284ef2800e8b1a
Gerrit-Change-Number: 21441
Gerrit-PatchSet: 6
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Comment-Date: Wed, 12 Jun 2024 12:18:34 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12562: Cast double and float to string with exact presicion

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

Change subject: IMPALA-12562: Cast double and float to string with exact presicion
......................................................................


Patch Set 1: Code-Review+1

(1 comment)

Thanks for fixing this!

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

http://gerrit.cloudera.org:8080/#/c/21441/1/testdata/workloads/functional-query/queries/QueryTest/exprs.test@3300
PS1, Line 3300: select cast(round(cast(1.33 as double), 2) as string)
A few more test cases around the limits of large/small number of decimals would be nice.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icd79c55dd57dc0fa13e4ec11c2284ef2800e8b1a
Gerrit-Change-Number: 21441
Gerrit-PatchSet: 1
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Comment-Date: Fri, 17 May 2024 17:47:25 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12562: Cast double and float to string with exact presicion

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

Change subject: IMPALA-12562: Cast double and float to string with exact presicion
......................................................................


Patch Set 5: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icd79c55dd57dc0fa13e4ec11c2284ef2800e8b1a
Gerrit-Change-Number: 21441
Gerrit-PatchSet: 5
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Comment-Date: Fri, 07 Jun 2024 17:24:39 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12562: Cast double and float to string with exact presicion

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

Change subject: IMPALA-12562: Cast double and float to string with exact presicion
......................................................................


Patch Set 2:

(1 comment)

> Patch Set 1: Code-Review+1
> 
> (1 comment)
> 
> Thanks for fixing this!

Thanks for your review!

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

http://gerrit.cloudera.org:8080/#/c/21441/1/testdata/workloads/functional-query/queries/QueryTest/exprs.test@3300
PS1, Line 3300: select cast(round(cast(1.33 as double), 2) as string);
> A few more test cases around the limits of large/small number of decimals w
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icd79c55dd57dc0fa13e4ec11c2284ef2800e8b1a
Gerrit-Change-Number: 21441
Gerrit-PatchSet: 2
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Comment-Date: Sat, 18 May 2024 01:14:15 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12562: Cast double and float to string with exact presicion

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

Change subject: IMPALA-12562: Cast double and float to string with exact presicion
......................................................................


Patch Set 5: Code-Review+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icd79c55dd57dc0fa13e4ec11c2284ef2800e8b1a
Gerrit-Change-Number: 21441
Gerrit-PatchSet: 5
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Comment-Date: Wed, 29 May 2024 18:42:39 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12562: Cast double and float to string with exact presicion

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

Change subject: IMPALA-12562: Cast double and float to string with exact presicion
......................................................................


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/21441/3/be/src/exprs/cast-functions-ir.cc
File be/src/exprs/cast-functions-ir.cc:

http://gerrit.cloudera.org:8080/#/c/21441/3/be/src/exprs/cast-functions-ir.cc@352
PS3, Line 352: strlen
> strlen could be avoid by saving the original pointer and comparing the diff
It seems that the returned pointer doesn't point to the terminating null byte and it can't be used to compare with the original pointer.

I also found this strlen redundant, since snprintf returns the length of the buffer. Maybe we have to use strlen if we don't want to change implementations in gutil/strings.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icd79c55dd57dc0fa13e4ec11c2284ef2800e8b1a
Gerrit-Change-Number: 21441
Gerrit-PatchSet: 3
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Comment-Date: Thu, 23 May 2024 08:33:27 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12562: Cast double and float to string with exact presicion

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

Change subject: IMPALA-12562: Cast double and float to string with exact presicion
......................................................................


Patch Set 4:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/10665/ DRY_RUN=false


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icd79c55dd57dc0fa13e4ec11c2284ef2800e8b1a
Gerrit-Change-Number: 21441
Gerrit-PatchSet: 4
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Comment-Date: Thu, 23 May 2024 17:31:28 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12562: Cast double and float to string with exact presicion

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/21441 )

Change subject: IMPALA-12562: Cast double and float to string with exact presicion
......................................................................

IMPALA-12562: Cast double and float to string with exact presicion

The builtin functions casttostring(DOUBLE) and casttostring(FLOAT)
printed more digits when converting double and float values to
string values. This patch fixes this by switching to use the existing
methods DoubleToBuffer and FloatToBuffer, which are simple and fast
implementations to print necessary digits.

Testing:
  - Add end-to-end tests to verify the fixes
  - Add benchmarks for modified functions
  - Update tests in expr-test

Change-Id: Icd79c55dd57dc0fa13e4ec11c2284ef2800e8b1a
Reviewed-on: http://gerrit.cloudera.org:8080/21441
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/src/benchmarks/expr-benchmark.cc
M be/src/exprs/cast-functions-ir.cc
M be/src/exprs/expr-test.cc
M testdata/workloads/functional-query/queries/QueryTest/exprs.test
4 files changed, 173 insertions(+), 71 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Icd79c55dd57dc0fa13e4ec11c2284ef2800e8b1a
Gerrit-Change-Number: 21441
Gerrit-PatchSet: 7
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>

[Impala-ASF-CR] IMPALA-12562: Cast double and float to string with exact presicion

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

Change subject: IMPALA-12562: Cast double and float to string with exact presicion
......................................................................


Patch Set 6: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icd79c55dd57dc0fa13e4ec11c2284ef2800e8b1a
Gerrit-Change-Number: 21441
Gerrit-PatchSet: 6
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Comment-Date: Wed, 12 Jun 2024 17:22:29 +0000
Gerrit-HasComments: No