[Impala-ASF-CR] IMPALA-7923: DecimalValue should be marked as packed

2020-07-08 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16134 )

Change subject: IMPALA-7923: DecimalValue should be marked as packed
..


Patch Set 5: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I55f936a4f4f4b5faf129a9265222e64fc486b8ed
Gerrit-Change-Number: 16134
Gerrit-PatchSet: 5
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 08 Jul 2020 20:23:01 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7923: DecimalValue should be marked as packed

2020-07-08 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/16134 )

Change subject: IMPALA-7923: DecimalValue should be marked as packed
..

IMPALA-7923: DecimalValue should be marked as packed

IMPALA-7473 and IMPALA-9111 were symptoms of a more general problem that
DecimalValue is not guaranteed to be aligned by the Impala runtime but
the compiler assumes it is and under some circumstances, it will emit
code for aligned loads to value_ when value_ is an int128.

This commit marks DecimalValue as packed so that the compiler does not
assume any alignment.

TODO: Maybe benchmark if this introduces performance regressions, but it
shouldn't.

Change-Id: I55f936a4f4f4b5faf129a9265222e64fc486b8ed
Reviewed-on: http://gerrit.cloudera.org:8080/16134
Reviewed-by: Impala Public Jenkins 
Tested-by: Impala Public Jenkins 
---
M be/src/exec/hdfs-avro-scanner-test.cc
M be/src/exec/orc-column-readers.h
M be/src/runtime/decimal-value.h
M be/src/runtime/decimal-value.inline.h
M be/src/util/decimal-util.h
M be/src/util/dict-test.cc
6 files changed, 35 insertions(+), 21 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I55f936a4f4f4b5faf129a9265222e64fc486b8ed
Gerrit-Change-Number: 16134
Gerrit-PatchSet: 6
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-7923: DecimalValue should be marked as packed

2020-07-08 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16134 )

Change subject: IMPALA-7923: DecimalValue should be marked as packed
..


Patch Set 5:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I55f936a4f4f4b5faf129a9265222e64fc486b8ed
Gerrit-Change-Number: 16134
Gerrit-PatchSet: 5
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 08 Jul 2020 15:09:13 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7923: DecimalValue should be marked as packed

2020-07-08 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16134 )

Change subject: IMPALA-7923: DecimalValue should be marked as packed
..


Patch Set 5: Verified-1

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I55f936a4f4f4b5faf129a9265222e64fc486b8ed
Gerrit-Change-Number: 16134
Gerrit-PatchSet: 5
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 08 Jul 2020 13:59:06 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7923: DecimalValue should be marked as packed

2020-07-08 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16134 )

Change subject: IMPALA-7923: DecimalValue should be marked as packed
..


Patch Set 5:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I55f936a4f4f4b5faf129a9265222e64fc486b8ed
Gerrit-Change-Number: 16134
Gerrit-PatchSet: 5
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 08 Jul 2020 08:52:13 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7923: DecimalValue should be marked as packed

2020-07-08 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16134 )

Change subject: IMPALA-7923: DecimalValue should be marked as packed
..


Patch Set 5: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I55f936a4f4f4b5faf129a9265222e64fc486b8ed
Gerrit-Change-Number: 16134
Gerrit-PatchSet: 5
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 08 Jul 2020 08:52:12 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7923: DecimalValue should be marked as packed

2020-07-07 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16134 )

Change subject: IMPALA-7923: DecimalValue should be marked as packed
..


Patch Set 4: Verified-1

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I55f936a4f4f4b5faf129a9265222e64fc486b8ed
Gerrit-Change-Number: 16134
Gerrit-PatchSet: 4
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 07 Jul 2020 22:04:15 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7923: DecimalValue should be marked as packed

2020-07-07 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16134 )

Change subject: IMPALA-7923: DecimalValue should be marked as packed
..


Patch Set 3:

I was a little scared since Q1 is a decimal-intensive query, but seems to be 
fine when I tried to repro:

Report Generated on 2020-07-07
Run Description: "6c8a3dfc339e43a8992af2ff3429ba5940a061ec vs 
513c19bc0a750960b97f0d4cd14a9bdc8bbd2860"

Cluster Name: UNKNOWN
Lab Run Info: UNKNOWN
Impala Version:  impalad version 4.0.0-SNAPSHOT RELEASE ()
Baseline Impala Version: impalad version 4.0.0-SNAPSHOT RELEASE (2020-07-01)

+--+---+-++++
| Workload | File Format   | Avg (s) | Delta(Avg) | GeoMean(s) | 
Delta(GeoMean) |
+--+---+-++++
| TPCH(30) | parquet / none / none | 10.05   | +0.18% | 10.05  | +0.18% 
|
+--+---+-++++

