You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@hive.apache.org by "simhadri-g (via GitHub)" <gi...@apache.org> on 2023/01/31 10:28:50 UTC
[GitHub] [hive] simhadri-g opened a new pull request, #4000: HIVE-27007: Iceberg: Use BasicStats from iceberg table's currrentSnap…
simhadri-g opened a new pull request, #4000:
URL: https://github.com/apache/hive/pull/4000
…shot.summary() for query planning
<!--
Thanks for sending a pull request! Here are some tips for you:
1. If this is your first time, please read our contributor guidelines: https://cwiki.apache.org/confluence/display/Hive/HowToContribute
2. Ensure that you have created an issue on the Hive project JIRA: https://issues.apache.org/jira/projects/HIVE/summary
3. Ensure you have added or run the appropriate tests for your PR:
4. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP]HIVE-XXXXX: Your PR title ...'.
5. Be sure to keep the PR description updated to reflect all changes.
6. Please write your PR title to summarize what this PR proposes.
7. If possible, provide a concise example to reproduce the issue for a faster review.
-->
### What changes were proposed in this pull request?
<!--
Please clarify what changes you are proposing. The purpose of this section is to outline the changes and how this PR fixes the issue.
If possible, please consider writing useful notes for better and faster reviews in your PR. See the examples below.
1. If you refactor some codes with changing classes, showing the class hierarchy will help reviewers.
2. If you fix some SQL features, you can provide some references of other DBMSes.
3. If there is design documentation, please add the link.
4. If there is a discussion in the mailing list, please add the link.
-->
### Why are the changes needed?
<!--
Please clarify why the changes are needed. For instance,
1. If you propose a new API, clarify the use case for a new API.
2. If you fix a bug, you can clarify why it is a bug.
-->
### Does this PR introduce _any_ user-facing change?
<!--
Note that it means *any* user-facing change including all aspects such as the documentation fix.
If yes, please clarify the previous behavior and the change this PR proposes - provide the console output, description, screenshot and/or a reproducable example to show the behavior difference if possible.
If possible, please also clarify if this is a user-facing change compared to the released Hive versions or within the unreleased branches such as master.
If no, write 'No'.
-->
### How was this patch tested?
<!--
If tests were added, say they were added here. Please make sure to add some test cases that check the changes thoroughly including negative and positive cases if possible.
If it was tested in a way different from regular unit tests, please clarify how you tested step by step, ideally copy and paste-able, so that other reviewers can test and check, and descendants can verify in the future.
If tests were not added, please describe why they were not added and/or why it was difficult to add.
-->
q test and manual tests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] simhadri-g commented on a diff in pull request #4000: HIVE-27007: Iceberg: Use BasicStats from iceberg table's currrentSnap…
Posted by "simhadri-g (via GitHub)" <gi...@apache.org>.
simhadri-g commented on code in PR #4000:
URL: https://github.com/apache/hive/pull/4000#discussion_r1093299476
##########
ql/src/java/org/apache/hadoop/hive/ql/stats/StatsUtils.java:
##########
@@ -277,23 +277,31 @@ private static Statistics collectStatistics(HiveConf conf, PrunedPartitionList p
boolean metaTable = table.getMetaTable() != null;
if (!table.isPartitioned()) {
+ long ds, nr, fs;
+ if (table.isNonNative() && table.getStorageHandler().canProvideBasicStatistics()) {
+ Map<String, String> icebergBasicStatMap = table.getStorageHandler().getBasicStatistics(Partish.buildFor(table));
+ ds = Long.parseLong(icebergBasicStatMap.get("totalSize"));
+ nr = Long.parseLong(icebergBasicStatMap.get("numRows"));
+ fs = Long.parseLong(icebergBasicStatMap.get("numFiles"));
Review Comment:
Thanks @kasakrisz , i have updated the PR to catch numberFormatException and fall back to the pre existing implementation of basic stats.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] simhadri-g commented on a diff in pull request #4000: HIVE-27007: Iceberg: Use BasicStats from iceberg table's currrentSnap…
Posted by "simhadri-g (via GitHub)" <gi...@apache.org>.
simhadri-g commented on code in PR #4000:
URL: https://github.com/apache/hive/pull/4000#discussion_r1091975847
##########
ql/src/java/org/apache/hadoop/hive/ql/stats/StatsUtils.java:
##########
@@ -277,23 +277,31 @@ private static Statistics collectStatistics(HiveConf conf, PrunedPartitionList p
boolean metaTable = table.getMetaTable() != null;
if (!table.isPartitioned()) {
+ long ds, nr, fs;
Review Comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] sonarcloud[bot] commented on pull request #4000: HIVE-27007: Iceberg: Use BasicStats from iceberg table's currrentSnap…
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #4000:
URL: https://github.com/apache/hive/pull/4000#issuecomment-1412727326
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=4000)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4000&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4000&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4000&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4000&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4000&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4000&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4000&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4000&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4000&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4000&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4000&resolved=false&types=CODE_SMELL) [2 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4000&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4000&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4000&metric=duplicated_lines_density&view=list) No Duplication information
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] kasakrisz commented on a diff in pull request #4000: HIVE-27007: Iceberg: Use BasicStats from iceberg table's currrentSnap…
Posted by "kasakrisz (via GitHub)" <gi...@apache.org>.
kasakrisz commented on code in PR #4000:
URL: https://github.com/apache/hive/pull/4000#discussion_r1097173244
##########
ql/src/java/org/apache/hadoop/hive/ql/stats/StatsUtils.java:
##########
@@ -454,6 +468,27 @@ private static Statistics collectStatistics(HiveConf conf, PrunedPartitionList p
return stats;
}
+ private static Map<String, Long> getValidBasicStatsFromStorageHandler(Table table) {
+
+ Map<String, String> storageHandlerBasicStatsMap =
+ (table.isNonNative() && table.getStorageHandler().canProvideBasicStatistics()) ? table.getStorageHandler()
+ .getBasicStatistics(Partish.buildFor(table)) : null;
Review Comment:
How about moving this check and fetch into `BasicStats` and `StatsUtils.collectStatistics` don't have don't have to be changed at all?
Example: https://github.com/kasakrisz/hive/commit/e03441477a0c616f1b3012dc4ddc6dfe0f01743b
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] simhadri-g commented on a diff in pull request #4000: HIVE-27007: Iceberg: Use BasicStats from iceberg table's currrentSnap…
Posted by "simhadri-g (via GitHub)" <gi...@apache.org>.
simhadri-g commented on code in PR #4000:
URL: https://github.com/apache/hive/pull/4000#discussion_r1094837888
##########
ql/src/java/org/apache/hadoop/hive/ql/stats/StatsUtils.java:
##########
@@ -454,6 +468,26 @@ private static Statistics collectStatistics(HiveConf conf, PrunedPartitionList p
return stats;
}
+ private static Map<String, Long> getValidBasisStatsFromStorageHandler(Table table) {
Review Comment:
Thanks @akshat0395 , i have updated the PR :) .
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] simhadri-g commented on a diff in pull request #4000: HIVE-27007: Iceberg: Use BasicStats from iceberg table's currrentSnap…
Posted by "simhadri-g (via GitHub)" <gi...@apache.org>.
simhadri-g commented on code in PR #4000:
URL: https://github.com/apache/hive/pull/4000#discussion_r1095405415
##########
ql/src/java/org/apache/hadoop/hive/ql/stats/StatsUtils.java:
##########
@@ -454,6 +468,27 @@ private static Statistics collectStatistics(HiveConf conf, PrunedPartitionList p
return stats;
}
+ private static Map<String, Long> getValidBasicStatsFromStorageHandler(Table table) {
+
+ Map<String, String> storageHandlerBasicStatsMap =
+ (table.isNonNative() && table.getStorageHandler().canProvideBasicStatistics()) ? table.getStorageHandler()
+ .getBasicStatistics(Partish.buildFor(table)) : null;
+
+ try {
+ if (storageHandlerBasicStatsMap != null &&
+ Long.parseLong(storageHandlerBasicStatsMap.get(StatsSetupConst.ROW_COUNT)) > 0) {
Review Comment:
Yes, snapshots are not created for a new table until data is inserted. As a result, stats are not available.
The following line turns out to be null for the new table.
` if (table.currentSnapshot() != null) `
https://github.com/apache/hive/blob/master/iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java#L316
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] sonarcloud[bot] commented on pull request #4000: HIVE-27007: Iceberg: Use BasicStats from iceberg table's currrentSnap…
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #4000:
URL: https://github.com/apache/hive/pull/4000#issuecomment-1415421782
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=4000)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4000&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4000&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4000&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4000&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4000&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4000&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4000&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4000&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4000&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4000&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4000&resolved=false&types=CODE_SMELL) [3 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4000&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4000&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4000&metric=duplicated_lines_density&view=list) No Duplication information
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] sonarcloud[bot] commented on pull request #4000: HIVE-27007: Iceberg: Use BasicStats from iceberg table's currrentSnap…
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #4000:
URL: https://github.com/apache/hive/pull/4000#issuecomment-1424997332
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=4000)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4000&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4000&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4000&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4000&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4000&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4000&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4000&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4000&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4000&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4000&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4000&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4000&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4000&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4000&metric=duplicated_lines_density&view=list) No Duplication information
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] kasakrisz commented on a diff in pull request #4000: HIVE-27007: Iceberg: Use BasicStats from iceberg table's currrentSnap…
Posted by "kasakrisz (via GitHub)" <gi...@apache.org>.
kasakrisz commented on code in PR #4000:
URL: https://github.com/apache/hive/pull/4000#discussion_r1095356638
##########
iceberg/iceberg-handler/src/test/results/positive/use_basic_stats_from_iceberg.q.out:
##########
@@ -0,0 +1,162 @@
+PREHOOK: query: drop table if exists tbl_ice
+PREHOOK: type: DROPTABLE
+POSTHOOK: query: drop table if exists tbl_ice
+POSTHOOK: type: DROPTABLE
+PREHOOK: query: create external table tbl_ice(a int, b string, c int) stored by iceberg tblproperties ('format-version'='2')
+PREHOOK: type: CREATETABLE
+PREHOOK: Output: database:default
+PREHOOK: Output: default@tbl_ice
+POSTHOOK: query: create external table tbl_ice(a int, b string, c int) stored by iceberg tblproperties ('format-version'='2')
+POSTHOOK: type: CREATETABLE
+POSTHOOK: Output: database:default
+POSTHOOK: Output: default@tbl_ice
+PREHOOK: query: select count(*) from tbl_ice
+PREHOOK: type: QUERY
+PREHOOK: Input: default@tbl_ice
+PREHOOK: Output: hdfs://### HDFS PATH ###
+POSTHOOK: query: select count(*) from tbl_ice
+POSTHOOK: type: QUERY
+POSTHOOK: Input: default@tbl_ice
+POSTHOOK: Output: hdfs://### HDFS PATH ###
+0
+PREHOOK: query: insert into tbl_ice values (1, 'one', 50), (2, 'two', 51),(2, 'two', 51),(2, 'two', 51), (3, 'three', 52), (4, 'four', 53), (5, 'five', 54), (111, 'one', 55), (333, 'two', 56)
+PREHOOK: type: QUERY
+PREHOOK: Input: _dummy_database@_dummy_table
+PREHOOK: Output: default@tbl_ice
+POSTHOOK: query: insert into tbl_ice values (1, 'one', 50), (2, 'two', 51),(2, 'two', 51),(2, 'two', 51), (3, 'three', 52), (4, 'four', 53), (5, 'five', 54), (111, 'one', 55), (333, 'two', 56)
+POSTHOOK: type: QUERY
+POSTHOOK: Input: _dummy_database@_dummy_table
+POSTHOOK: Output: default@tbl_ice
+PREHOOK: query: explain select * from tbl_ice order by a, b, c
+PREHOOK: type: QUERY
+PREHOOK: Input: default@tbl_ice
+PREHOOK: Output: hdfs://### HDFS PATH ###
+POSTHOOK: query: explain select * from tbl_ice order by a, b, c
+POSTHOOK: type: QUERY
+POSTHOOK: Input: default@tbl_ice
+POSTHOOK: Output: hdfs://### HDFS PATH ###
+Plan optimized by CBO.
+
+Vertex dependency in root stage
+Reducer 2 <- Map 1 (SIMPLE_EDGE)
+
+Stage-0
+ Fetch Operator
+ limit:-1
+ Stage-1
+ Reducer 2 vectorized
+ File Output Operator [FS_8]
+ Select Operator [SEL_7] (rows=9 width=95)
Review Comment:
Please add an other test case when joining empty tables or at least one of the tables are empty.
##########
ql/src/java/org/apache/hadoop/hive/ql/stats/StatsUtils.java:
##########
@@ -454,6 +468,27 @@ private static Statistics collectStatistics(HiveConf conf, PrunedPartitionList p
return stats;
}
+ private static Map<String, Long> getValidBasicStatsFromStorageHandler(Table table) {
+
+ Map<String, String> storageHandlerBasicStatsMap =
+ (table.isNonNative() && table.getStorageHandler().canProvideBasicStatistics()) ? table.getStorageHandler()
+ .getBasicStatistics(Partish.buildFor(table)) : null;
+
+ try {
+ if (storageHandlerBasicStatsMap != null &&
+ Long.parseLong(storageHandlerBasicStatsMap.get(StatsSetupConst.ROW_COUNT)) > 0) {
Review Comment:
Does it mean that an empty table has no stats?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] InvisibleProgrammer commented on pull request #4000: HIVE-27007: Iceberg: Use BasicStats from iceberg table's currrentSnap…
Posted by "InvisibleProgrammer (via GitHub)" <gi...@apache.org>.
InvisibleProgrammer commented on PR #4000:
URL: https://github.com/apache/hive/pull/4000#issuecomment-1415962968
Now we use the current snapshot's statistics, by default. Is that means in case of time travel queries we use the current snapshot statistics as too? I assume we have no better solution, I think Hive has no time travel statistics as well.
Anyway, LGTM.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] simhadri-g commented on a diff in pull request #4000: HIVE-27007: Iceberg: Use BasicStats from iceberg table's currrentSnap…
Posted by "simhadri-g (via GitHub)" <gi...@apache.org>.
simhadri-g commented on code in PR #4000:
URL: https://github.com/apache/hive/pull/4000#discussion_r1101495223
##########
ql/src/java/org/apache/hadoop/hive/ql/stats/StatsUtils.java:
##########
@@ -454,6 +468,27 @@ private static Statistics collectStatistics(HiveConf conf, PrunedPartitionList p
return stats;
}
+ private static Map<String, Long> getValidBasicStatsFromStorageHandler(Table table) {
+
+ Map<String, String> storageHandlerBasicStatsMap =
+ (table.isNonNative() && table.getStorageHandler().canProvideBasicStatistics()) ? table.getStorageHandler()
+ .getBasicStatistics(Partish.buildFor(table)) : null;
Review Comment:
Done.
##########
iceberg/iceberg-handler/src/test/queries/positive/use_basic_stats_from_iceberg.q:
##########
@@ -0,0 +1,32 @@
+-- Mask random uuid
+--! qt:replace:/(\s+uuid\s+)\S+(\s*)/$1#Masked#$2/
+set hive.stats.autogather=true;
+set hive.stats.column.autogather=true;
+
+drop table if exists t1 ;
+drop table if exists t2 ;
+create table t1 (a int) stored by iceberg tblproperties ('format-version'='2');
+create table t2 (b int) stored by iceberg tblproperties ('format-version'='2');
+describe formatted t1;
+describe formatted t2;
+explain select * from t1 join t2 on t1.a = t2.b;
+
+drop table if exists tbl_ice;
+create external table tbl_ice(a int, b string, c int) stored by iceberg tblproperties ('format-version'='2');
+explain select * from tbl_ice order by a, b, c;
+select count(*) from tbl_ice ;
+insert into tbl_ice values (1, 'one', 50), (2, 'two', 51),(2, 'two', 51),(2, 'two', 51), (3, 'three', 52), (4, 'four', 53), (5, 'five', 54), (111, 'one', 55), (333, 'two', 56);
+
+explain select * from tbl_ice order by a, b, c;
+select * from tbl_ice order by a, b, c;
+select count(*) from tbl_ice ;
+
+insert into tbl_ice values (1, 'one', 50), (2, 'two', 51),(2, 'two', 51),(2, 'two', 51), (3, 'three', 52), (4, 'four', 53), (5, 'five', 54), (111, 'one', 55), (333, 'two', 56);
+explain select * from tbl_ice order by a, b, c;
+select count(*) from tbl_ice ;
+
+drop table if exists tbl_orc;
+create external table tbl_orc(a int, b string, c int) stored as orc tblproperties ('format-version'='2');
+insert into tbl_orc values (1, 'one', 50), (2, 'two', 51),(2, 'two', 51),(2, 'two', 51), (3, 'three', 52), (4, 'four',
+53), (5, 'five', 54), (111, 'one', 55), (333, 'two', 56);
+select count(*) from tbl_orc ;
Review Comment:
done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] kasakrisz commented on a diff in pull request #4000: HIVE-27007: Iceberg: Use BasicStats from iceberg table's currrentSnap…
Posted by "kasakrisz (via GitHub)" <gi...@apache.org>.
kasakrisz commented on code in PR #4000:
URL: https://github.com/apache/hive/pull/4000#discussion_r1091850767
##########
ql/src/java/org/apache/hadoop/hive/ql/stats/StatsUtils.java:
##########
@@ -277,23 +277,31 @@ private static Statistics collectStatistics(HiveConf conf, PrunedPartitionList p
boolean metaTable = table.getMetaTable() != null;
if (!table.isPartitioned()) {
+ long ds, nr, fs;
+ if (table.isNonNative() && table.getStorageHandler().canProvideBasicStatistics()) {
+ Map<String, String> icebergBasicStatMap = table.getStorageHandler().getBasicStatistics(Partish.buildFor(table));
+ ds = Long.parseLong(icebergBasicStatMap.get("totalSize"));
+ nr = Long.parseLong(icebergBasicStatMap.get("numRows"));
+ fs = Long.parseLong(icebergBasicStatMap.get("numFiles"));
Review Comment:
Should a null check be added here and/or handle NumberFormatException?
What would be the stats values if it can not be extracted via storage handler?
In case of non-storage handler path there are so called `enhancers` to estimate and fix the values:
```
basicStatsFactory.addEnhancer(new BasicStats.RowNumEstimator(estimateRowSizeFromSchema(conf, schema)));
basicStatsFactory.addEnhancer(new BasicStats.SetMinRowNumber01());
```
##########
ql/src/java/org/apache/hadoop/hive/ql/stats/StatsUtils.java:
##########
@@ -277,23 +277,31 @@ private static Statistics collectStatistics(HiveConf conf, PrunedPartitionList p
boolean metaTable = table.getMetaTable() != null;
if (!table.isPartitioned()) {
+ long ds, nr, fs;
Review Comment:
Could you please declare each variable in separate lines.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] simhadri-g commented on a diff in pull request #4000: HIVE-27007: Iceberg: Use BasicStats from iceberg table's currrentSnap…
Posted by "simhadri-g (via GitHub)" <gi...@apache.org>.
simhadri-g commented on code in PR #4000:
URL: https://github.com/apache/hive/pull/4000#discussion_r1095407711
##########
iceberg/iceberg-handler/src/test/results/positive/use_basic_stats_from_iceberg.q.out:
##########
@@ -0,0 +1,162 @@
+PREHOOK: query: drop table if exists tbl_ice
+PREHOOK: type: DROPTABLE
+POSTHOOK: query: drop table if exists tbl_ice
+POSTHOOK: type: DROPTABLE
+PREHOOK: query: create external table tbl_ice(a int, b string, c int) stored by iceberg tblproperties ('format-version'='2')
+PREHOOK: type: CREATETABLE
+PREHOOK: Output: database:default
+PREHOOK: Output: default@tbl_ice
+POSTHOOK: query: create external table tbl_ice(a int, b string, c int) stored by iceberg tblproperties ('format-version'='2')
+POSTHOOK: type: CREATETABLE
+POSTHOOK: Output: database:default
+POSTHOOK: Output: default@tbl_ice
+PREHOOK: query: select count(*) from tbl_ice
+PREHOOK: type: QUERY
+PREHOOK: Input: default@tbl_ice
+PREHOOK: Output: hdfs://### HDFS PATH ###
+POSTHOOK: query: select count(*) from tbl_ice
+POSTHOOK: type: QUERY
+POSTHOOK: Input: default@tbl_ice
+POSTHOOK: Output: hdfs://### HDFS PATH ###
+0
+PREHOOK: query: insert into tbl_ice values (1, 'one', 50), (2, 'two', 51),(2, 'two', 51),(2, 'two', 51), (3, 'three', 52), (4, 'four', 53), (5, 'five', 54), (111, 'one', 55), (333, 'two', 56)
+PREHOOK: type: QUERY
+PREHOOK: Input: _dummy_database@_dummy_table
+PREHOOK: Output: default@tbl_ice
+POSTHOOK: query: insert into tbl_ice values (1, 'one', 50), (2, 'two', 51),(2, 'two', 51),(2, 'two', 51), (3, 'three', 52), (4, 'four', 53), (5, 'five', 54), (111, 'one', 55), (333, 'two', 56)
+POSTHOOK: type: QUERY
+POSTHOOK: Input: _dummy_database@_dummy_table
+POSTHOOK: Output: default@tbl_ice
+PREHOOK: query: explain select * from tbl_ice order by a, b, c
+PREHOOK: type: QUERY
+PREHOOK: Input: default@tbl_ice
+PREHOOK: Output: hdfs://### HDFS PATH ###
+POSTHOOK: query: explain select * from tbl_ice order by a, b, c
+POSTHOOK: type: QUERY
+POSTHOOK: Input: default@tbl_ice
+POSTHOOK: Output: hdfs://### HDFS PATH ###
+Plan optimized by CBO.
+
+Vertex dependency in root stage
+Reducer 2 <- Map 1 (SIMPLE_EDGE)
+
+Stage-0
+ Fetch Operator
+ limit:-1
+ Stage-1
+ Reducer 2 vectorized
+ File Output Operator [FS_8]
+ Select Operator [SEL_7] (rows=9 width=95)
Review Comment:
Sure.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] soumyakanti3578 commented on a diff in pull request #4000: HIVE-27007: Iceberg: Use BasicStats from iceberg table's currrentSnap…
Posted by "soumyakanti3578 (via GitHub)" <gi...@apache.org>.
soumyakanti3578 commented on code in PR #4000:
URL: https://github.com/apache/hive/pull/4000#discussion_r1097809424
##########
iceberg/iceberg-handler/src/test/queries/positive/use_basic_stats_from_iceberg.q:
##########
@@ -0,0 +1,32 @@
+-- Mask random uuid
+--! qt:replace:/(\s+uuid\s+)\S+(\s*)/$1#Masked#$2/
+set hive.stats.autogather=true;
+set hive.stats.column.autogather=true;
+
+drop table if exists t1 ;
+drop table if exists t2 ;
+create table t1 (a int) stored by iceberg tblproperties ('format-version'='2');
+create table t2 (b int) stored by iceberg tblproperties ('format-version'='2');
+describe formatted t1;
+describe formatted t2;
+explain select * from t1 join t2 on t1.a = t2.b;
+
+drop table if exists tbl_ice;
+create external table tbl_ice(a int, b string, c int) stored by iceberg tblproperties ('format-version'='2');
+explain select * from tbl_ice order by a, b, c;
+select count(*) from tbl_ice ;
+insert into tbl_ice values (1, 'one', 50), (2, 'two', 51),(2, 'two', 51),(2, 'two', 51), (3, 'three', 52), (4, 'four', 53), (5, 'five', 54), (111, 'one', 55), (333, 'two', 56);
+
+explain select * from tbl_ice order by a, b, c;
+select * from tbl_ice order by a, b, c;
+select count(*) from tbl_ice ;
+
+insert into tbl_ice values (1, 'one', 50), (2, 'two', 51),(2, 'two', 51),(2, 'two', 51), (3, 'three', 52), (4, 'four', 53), (5, 'five', 54), (111, 'one', 55), (333, 'two', 56);
+explain select * from tbl_ice order by a, b, c;
+select count(*) from tbl_ice ;
+
+drop table if exists tbl_orc;
+create external table tbl_orc(a int, b string, c int) stored as orc tblproperties ('format-version'='2');
+insert into tbl_orc values (1, 'one', 50), (2, 'two', 51),(2, 'two', 51),(2, 'two', 51), (3, 'three', 52), (4, 'four',
+53), (5, 'five', 54), (111, 'one', 55), (333, 'two', 56);
+select count(*) from tbl_orc ;
Review Comment:
nit: please add a new line at eof.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] sonarcloud[bot] commented on pull request #4000: HIVE-27007: Iceberg: Use BasicStats from iceberg table's currrentSnap…
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #4000:
URL: https://github.com/apache/hive/pull/4000#issuecomment-1424699612
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=4000)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4000&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4000&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4000&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4000&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4000&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4000&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4000&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4000&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4000&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4000&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4000&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4000&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4000&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4000&metric=duplicated_lines_density&view=list) No Duplication information
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] simhadri-g commented on a diff in pull request #4000: HIVE-27007: Iceberg: Use BasicStats from iceberg table's currrentSnap…
Posted by "simhadri-g (via GitHub)" <gi...@apache.org>.
simhadri-g commented on code in PR #4000:
URL: https://github.com/apache/hive/pull/4000#discussion_r1093299476
##########
ql/src/java/org/apache/hadoop/hive/ql/stats/StatsUtils.java:
##########
@@ -277,23 +277,31 @@ private static Statistics collectStatistics(HiveConf conf, PrunedPartitionList p
boolean metaTable = table.getMetaTable() != null;
if (!table.isPartitioned()) {
+ long ds, nr, fs;
+ if (table.isNonNative() && table.getStorageHandler().canProvideBasicStatistics()) {
+ Map<String, String> icebergBasicStatMap = table.getStorageHandler().getBasicStatistics(Partish.buildFor(table));
+ ds = Long.parseLong(icebergBasicStatMap.get("totalSize"));
+ nr = Long.parseLong(icebergBasicStatMap.get("numRows"));
+ fs = Long.parseLong(icebergBasicStatMap.get("numFiles"));
Review Comment:
Thanks, @kasakrisz , I have updated the PR to catch numberFormatException and fall back to the pre-existing implementation of basic stats in case the storage handler cannot provide the stats.
Please have a look at the updated PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] sonarcloud[bot] commented on pull request #4000: HIVE-27007: Iceberg: Use BasicStats from iceberg table's currrentSnap…
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #4000:
URL: https://github.com/apache/hive/pull/4000#issuecomment-1415846127
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=4000)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4000&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4000&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4000&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4000&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4000&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4000&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4000&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4000&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4000&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4000&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4000&resolved=false&types=CODE_SMELL) [9 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4000&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4000&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4000&metric=duplicated_lines_density&view=list) No Duplication information
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] akshat0395 commented on a diff in pull request #4000: HIVE-27007: Iceberg: Use BasicStats from iceberg table's currrentSnap…
Posted by "akshat0395 (via GitHub)" <gi...@apache.org>.
akshat0395 commented on code in PR #4000:
URL: https://github.com/apache/hive/pull/4000#discussion_r1093602583
##########
ql/src/java/org/apache/hadoop/hive/ql/stats/StatsUtils.java:
##########
@@ -454,6 +468,26 @@ private static Statistics collectStatistics(HiveConf conf, PrunedPartitionList p
return stats;
}
+ private static Map<String, Long> getValidBasisStatsFromStorageHandler(Table table) {
Review Comment:
Seems like a typo in method name, Should it be `getValidBasicStatsFromStorageHandler`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] sonarcloud[bot] commented on pull request #4000: HIVE-27007: Iceberg: Use BasicStats from iceberg table's currrentSnap…
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #4000:
URL: https://github.com/apache/hive/pull/4000#issuecomment-1413945063
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=4000)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4000&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4000&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4000&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4000&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4000&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4000&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4000&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4000&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4000&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4000&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4000&resolved=false&types=CODE_SMELL) [2 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4000&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4000&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4000&metric=duplicated_lines_density&view=list) No Duplication information
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] kasakrisz commented on a diff in pull request #4000: HIVE-27007: Iceberg: Use BasicStats from iceberg table's currrentSnap…
Posted by "kasakrisz (via GitHub)" <gi...@apache.org>.
kasakrisz commented on code in PR #4000:
URL: https://github.com/apache/hive/pull/4000#discussion_r1092068769
##########
ql/src/java/org/apache/hadoop/hive/ql/stats/StatsUtils.java:
##########
@@ -277,23 +277,31 @@ private static Statistics collectStatistics(HiveConf conf, PrunedPartitionList p
boolean metaTable = table.getMetaTable() != null;
if (!table.isPartitioned()) {
+ long ds, nr, fs;
+ if (table.isNonNative() && table.getStorageHandler().canProvideBasicStatistics()) {
+ Map<String, String> icebergBasicStatMap = table.getStorageHandler().getBasicStatistics(Partish.buildFor(table));
+ ds = Long.parseLong(icebergBasicStatMap.get("totalSize"));
+ nr = Long.parseLong(icebergBasicStatMap.get("numRows"));
+ fs = Long.parseLong(icebergBasicStatMap.get("numFiles"));
Review Comment:
1. It seems to me that it check if the currentSnapshot exists or not but what if summary does not contain for example `SnapshotSummary.TOTAL_DATA_FILES_PROP`.
2. The line
```
Long.parseLong(icebergBasicStatMap.get("totalSize"))
```
can throw `NumberFormatException` because
* `icebergBasicStatMap.get("totalSize")` returns null because the map does have an entry with `totalSize`
* or it has such entry but the value is not a valid `Long` value.
3. I think the name `icebergBasicStatMap` is too specific here because at this point we should not expect an `IcebergStorageHandler` but any `HiveStorageHandler` which supports providing BasicStatistics. So a name like `basicStatMap` would be more generic.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] sonarcloud[bot] commented on pull request #4000: HIVE-27007: Iceberg: Use BasicStats from iceberg table's currrentSnap…
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #4000:
URL: https://github.com/apache/hive/pull/4000#issuecomment-1425286954
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=4000)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4000&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4000&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4000&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4000&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4000&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4000&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4000&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4000&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4000&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4000&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4000&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4000&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4000&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4000&metric=duplicated_lines_density&view=list) No Duplication information
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] kasakrisz merged pull request #4000: HIVE-27007: Iceberg: Use BasicStats from iceberg table's currrentSnap…
Posted by "kasakrisz (via GitHub)" <gi...@apache.org>.
kasakrisz merged PR #4000:
URL: https://github.com/apache/hive/pull/4000
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] simhadri-g commented on a diff in pull request #4000: HIVE-27007: Iceberg: Use BasicStats from iceberg table's currrentSnap…
Posted by "simhadri-g (via GitHub)" <gi...@apache.org>.
simhadri-g commented on code in PR #4000:
URL: https://github.com/apache/hive/pull/4000#discussion_r1101492692
##########
ql/src/java/org/apache/hadoop/hive/ql/stats/StatsUtils.java:
##########
@@ -454,6 +468,27 @@ private static Statistics collectStatistics(HiveConf conf, PrunedPartitionList p
return stats;
}
+ private static Map<String, Long> getValidBasicStatsFromStorageHandler(Table table) {
+
+ Map<String, String> storageHandlerBasicStatsMap =
+ (table.isNonNative() && table.getStorageHandler().canProvideBasicStatistics()) ? table.getStorageHandler()
+ .getBasicStatistics(Partish.buildFor(table)) : null;
Review Comment:
Done.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] InvisibleProgrammer commented on pull request #4000: HIVE-27007: Iceberg: Use BasicStats from iceberg table's currrentSnap…
Posted by "InvisibleProgrammer (via GitHub)" <gi...@apache.org>.
InvisibleProgrammer commented on PR #4000:
URL: https://github.com/apache/hive/pull/4000#issuecomment-1416015458
Nice, thank you.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] simhadri-g commented on a diff in pull request #4000: HIVE-27007: Iceberg: Use BasicStats from iceberg table's currrentSnap…
Posted by "simhadri-g (via GitHub)" <gi...@apache.org>.
simhadri-g commented on code in PR #4000:
URL: https://github.com/apache/hive/pull/4000#discussion_r1091920961
##########
ql/src/java/org/apache/hadoop/hive/ql/stats/StatsUtils.java:
##########
@@ -277,23 +277,31 @@ private static Statistics collectStatistics(HiveConf conf, PrunedPartitionList p
boolean metaTable = table.getMetaTable() != null;
if (!table.isPartitioned()) {
+ long ds, nr, fs;
+ if (table.isNonNative() && table.getStorageHandler().canProvideBasicStatistics()) {
+ Map<String, String> icebergBasicStatMap = table.getStorageHandler().getBasicStatistics(Partish.buildFor(table));
+ ds = Long.parseLong(icebergBasicStatMap.get("totalSize"));
+ nr = Long.parseLong(icebergBasicStatMap.get("numRows"));
+ fs = Long.parseLong(icebergBasicStatMap.get("numFiles"));
Review Comment:
Thanks for the review :) .
I thought that "null" check here would be sufficient:
https://github.com/apache/hive/blob/master/iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java#L316
I will update the PR and include a null check.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] simhadri-g commented on pull request #4000: HIVE-27007: Iceberg: Use BasicStats from iceberg table's currrentSnap…
Posted by "simhadri-g (via GitHub)" <gi...@apache.org>.
simhadri-g commented on PR #4000:
URL: https://github.com/apache/hive/pull/4000#issuecomment-1415983594
@InvisibleProgrammer , on time-travel the current snapshot id will change to the snapshot we are rolling back to. So currentSnapshot summary will provide the correct stats.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org