You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org> on 2018/09/12 15:26:08 UTC
[Impala-ASF-CR] IMPALA-7559: Disable stat filtering for UTC-normalized timestamp columns
Csaba Ringhofer has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11431
Change subject: IMPALA-7559: Disable stat filtering for UTC-normalized timestamp columns
......................................................................
IMPALA-7559: Disable stat filtering for UTC-normalized timestamp columns
If convert_legacy_hive_parquet_utc_timestamps=true and the Parquet
file is by parquet-mr (also used by Hive), then timestamps are
converted from UTC to local time during scanning. Stat filtering
did not handle this case correctly and compared UTC min/max values
from stats with local min/max values from predicates. This could
lead to skipping row groups incorrectly.
Note that parquet-mr only writes stats if min and max are equal,
because it cannot order timestamps correctly, so the only case
affected here is when every value is the same in the column chunk.
It would be possible to implement stat filtering correctly, but
this is non-trivial because of DST and historical timezone rule
changes.
Testing:
- added a Hive generated parquet file + custom cluster test
that could reproduce this issue
Change-Id: Id4c02230993f2390c03d513f08bae2e9d3d538fa
---
M be/src/exec/hdfs-parquet-scanner.cc
A testdata/data/hive_single_value_timestamp.parq
M tests/custom_cluster/test_hive_parquet_timestamp_conversion.py
3 files changed, 37 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/31/11431/1
--
To view, visit http://gerrit.cloudera.org:8080/11431
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Id4c02230993f2390c03d513f08bae2e9d3d538fa
Gerrit-Change-Number: 11431
Gerrit-PatchSet: 1
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
[Impala-ASF-CR] IMPALA-7559: Disable stat filtering for UTC-normalized timestamp columns
Posted by "Attila Jeges (Code Review)" <ge...@cloudera.org>.
Attila Jeges has posted comments on this change. ( http://gerrit.cloudera.org:8080/11431 )
Change subject: IMPALA-7559: Disable stat filtering for UTC-normalized timestamp columns
......................................................................
Patch Set 1:
It would be useful to add a description about hive_single_value_timestamp.parq to testdata/data/README
--
To view, visit http://gerrit.cloudera.org:8080/11431
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id4c02230993f2390c03d513f08bae2e9d3d538fa
Gerrit-Change-Number: 11431
Gerrit-PatchSet: 1
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 12 Sep 2018 16:12:34 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7559: Disable stat filtering for UTC-normalized timestamp columns
Posted by "Attila Jeges (Code Review)" <ge...@cloudera.org>.
Attila Jeges has posted comments on this change. ( http://gerrit.cloudera.org:8080/11431 )
Change subject: IMPALA-7559: Disable stat filtering for UTC-normalized timestamp columns
......................................................................
Patch Set 3: Code-Review+2
(2 comments)
http://gerrit.cloudera.org:8080/#/c/11431/3/be/src/exec/hdfs-parquet-scanner.cc
File be/src/exec/hdfs-parquet-scanner.cc:
http://gerrit.cloudera.org:8080/#/c/11431/3/be/src/exec/hdfs-parquet-scanner.cc@517
PS3, Line 517: IsTimezoneConversionNeededForTimestamps()
nit: this call could be moved out of the loop, before L465
http://gerrit.cloudera.org:8080/#/c/11431/3/testdata/data/README
File testdata/data/README:
http://gerrit.cloudera.org:8080/#/c/11431/3/testdata/data/README@222
PS3, Line 222: Contains a single row. It is used to test IMPALA-7559 which only occurs when all
: values in a column chunk are the same timestamp and the file is written with
: parquet-mr (which is used by Hive).
Thanks for adding the description!
nit: apply the 90-char line policy.
--
To view, visit http://gerrit.cloudera.org:8080/11431
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id4c02230993f2390c03d513f08bae2e9d3d538fa
Gerrit-Change-Number: 11431
Gerrit-PatchSet: 3
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 13 Sep 2018 15:37:05 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7559: Disable stat filtering for UTC-normalized timestamp columns
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/11431 )
Change subject: IMPALA-7559: Disable stat filtering for UTC-normalized timestamp columns
......................................................................
Patch Set 1:
(6 comments)
> It would be useful to add a description about hive_single_value_timestamp.parq
> to testdata/data/README
Thanks, I forgot README.
http://gerrit.cloudera.org:8080/#/c/11431/1/be/src/exec/hdfs-parquet-scanner.cc
File be/src/exec/hdfs-parquet-scanner.cc:
http://gerrit.cloudera.org:8080/#/c/11431/1/be/src/exec/hdfs-parquet-scanner.cc@471
PS1, Line 471: // need to be converted from local to UTC. Doing this correctly is quite complex if
> Presumably the solution is to do the same conversion on the stat value that
I think that converting the predicate's min/max value to UTC will be the better way (if ambiguous, choose smaller for min, bigger for max). The problem is that during a UTC->local conversion the order of timestamps may become different, so if the "which one is min/max" was calculated on UTC timestamps, then the filtering also has to be done on UTC timestamps.
There is no Jira yet. My plan is look into parquet-mr a bit deeper to see how it handles this.
http://gerrit.cloudera.org:8080/#/c/11431/1/be/src/exec/hdfs-parquet-scanner.cc@478
PS1, Line 478: if (slot_desc->type().IsTimestampType() &&
> This is the same condition as in parquet-column-readers.cc
Both conditions will have to be touched in https://gerrit.cloudera.org/#/c/11057/, so I would prefer to postpone refactoring to that patch.
http://gerrit.cloudera.org:8080/#/c/11431/1/be/src/exec/hdfs-parquet-scanner.cc@480
PS1, Line 480: continue;
> nit: since the if condition stretches over multiple lines, the body of the
Done
http://gerrit.cloudera.org:8080/#/c/11431/1/tests/custom_cluster/test_hive_parquet_timestamp_conversion.py
File tests/custom_cluster/test_hive_parquet_timestamp_conversion.py:
http://gerrit.cloudera.org:8080/#/c/11431/1/tests/custom_cluster/test_hive_parquet_timestamp_conversion.py@147
PS1, Line 147: data = self.execute_query_expect_success(self.client,
: 'select * from %s.t where d = "2018-10-01 02:30:00"' % unique_database,
: query_options = {"timezone": "CET"}).get_data()
> It might be useful to execute a "select * from db.t" and examine what's in
Done
http://gerrit.cloudera.org:8080/#/c/11431/1/tests/custom_cluster/test_hive_parquet_timestamp_conversion.py@149
PS1, Line 149:
> flake8: E251 unexpected spaces around keyword / parameter equals
Done
http://gerrit.cloudera.org:8080/#/c/11431/1/tests/custom_cluster/test_hive_parquet_timestamp_conversion.py@149
PS1, Line 149:
> flake8: E251 unexpected spaces around keyword / parameter equals
Done
--
To view, visit http://gerrit.cloudera.org:8080/11431
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id4c02230993f2390c03d513f08bae2e9d3d538fa
Gerrit-Change-Number: 11431
Gerrit-PatchSet: 1
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 12 Sep 2018 17:47:24 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7559: Disable stat filtering for UTC-normalized timestamp columns
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/11431 )
Change subject: IMPALA-7559: Disable stat filtering for UTC-normalized timestamp columns
......................................................................
Patch Set 4: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/11431
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id4c02230993f2390c03d513f08bae2e9d3d538fa
Gerrit-Change-Number: 11431
Gerrit-PatchSet: 4
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 13 Sep 2018 18:01:33 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7559: Disable stat filtering for UTC-normalized timestamp columns
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11431 )
Change subject: IMPALA-7559: Disable stat filtering for UTC-normalized timestamp columns
......................................................................
Patch Set 2:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/651/ : 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/11431
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id4c02230993f2390c03d513f08bae2e9d3d538fa
Gerrit-Change-Number: 11431
Gerrit-PatchSet: 2
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 12 Sep 2018 17:42:24 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7559: Disable stat filtering for UTC-normalized timestamp columns
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11431 )
Change subject: IMPALA-7559: Disable stat filtering for UTC-normalized timestamp columns
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/649/ : 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/11431
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id4c02230993f2390c03d513f08bae2e9d3d538fa
Gerrit-Change-Number: 11431
Gerrit-PatchSet: 1
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Wed, 12 Sep 2018 15:59:50 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7559: Disable stat filtering for UTC-normalized timestamp columns
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/11431 )
Change subject: IMPALA-7559: Disable stat filtering for UTC-normalized timestamp columns
......................................................................
IMPALA-7559: Disable stat filtering for UTC-normalized timestamp columns
If convert_legacy_hive_parquet_utc_timestamps=true and the Parquet
file is by parquet-mr (also used by Hive), then timestamps are
converted from UTC to local time during scanning. Stat filtering
did not handle this case correctly and compared UTC min/max values
from stats with local min/max values from predicates. This could
lead to skipping row groups incorrectly.
Note that parquet-mr only writes stats if min and max are equal,
because it cannot order timestamps correctly, so the only case
affected here is when every value is the same in the column chunk.
It would be possible to implement stat filtering correctly, but
this is non-trivial because of DST and historical timezone rule
changes.
Testing:
- added a Hive generated parquet file + custom cluster test
that could reproduce this issue
Change-Id: Id4c02230993f2390c03d513f08bae2e9d3d538fa
Reviewed-on: http://gerrit.cloudera.org:8080/11431
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/src/exec/hdfs-parquet-scanner.cc
M be/src/exec/hdfs-parquet-scanner.h
M be/src/exec/parquet-column-readers.cc
M testdata/data/README
A testdata/data/hive_single_value_timestamp.parq
M tests/custom_cluster/test_hive_parquet_timestamp_conversion.py
6 files changed, 74 insertions(+), 12 deletions(-)
Approvals:
Impala Public Jenkins: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/11431
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Id4c02230993f2390c03d513f08bae2e9d3d538fa
Gerrit-Change-Number: 11431
Gerrit-PatchSet: 6
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-7559: Disable stat filtering for UTC-normalized timestamp columns
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11431 )
Change subject: IMPALA-7559: Disable stat filtering for UTC-normalized timestamp columns
......................................................................
Patch Set 3:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/660/ : 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/11431
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id4c02230993f2390c03d513f08bae2e9d3d538fa
Gerrit-Change-Number: 11431
Gerrit-PatchSet: 3
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 13 Sep 2018 12:48:02 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7559: Disable stat filtering for UTC-normalized timestamp columns
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11431 )
Change subject: IMPALA-7559: Disable stat filtering for UTC-normalized timestamp columns
......................................................................
Patch Set 5:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3155/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/11431
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id4c02230993f2390c03d513f08bae2e9d3d538fa
Gerrit-Change-Number: 11431
Gerrit-PatchSet: 5
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 13 Sep 2018 18:02:28 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7559: Disable stat filtering for UTC-normalized timestamp columns
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/11431 )
Change subject: IMPALA-7559: Disable stat filtering for UTC-normalized timestamp columns
......................................................................
Patch Set 4:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/11431/3/be/src/exec/hdfs-parquet-scanner.cc
File be/src/exec/hdfs-parquet-scanner.cc:
http://gerrit.cloudera.org:8080/#/c/11431/3/be/src/exec/hdfs-parquet-scanner.cc@517
PS3, Line 517: desc->type();
> nit: this call could be moved out of the loop, before L465
Done.
I have also moved the null_count check before the min_max check because null_count check is a valid optimization in this case.
http://gerrit.cloudera.org:8080/#/c/11431/3/testdata/data/README
File testdata/data/README:
http://gerrit.cloudera.org:8080/#/c/11431/3/testdata/data/README@222
PS3, Line 222: Contains a single row. It is used to test IMPALA-7559 which only occurs when all values
: in a column chunk are the same timestamp and the file is written with parquet-mr (which
: is used by Hive).
> Thanks for adding the description!
Done
--
To view, visit http://gerrit.cloudera.org:8080/11431
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id4c02230993f2390c03d513f08bae2e9d3d538fa
Gerrit-Change-Number: 11431
Gerrit-PatchSet: 4
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 13 Sep 2018 16:05:10 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7559: Disable stat filtering for UTC-normalized timestamp columns
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11431 )
Change subject: IMPALA-7559: Disable stat filtering for UTC-normalized timestamp columns
......................................................................
Patch Set 4:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/662/ : 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/11431
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id4c02230993f2390c03d513f08bae2e9d3d538fa
Gerrit-Change-Number: 11431
Gerrit-PatchSet: 4
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 13 Sep 2018 16:46:29 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7559: Disable stat filtering for UTC-normalized timestamp columns
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11431 )
Change subject: IMPALA-7559: Disable stat filtering for UTC-normalized timestamp columns
......................................................................
Patch Set 1:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3148/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/11431
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id4c02230993f2390c03d513f08bae2e9d3d538fa
Gerrit-Change-Number: 11431
Gerrit-PatchSet: 1
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Wed, 12 Sep 2018 15:27:23 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7559: Disable stat filtering for UTC-normalized timestamp columns
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11431 )
Change subject: IMPALA-7559: Disable stat filtering for UTC-normalized timestamp columns
......................................................................
Patch Set 1: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/11431
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id4c02230993f2390c03d513f08bae2e9d3d538fa
Gerrit-Change-Number: 11431
Gerrit-PatchSet: 1
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 12 Sep 2018 18:59:10 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7559: Disable stat filtering for UTC-normalized timestamp columns
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Hello Attila Jeges, Tim Armstrong, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/11431
to look at the new patch set (#4).
Change subject: IMPALA-7559: Disable stat filtering for UTC-normalized timestamp columns
......................................................................
IMPALA-7559: Disable stat filtering for UTC-normalized timestamp columns
If convert_legacy_hive_parquet_utc_timestamps=true and the Parquet
file is by parquet-mr (also used by Hive), then timestamps are
converted from UTC to local time during scanning. Stat filtering
did not handle this case correctly and compared UTC min/max values
from stats with local min/max values from predicates. This could
lead to skipping row groups incorrectly.
Note that parquet-mr only writes stats if min and max are equal,
because it cannot order timestamps correctly, so the only case
affected here is when every value is the same in the column chunk.
It would be possible to implement stat filtering correctly, but
this is non-trivial because of DST and historical timezone rule
changes.
Testing:
- added a Hive generated parquet file + custom cluster test
that could reproduce this issue
Change-Id: Id4c02230993f2390c03d513f08bae2e9d3d538fa
---
M be/src/exec/hdfs-parquet-scanner.cc
M be/src/exec/hdfs-parquet-scanner.h
M be/src/exec/parquet-column-readers.cc
M testdata/data/README
A testdata/data/hive_single_value_timestamp.parq
M tests/custom_cluster/test_hive_parquet_timestamp_conversion.py
6 files changed, 74 insertions(+), 12 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/31/11431/4
--
To view, visit http://gerrit.cloudera.org:8080/11431
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id4c02230993f2390c03d513f08bae2e9d3d538fa
Gerrit-Change-Number: 11431
Gerrit-PatchSet: 4
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-7559: Disable stat filtering for UTC-normalized timestamp columns
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11431 )
Change subject: IMPALA-7559: Disable stat filtering for UTC-normalized timestamp columns
......................................................................
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/11431/1/tests/custom_cluster/test_hive_parquet_timestamp_conversion.py
File tests/custom_cluster/test_hive_parquet_timestamp_conversion.py:
http://gerrit.cloudera.org:8080/#/c/11431/1/tests/custom_cluster/test_hive_parquet_timestamp_conversion.py@149
PS1, Line 149:
flake8: E251 unexpected spaces around keyword / parameter equals
http://gerrit.cloudera.org:8080/#/c/11431/1/tests/custom_cluster/test_hive_parquet_timestamp_conversion.py@149
PS1, Line 149:
flake8: E251 unexpected spaces around keyword / parameter equals
--
To view, visit http://gerrit.cloudera.org:8080/11431
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id4c02230993f2390c03d513f08bae2e9d3d538fa
Gerrit-Change-Number: 11431
Gerrit-PatchSet: 1
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Wed, 12 Sep 2018 16:02:52 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7559: Disable stat filtering for UTC-normalized timestamp columns
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Hello Attila Jeges, Tim Armstrong, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/11431
to look at the new patch set (#2).
Change subject: IMPALA-7559: Disable stat filtering for UTC-normalized timestamp columns
......................................................................
IMPALA-7559: Disable stat filtering for UTC-normalized timestamp columns
If convert_legacy_hive_parquet_utc_timestamps=true and the Parquet
file is by parquet-mr (also used by Hive), then timestamps are
converted from UTC to local time during scanning. Stat filtering
did not handle this case correctly and compared UTC min/max values
from stats with local min/max values from predicates. This could
lead to skipping row groups incorrectly.
Note that parquet-mr only writes stats if min and max are equal,
because it cannot order timestamps correctly, so the only case
affected here is when every value is the same in the column chunk.
It would be possible to implement stat filtering correctly, but
this is non-trivial because of DST and historical timezone rule
changes.
Testing:
- added a Hive generated parquet file + custom cluster test
that could reproduce this issue
Change-Id: Id4c02230993f2390c03d513f08bae2e9d3d538fa
---
M be/src/exec/hdfs-parquet-scanner.cc
M testdata/data/README
A testdata/data/hive_single_value_timestamp.parq
M tests/custom_cluster/test_hive_parquet_timestamp_conversion.py
4 files changed, 56 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/31/11431/2
--
To view, visit http://gerrit.cloudera.org:8080/11431
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id4c02230993f2390c03d513f08bae2e9d3d538fa
Gerrit-Change-Number: 11431
Gerrit-PatchSet: 2
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-7559: Disable stat filtering for UTC-normalized timestamp columns
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11431 )
Change subject: IMPALA-7559: Disable stat filtering for UTC-normalized timestamp columns
......................................................................
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/11431/1/be/src/exec/hdfs-parquet-scanner.cc
File be/src/exec/hdfs-parquet-scanner.cc:
http://gerrit.cloudera.org:8080/#/c/11431/1/be/src/exec/hdfs-parquet-scanner.cc@471
PS1, Line 471: // need to be converted from local to UTC. Doing this correctly is quite complex if
Presumably the solution is to do the same conversion on the stat value that we would do on the table value, right?
Is there a JIRA to implement this fix and re-enable filtering?
http://gerrit.cloudera.org:8080/#/c/11431/1/be/src/exec/hdfs-parquet-scanner.cc@478
PS1, Line 478: if (slot_desc->type().IsTimestampType() &&
This is the same condition as in parquet-column-readers.cc
{code}
FLAGS_convert_legacy_hive_parquet_utc_timestamps &&
slot_desc_->type().type == TYPE_TIMESTAMP &&
parent->file_version_.application == "parquet-mr")
{code}
It would be good to de-duplicate the logic into a common function so that it's obvious that the preconditions are meant to be identical. Otherwise at least add comments to cross-reference so future people know they need to be updated in sync.
--
To view, visit http://gerrit.cloudera.org:8080/11431
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id4c02230993f2390c03d513f08bae2e9d3d538fa
Gerrit-Change-Number: 11431
Gerrit-PatchSet: 1
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 12 Sep 2018 16:02:56 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7559: Disable stat filtering for UTC-normalized timestamp columns
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11431 )
Change subject: IMPALA-7559: Disable stat filtering for UTC-normalized timestamp columns
......................................................................
Patch Set 5: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/11431
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id4c02230993f2390c03d513f08bae2e9d3d538fa
Gerrit-Change-Number: 11431
Gerrit-PatchSet: 5
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 13 Sep 2018 18:02:27 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7559: Disable stat filtering for UTC-normalized timestamp columns
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11431 )
Change subject: IMPALA-7559: Disable stat filtering for UTC-normalized timestamp columns
......................................................................
Patch Set 5: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/11431
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id4c02230993f2390c03d513f08bae2e9d3d538fa
Gerrit-Change-Number: 11431
Gerrit-PatchSet: 5
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 13 Sep 2018 21:18:54 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7559: Disable stat filtering for UTC-normalized timestamp columns
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/11431 )
Change subject: IMPALA-7559: Disable stat filtering for UTC-normalized timestamp columns
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/11431/2/be/src/exec/hdfs-parquet-scanner.cc
File be/src/exec/hdfs-parquet-scanner.cc:
http://gerrit.cloudera.org:8080/#/c/11431/2/be/src/exec/hdfs-parquet-scanner.cc@482
PS2, Line 482: break;
> Can you add a note in both places that this appears then just so it's obvio
Finally I have moved the code into a separate function.
+ this block is moved a few lines below to keep the "skip the row group if there are missing fields with min/max conjunct" rule.
--
To view, visit http://gerrit.cloudera.org:8080/11431
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id4c02230993f2390c03d513f08bae2e9d3d538fa
Gerrit-Change-Number: 11431
Gerrit-PatchSet: 3
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 13 Sep 2018 12:20:27 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7559: Disable stat filtering for UTC-normalized timestamp columns
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11431 )
Change subject: IMPALA-7559: Disable stat filtering for UTC-normalized timestamp columns
......................................................................
Patch Set 3: Code-Review+1
I'll give Attila a chance to look and +2
--
To view, visit http://gerrit.cloudera.org:8080/11431
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id4c02230993f2390c03d513f08bae2e9d3d538fa
Gerrit-Change-Number: 11431
Gerrit-PatchSet: 3
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 13 Sep 2018 14:56:47 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7559: Disable stat filtering for UTC-normalized timestamp columns
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Hello Attila Jeges, Tim Armstrong, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/11431
to look at the new patch set (#3).
Change subject: IMPALA-7559: Disable stat filtering for UTC-normalized timestamp columns
......................................................................
IMPALA-7559: Disable stat filtering for UTC-normalized timestamp columns
If convert_legacy_hive_parquet_utc_timestamps=true and the Parquet
file is by parquet-mr (also used by Hive), then timestamps are
converted from UTC to local time during scanning. Stat filtering
did not handle this case correctly and compared UTC min/max values
from stats with local min/max values from predicates. This could
lead to skipping row groups incorrectly.
Note that parquet-mr only writes stats if min and max are equal,
because it cannot order timestamps correctly, so the only case
affected here is when every value is the same in the column chunk.
It would be possible to implement stat filtering correctly, but
this is non-trivial because of DST and historical timezone rule
changes.
Testing:
- added a Hive generated parquet file + custom cluster test
that could reproduce this issue
Change-Id: Id4c02230993f2390c03d513f08bae2e9d3d538fa
---
M be/src/exec/hdfs-parquet-scanner.cc
M be/src/exec/hdfs-parquet-scanner.h
M be/src/exec/parquet-column-readers.cc
M testdata/data/README
A testdata/data/hive_single_value_timestamp.parq
M tests/custom_cluster/test_hive_parquet_timestamp_conversion.py
6 files changed, 67 insertions(+), 5 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/31/11431/3
--
To view, visit http://gerrit.cloudera.org:8080/11431
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id4c02230993f2390c03d513f08bae2e9d3d538fa
Gerrit-Change-Number: 11431
Gerrit-PatchSet: 3
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-7559: Disable stat filtering for UTC-normalized timestamp columns
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11431 )
Change subject: IMPALA-7559: Disable stat filtering for UTC-normalized timestamp columns
......................................................................
Patch Set 2: Code-Review+1
(1 comment)
http://gerrit.cloudera.org:8080/#/c/11431/2/be/src/exec/hdfs-parquet-scanner.cc
File be/src/exec/hdfs-parquet-scanner.cc:
http://gerrit.cloudera.org:8080/#/c/11431/2/be/src/exec/hdfs-parquet-scanner.cc@482
PS2, Line 482: if (slot_desc->type().IsTimestampType() &&
Can you add a note in both places that this appears then just so it's obvious they need to be updated in sync
--
To view, visit http://gerrit.cloudera.org:8080/11431
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id4c02230993f2390c03d513f08bae2e9d3d538fa
Gerrit-Change-Number: 11431
Gerrit-PatchSet: 2
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 12 Sep 2018 17:58:52 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7559: Disable stat filtering for UTC-normalized timestamp columns
Posted by "Attila Jeges (Code Review)" <ge...@cloudera.org>.
Attila Jeges has posted comments on this change. ( http://gerrit.cloudera.org:8080/11431 )
Change subject: IMPALA-7559: Disable stat filtering for UTC-normalized timestamp columns
......................................................................
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/11431/1/be/src/exec/hdfs-parquet-scanner.cc
File be/src/exec/hdfs-parquet-scanner.cc:
http://gerrit.cloudera.org:8080/#/c/11431/1/be/src/exec/hdfs-parquet-scanner.cc@480
PS1, Line 480: continue;
nit: since the if condition stretches over multiple lines, the body of the statement should be put between { and } for readability.
http://gerrit.cloudera.org:8080/#/c/11431/1/tests/custom_cluster/test_hive_parquet_timestamp_conversion.py
File tests/custom_cluster/test_hive_parquet_timestamp_conversion.py:
http://gerrit.cloudera.org:8080/#/c/11431/1/tests/custom_cluster/test_hive_parquet_timestamp_conversion.py@147
PS1, Line 147: data = self.execute_query_expect_success(self.client,
: 'select * from %s.t where d = "2018-10-01 02:30:00"' % unique_database,
: query_options = {"timezone": "CET"}).get_data()
It might be useful to execute a "select * from db.t" and examine what's in the table first, before testing the stat filtering.
--
To view, visit http://gerrit.cloudera.org:8080/11431
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id4c02230993f2390c03d513f08bae2e9d3d538fa
Gerrit-Change-Number: 11431
Gerrit-PatchSet: 1
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 12 Sep 2018 16:39:08 +0000
Gerrit-HasComments: Yes