+--+-+---++-++---++---++-+--+
| Workload | Query   | File Format   | Avg(s) | Base Avg(s) | 
Delta(Avg) | StdDev(%) | Base StdDev(%) | Iters | Median Diff(%) | MW Zval | 
Tval |
+--+-+---++-++---++---++-+--+
| TPCH(30) | TPCH-Q1 | parquet / none / none | 10.05  | 10.03   |   +0.18%  
 |   1.16%   |   0.64%| 20|   -0.00%   | -0.04   | 0.61 |
+--+-+---++-++---++---++-+--+


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I55f936a4f4f4b5faf129a9265222e64fc486b8ed
Gerrit-Change-Number: 16134
Gerrit-PatchSet: 3
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 07 Jul 2020 16:57:14 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7923: DecimalValue should be marked as packed

2020-07-07 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16134 )

Change subject: IMPALA-7923: DecimalValue should be marked as packed
..


Patch Set 4: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I55f936a4f4f4b5faf129a9265222e64fc486b8ed
Gerrit-Change-Number: 16134
Gerrit-PatchSet: 4
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 07 Jul 2020 16:57:28 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7923: DecimalValue should be marked as packed

2020-07-07 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16134 )

Change subject: IMPALA-7923: DecimalValue should be marked as packed
..


Patch Set 4:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I55f936a4f4f4b5faf129a9265222e64fc486b8ed
Gerrit-Change-Number: 16134
Gerrit-PatchSet: 4
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 07 Jul 2020 16:57:29 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7923: DecimalValue should be marked as packed

2020-07-07 Thread Daniel Becker (Code Review)
Daniel Becker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16134 )

Change subject: IMPALA-7923: DecimalValue should be marked as packed
..


Patch Set 3:

I did the test on parquet/none/none, scale factor 15. Copied the results to the 
Jira issue.
TPCH-Q1 became a regression.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I55f936a4f4f4b5faf129a9265222e64fc486b8ed
Gerrit-Change-Number: 16134
Gerrit-PatchSet: 3
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 07 Jul 2020 10:20:20 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7923: DecimalValue should be marked as packed

2020-07-06 Thread Daniel Becker (Code Review)
Daniel Becker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16134 )

Change subject: IMPALA-7923: DecimalValue should be marked as packed
..


Patch Set 3:

I'm in the process of loading the tables for TPCH scale factor 15, but it seems 
to be very slow but let's see.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I55f936a4f4f4b5faf129a9265222e64fc486b8ed
Gerrit-Change-Number: 16134
Gerrit-PatchSet: 3
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 06 Jul 2020 17:15:01 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7923: DecimalValue should be marked as packed

2020-07-06 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16134 )

Change subject: IMPALA-7923: DecimalValue should be marked as packed
..


Patch Set 3: Code-Review+2

(1 comment)

GOod point about fixing the crash. I think the risk of a performance regression 
is low.

http://gerrit.cloudera.org:8080/#/c/16134/3/be/src/exec/hdfs-avro-scanner-test.cc
File be/src/exec/hdfs-avro-scanner-test.cc:

http://gerrit.cloudera.org:8080/#/c/16134/3/be/src/exec/hdfs-avro-scanner-test.cc@519
PS3, Line 519:   memcpy([1], _value2, 16);
> I wonder if it makes sense to keep and update the BIG_ENDIAN branch. AFAIK
Yeah we should feel free to remove these. There was some effort to make stuff 
endian-neutral, I think cause it's just a "good practice", but that code isn't 
tested and will probably never be used, so no point maintaining it.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I55f936a4f4f4b5faf129a9265222e64fc486b8ed
Gerrit-Change-Number: 16134
Gerrit-PatchSet: 3
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 06 Jul 2020 16:38:20 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7923: DecimalValue should be marked as packed

2020-07-06 Thread Csaba Ringhofer (Code Review)
Csaba Ringhofer has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16134 )

Change subject: IMPALA-7923: DecimalValue should be marked as packed
..


Patch Set 3: Code-Review+1

(1 comment)

lgtm
I am also interested in the benchmark, but from my side it can be merged as it 
is even if the patch introduces regression, as it fixes a crash.

http://gerrit.cloudera.org:8080/#/c/16134/3/be/src/exec/hdfs-avro-scanner-test.cc
File be/src/exec/hdfs-avro-scanner-test.cc:

http://gerrit.cloudera.org:8080/#/c/16134/3/be/src/exec/hdfs-avro-scanner-test.cc@519
PS3, Line 519:   memcpy([1], _value2, 16);
I wonder if it makes sense to keep and update the BIG_ENDIAN branch. AFAIK 
Impala is never compiled on BIG_ENDIAN architectures, end we have 
static_asserts to ensure this.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I55f936a4f4f4b5faf129a9265222e64fc486b8ed
Gerrit-Change-Number: 16134
Gerrit-PatchSet: 3
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 06 Jul 2020 14:22:09 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7923: DecimalValue should be marked as packed

2020-07-03 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16134 )

Change subject: IMPALA-7923: DecimalValue should be marked as packed
..


Patch Set 3:

I think scale factor 2 might be too small to get meaningful results - the query 
runtime is probably dominated by the start-up time at that scale so might not 
capture differences in execution. Could you maybe rerun with SF >= 10? That's 
usually enough so that startup time is a smaller fraction.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I55f936a4f4f4b5faf129a9265222e64fc486b8ed
Gerrit-Change-Number: 16134
Gerrit-PatchSet: 3
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 03 Jul 2020 18:52:16 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7923: DecimalValue should be marked as packed

2020-07-02 Thread Daniel Becker (Code Review)
Daniel Becker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16134 )

Change subject: IMPALA-7923: DecimalValue should be marked as packed
..


Patch Set 3:

I run single node perf tests on TPCH with scale factor 2, copied the report to 
the Jira issue.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I55f936a4f4f4b5faf129a9265222e64fc486b8ed
Gerrit-Change-Number: 16134
Gerrit-PatchSet: 3
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 02 Jul 2020 23:57:12 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7923: DecimalValue should be marked as packed

2020-07-01 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16134 )

Change subject: IMPALA-7923: DecimalValue should be marked as packed
..


Patch Set 3:

Build Successful

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I55f936a4f4f4b5faf129a9265222e64fc486b8ed
Gerrit-Change-Number: 16134
Gerrit-PatchSet: 3
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 02 Jul 2020 00:10:12 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7923: DecimalValue should be marked as packed

2020-07-01 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16134 )

Change subject: IMPALA-7923: DecimalValue should be marked as packed
..


Patch Set 2:

Build Successful

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I55f936a4f4f4b5faf129a9265222e64fc486b8ed
Gerrit-Change-Number: 16134
Gerrit-PatchSet: 2
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 02 Jul 2020 00:08:22 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7923: DecimalValue should be marked as packed

2020-07-01 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16134 )

Change subject: IMPALA-7923: DecimalValue should be marked as packed
..


Patch Set 3:

I took a quick pass and this seems fine in concept. I think we probably want to 
run tests with a release build before merging. It'd be good to do a TPC-H 
single node run as a sanity check. You could spot check with TPC-H Q1 since 
that's very decimal intensive.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I55f936a4f4f4b5faf129a9265222e64fc486b8ed
Gerrit-Change-Number: 16134
Gerrit-PatchSet: 3
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 02 Jul 2020 00:07:54 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7923: DecimalValue should be marked as packed

2020-07-01 Thread Daniel Becker (Code Review)
Daniel Becker has uploaded a new patch set (#3). ( 
http://gerrit.cloudera.org:8080/16134 )

Change subject: IMPALA-7923: DecimalValue should be marked as packed
..

IMPALA-7923: DecimalValue should be marked as packed

IMPALA-7473 and IMPALA-9111 were symptoms of a more general problem that
DecimalValue is not guaranteed to be aligned by the Impala runtime but
the compiler assumes it is and under some circumstances, it will emit
code for aligned loads to value_ when value_ is an int128.

This commit marks DecimalValue as packed so that the compiler does not
assume any alignment.

TODO: Maybe benchmark if this introduces performance regressions, but it
shouldn't.

Change-Id: I55f936a4f4f4b5faf129a9265222e64fc486b8ed
---
M be/src/exec/hdfs-avro-scanner-test.cc
M be/src/exec/orc-column-readers.h
M be/src/runtime/decimal-value.h
M be/src/runtime/decimal-value.inline.h
M be/src/util/decimal-util.h
M be/src/util/dict-test.cc
6 files changed, 35 insertions(+), 21 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I55f936a4f4f4b5faf129a9265222e64fc486b8ed
Gerrit-Change-Number: 16134
Gerrit-PatchSet: 3
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-7923: DecimalValue should be marked as packed

2020-07-01 Thread Daniel Becker (Code Review)
Daniel Becker has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/16134


Change subject: IMPALA-7923: DecimalValue should be marked as packed
..

IMPALA-7923: DecimalValue should be marked as packed

IMPALA-7473 and IMPALA-9111 were symptoms of a more general problem that
DecimalValue is not guaranteed to be aligned by the Impala runtime but
the compiler assumes it is and under some circumstances, it will emit
code for aligned loads to value_ when value_ is an int128.

This commit marks DecimalValue as packed so that the compiler does not
assume any alignment.

TODO: Maybe benchmark if this introduces performance regressions, but it
shouldn't.

Change-Id: I55f936a4f4f4b5faf129a9265222e64fc486b8ed
---
M be/src/exec/hdfs-avro-scanner-test.cc
M be/src/exec/orc-column-readers.h
M be/src/runtime/decimal-value.h
M be/src/util/decimal-util.h
M be/src/util/dict-test.cc
5 files changed, 33 insertions(+), 15 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I55f936a4f4f4b5faf129a9265222e64fc486b8ed
Gerrit-Change-Number: 16134
Gerrit-PatchSet: 2
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Daniel Becker