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/03/20 20:50:16 UTC
[GitHub] [hive] simhadri-g opened a new pull request, #4131: HIVE-27158: Store hive columns stats in puffin files for iceberg tables
simhadri-g opened a new pull request, #4131:
URL: https://github.com/apache/hive/pull/4131
<!--
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.
-->
--
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 #4131: HIVE-27158: Store hive columns stats in puffin files for iceberg tables
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #4131:
URL: https://github.com/apache/hive/pull/4131#issuecomment-1483846737
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=4131)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4131&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4131&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4131&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=CODE_SMELL) [6 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4131&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4131&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] deniskuzZ commented on a diff in pull request #4131: HIVE-27158: Store hive columns stats in puffin files for iceberg tables
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #4131:
URL: https://github.com/apache/hive/pull/4131#discussion_r1148989152
##########
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java:
##########
@@ -2207,6 +2207,8 @@ public static enum ConfVars {
"Whether to use codec pool in ORC. Disable if there are bugs with codec reuse."),
HIVE_USE_STATS_FROM("hive.use.stats.from","iceberg","Use stats from iceberg table snapshot for query " +
"planning. This has three values metastore, puffin and iceberg"),
+ HIVE_COL_STATS_SOURCE("hive.col.stats.source","metastore","Use stats from puffin file for query " +
Review Comment:
- do we support `iceberg` mode?
- please update the description - "Use stats from selected source for query planning"
##########
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java:
##########
@@ -2207,6 +2207,8 @@ public static enum ConfVars {
"Whether to use codec pool in ORC. Disable if there are bugs with codec reuse."),
HIVE_USE_STATS_FROM("hive.use.stats.from","iceberg","Use stats from iceberg table snapshot for query " +
"planning. This has three values metastore, puffin and iceberg"),
+ HIVE_COL_STATS_SOURCE("hive.col.stats.source","metastore","Use stats from puffin file for query " +
Review Comment:
- do we support `iceberg` mode?
- please update the description - "Use stats from the selected source for query planning"
--
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] deniskuzZ commented on a diff in pull request #4131: HIVE-27158: Store hive columns stats in puffin files for iceberg tables
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #4131:
URL: https://github.com/apache/hive/pull/4131#discussion_r1149025872
##########
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java:
##########
@@ -349,6 +365,96 @@ public Map<String, String> getBasicStatistics(Partish partish) {
return stats;
}
+
+ @Override
+ public boolean canSetColStatistics() {
+ String statsSource = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_USE_STATS_FROM).toLowerCase();
+ return statsSource.equals(PUFFIN);
+ }
+
+ @Override
+ public boolean canProvideColStatistics(org.apache.hadoop.hive.ql.metadata.Table tbl) {
+
+ org.apache.hadoop.hive.ql.metadata.Table hmsTable = tbl;
+ TableDesc tableDesc = Utilities.getTableDesc(hmsTable);
+ Table table = Catalogs.loadTable(conf, tableDesc.getProperties());
+ if (table.currentSnapshot() != null) {
+ String statsSource = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_COL_STATS_SOURCE).toLowerCase();
+ String statsPath = table.location() + STATS + table.name() + table.currentSnapshot().snapshotId();
+ if (statsSource.equals(PUFFIN)) {
+ try (FileSystem fs = new Path(table.location()).getFileSystem(conf)) {
+ if (fs.exists(new Path(statsPath))) {
+ return true;
+ }
+ } catch (IOException e) {
+ LOG.warn(e.getMessage());
+ }
+ }
+ }
+ return false;
+ }
+
+ @Override
+ public List<ColumnStatisticsObj> getColStatistics(org.apache.hadoop.hive.ql.metadata.Table tbl) {
+
+ org.apache.hadoop.hive.ql.metadata.Table hmsTable = tbl;
+ TableDesc tableDesc = Utilities.getTableDesc(hmsTable);
+ Table table = Catalogs.loadTable(conf, tableDesc.getProperties());
+ String statsSource = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_COL_STATS_SOURCE).toLowerCase();
+ switch (statsSource) {
+ case ICEBERG:
+ // Place holder for iceberg stats
+ break;
+ case PUFFIN:
+ String snapshotId = table.name() + table.currentSnapshot().snapshotId();
+ String statsPath = table.location() + STATS + snapshotId;
+ LOG.info("Using stats from puffin file at:" + statsPath);
+ try (PuffinReader reader = Puffin.read(table.io().newInputFile(statsPath)).build()) {
+ BlobMetadata blobMetadata = reader.fileMetadata().blobs().get(0);
Review Comment:
could it be empty (IndexOutOfBoundsException)?
--
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 #4131: HIVE-27158: Store hive columns stats in puffin files for iceberg tables
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #4131:
URL: https://github.com/apache/hive/pull/4131#issuecomment-1483680009
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=4131)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4131&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4131&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4131&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=CODE_SMELL) [6 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4131&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4131&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] deniskuzZ commented on a diff in pull request #4131: HIVE-27158: Store hive columns stats in puffin files for iceberg tables
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #4131:
URL: https://github.com/apache/hive/pull/4131#discussion_r1149016455
##########
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java:
##########
@@ -349,6 +365,96 @@ public Map<String, String> getBasicStatistics(Partish partish) {
return stats;
}
+
+ @Override
+ public boolean canSetColStatistics() {
+ String statsSource = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_USE_STATS_FROM).toLowerCase();
+ return statsSource.equals(PUFFIN);
+ }
+
+ @Override
+ public boolean canProvideColStatistics(org.apache.hadoop.hive.ql.metadata.Table tbl) {
+
+ org.apache.hadoop.hive.ql.metadata.Table hmsTable = tbl;
+ TableDesc tableDesc = Utilities.getTableDesc(hmsTable);
+ Table table = Catalogs.loadTable(conf, tableDesc.getProperties());
+ if (table.currentSnapshot() != null) {
+ String statsSource = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_COL_STATS_SOURCE).toLowerCase();
+ String statsPath = table.location() + STATS + table.name() + table.currentSnapshot().snapshotId();
+ if (statsSource.equals(PUFFIN)) {
+ try (FileSystem fs = new Path(table.location()).getFileSystem(conf)) {
+ if (fs.exists(new Path(statsPath))) {
+ return true;
+ }
+ } catch (IOException e) {
+ LOG.warn(e.getMessage());
+ }
+ }
+ }
+ return false;
+ }
+
+ @Override
+ public List<ColumnStatisticsObj> getColStatistics(org.apache.hadoop.hive.ql.metadata.Table tbl) {
+
+ org.apache.hadoop.hive.ql.metadata.Table hmsTable = tbl;
Review Comment:
why is this needed?
--
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 a diff in pull request #4131: HIVE-27158: Store hive columns stats in puffin files for iceberg tables
Posted by "InvisibleProgrammer (via GitHub)" <gi...@apache.org>.
InvisibleProgrammer commented on code in PR #4131:
URL: https://github.com/apache/hive/pull/4131#discussion_r1165156528
##########
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java:
##########
@@ -2205,9 +2205,8 @@ public static enum ConfVars {
"padding tolerance config (hive.exec.orc.block.padding.tolerance)."),
HIVE_ORC_CODEC_POOL("hive.use.orc.codec.pool", false,
"Whether to use codec pool in ORC. Disable if there are bugs with codec reuse."),
- HIVE_USE_STATS_FROM("hive.use.stats.from","iceberg","Use stats from iceberg table snapshot for query " +
- "planning. This has three values metastore, puffin and iceberg"),
-
+ HIVE_ICEBERG_STATS_SOURCE("hive.iceberg.stats.source","iceberg","Use stats from iceberg " +
Review Comment:
I have found a better way to handle this:
If you check that example, it not only defines the possible values, but also ensures the casing will be as it is expected. So that, you don't need to call `toLowerCase()` method when you read the value.
``` java
// setting up execution engine
HIVE_EXECUTION_ENGINE("hive.execution.engine", Runtime.TEZ.toString(),
new StringSet(true, Runtime.MR.toString(), Runtime.TEZ.toString(),
Runtime.IMPALA.toString()),
// and reading the value
if (HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_EXECUTION_ENGINE).equals("tez")) {
```
--
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 #4131: HIVE-27158: Store hive columns stats in puffin files for iceberg tables
Posted by "simhadri-g (via GitHub)" <gi...@apache.org>.
simhadri-g commented on code in PR #4131:
URL: https://github.com/apache/hive/pull/4131#discussion_r1161856412
##########
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java:
##########
@@ -361,6 +378,83 @@ private Table getTable(org.apache.hadoop.hive.ql.metadata.Table hmsTable) {
return table;
}
+
+ @Override
+ public boolean canSetColStatistics() {
+ return getStatsSource().equals(ICEBERG);
+ }
+
+ @Override
+ public boolean canProvideColStatistics(org.apache.hadoop.hive.ql.metadata.Table hmsTable) {
+ Table table = Catalogs.loadTable(conf, Utilities.getTableDesc(hmsTable).getProperties());
+ if (table.currentSnapshot() != null) {
+ Path statsPath = getStatsPath(table);
+ if (getStatsSource().equals(ICEBERG)) {
+ try (FileSystem fs = statsPath.getFileSystem(conf)) {
+ if (fs.exists(statsPath)) {
+ return true;
+ }
+ } catch (IOException e) {
+ LOG.warn(e.getMessage());
+ }
+ }
+ }
+ return false;
+ }
+
+ @Override
+ public List<ColumnStatisticsObj> getColStatistics(org.apache.hadoop.hive.ql.metadata.Table hmsTable) {
+ Table table = Catalogs.loadTable(conf, Utilities.getTableDesc(hmsTable).getProperties());
+ String statsPath = getStatsPath(table).toString();
+ LOG.info("Using stats from puffin file at:" + statsPath);
+ try (PuffinReader reader = Puffin.read(table.io().newInputFile(statsPath)).build()) {
+ List<BlobMetadata> blobMetadata = reader.fileMetadata().blobs();
+ Map<BlobMetadata, List<ColumnStatistics>> collect =
+ Streams.stream(reader.readAll(blobMetadata)).collect(Collectors.toMap(Pair::first,
+ blobMetadataByteBufferPair -> SerializationUtils.deserialize(
+ ByteBuffers.toByteArray(blobMetadataByteBufferPair.second()))));
+ return collect.get(blobMetadata.get(0)).get(0).getStatsObj();
+ } catch (IOException e) {
+ LOG.error(String.valueOf(e));
+ }
+ return null;
Review Comment:
Even in the absence of stats the query can run successfully.
I was thinking it would be better to throw an error message rather than filing the entire query.
##########
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java:
##########
@@ -361,6 +378,83 @@ private Table getTable(org.apache.hadoop.hive.ql.metadata.Table hmsTable) {
return table;
}
+
+ @Override
+ public boolean canSetColStatistics() {
+ return getStatsSource().equals(ICEBERG);
+ }
+
+ @Override
+ public boolean canProvideColStatistics(org.apache.hadoop.hive.ql.metadata.Table hmsTable) {
+ Table table = Catalogs.loadTable(conf, Utilities.getTableDesc(hmsTable).getProperties());
+ if (table.currentSnapshot() != null) {
+ Path statsPath = getStatsPath(table);
+ if (getStatsSource().equals(ICEBERG)) {
+ try (FileSystem fs = statsPath.getFileSystem(conf)) {
+ if (fs.exists(statsPath)) {
+ return true;
+ }
+ } catch (IOException e) {
+ LOG.warn(e.getMessage());
+ }
+ }
+ }
+ return false;
+ }
+
+ @Override
+ public List<ColumnStatisticsObj> getColStatistics(org.apache.hadoop.hive.ql.metadata.Table hmsTable) {
+ Table table = Catalogs.loadTable(conf, Utilities.getTableDesc(hmsTable).getProperties());
+ String statsPath = getStatsPath(table).toString();
+ LOG.info("Using stats from puffin file at:" + statsPath);
+ try (PuffinReader reader = Puffin.read(table.io().newInputFile(statsPath)).build()) {
+ List<BlobMetadata> blobMetadata = reader.fileMetadata().blobs();
+ Map<BlobMetadata, List<ColumnStatistics>> collect =
+ Streams.stream(reader.readAll(blobMetadata)).collect(Collectors.toMap(Pair::first,
+ blobMetadataByteBufferPair -> SerializationUtils.deserialize(
+ ByteBuffers.toByteArray(blobMetadataByteBufferPair.second()))));
+ return collect.get(blobMetadata.get(0)).get(0).getStatsObj();
+ } catch (IOException e) {
+ LOG.error(String.valueOf(e));
+ }
+ return null;
+ }
+
+
+ @Override
+ public boolean setColStatistics(org.apache.hadoop.hive.ql.metadata.Table table,
+ List<ColumnStatistics> colStats) {
+ TableDesc tableDesc = Utilities.getTableDesc(table);
+ Table tbl = Catalogs.loadTable(conf, tableDesc.getProperties());
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] simhadri-g commented on a diff in pull request #4131: HIVE-27158: Store hive columns stats in puffin files for iceberg tables
Posted by "simhadri-g (via GitHub)" <gi...@apache.org>.
simhadri-g commented on code in PR #4131:
URL: https://github.com/apache/hive/pull/4131#discussion_r1161856243
##########
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java:
##########
@@ -361,6 +378,83 @@ private Table getTable(org.apache.hadoop.hive.ql.metadata.Table hmsTable) {
return table;
}
+
+ @Override
+ public boolean canSetColStatistics() {
+ return getStatsSource().equals(ICEBERG);
+ }
+
+ @Override
+ public boolean canProvideColStatistics(org.apache.hadoop.hive.ql.metadata.Table hmsTable) {
+ Table table = Catalogs.loadTable(conf, Utilities.getTableDesc(hmsTable).getProperties());
+ if (table.currentSnapshot() != null) {
+ Path statsPath = getStatsPath(table);
+ if (getStatsSource().equals(ICEBERG)) {
+ try (FileSystem fs = statsPath.getFileSystem(conf)) {
+ if (fs.exists(statsPath)) {
+ return true;
+ }
+ } catch (IOException e) {
+ LOG.warn(e.getMessage());
Review Comment:
Done.
##########
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java:
##########
@@ -361,6 +378,83 @@ private Table getTable(org.apache.hadoop.hive.ql.metadata.Table hmsTable) {
return table;
}
+
+ @Override
+ public boolean canSetColStatistics() {
+ return getStatsSource().equals(ICEBERG);
+ }
+
+ @Override
+ public boolean canProvideColStatistics(org.apache.hadoop.hive.ql.metadata.Table hmsTable) {
+ Table table = Catalogs.loadTable(conf, Utilities.getTableDesc(hmsTable).getProperties());
+ if (table.currentSnapshot() != null) {
+ Path statsPath = getStatsPath(table);
+ if (getStatsSource().equals(ICEBERG)) {
+ try (FileSystem fs = statsPath.getFileSystem(conf)) {
+ if (fs.exists(statsPath)) {
+ return true;
+ }
+ } catch (IOException e) {
+ LOG.warn(e.getMessage());
+ }
+ }
+ }
+ return false;
+ }
+
+ @Override
+ public List<ColumnStatisticsObj> getColStatistics(org.apache.hadoop.hive.ql.metadata.Table hmsTable) {
+ Table table = Catalogs.loadTable(conf, Utilities.getTableDesc(hmsTable).getProperties());
Review Comment:
Done
##########
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java:
##########
@@ -361,6 +378,83 @@ private Table getTable(org.apache.hadoop.hive.ql.metadata.Table hmsTable) {
return table;
}
+
+ @Override
+ public boolean canSetColStatistics() {
+ return getStatsSource().equals(ICEBERG);
+ }
+
+ @Override
+ public boolean canProvideColStatistics(org.apache.hadoop.hive.ql.metadata.Table hmsTable) {
+ Table table = Catalogs.loadTable(conf, Utilities.getTableDesc(hmsTable).getProperties());
+ if (table.currentSnapshot() != null) {
+ Path statsPath = getStatsPath(table);
+ if (getStatsSource().equals(ICEBERG)) {
+ try (FileSystem fs = statsPath.getFileSystem(conf)) {
+ if (fs.exists(statsPath)) {
+ return true;
+ }
+ } catch (IOException e) {
+ LOG.warn(e.getMessage());
+ }
+ }
+ }
+ return false;
+ }
+
+ @Override
+ public List<ColumnStatisticsObj> getColStatistics(org.apache.hadoop.hive.ql.metadata.Table hmsTable) {
+ Table table = Catalogs.loadTable(conf, Utilities.getTableDesc(hmsTable).getProperties());
+ String statsPath = getStatsPath(table).toString();
+ LOG.info("Using stats from puffin file at:" + statsPath);
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] simhadri-g commented on a diff in pull request #4131: HIVE-27158: Store hive columns stats in puffin files for iceberg tables
Posted by "simhadri-g (via GitHub)" <gi...@apache.org>.
simhadri-g commented on code in PR #4131:
URL: https://github.com/apache/hive/pull/4131#discussion_r1165323157
##########
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java:
##########
@@ -318,7 +335,7 @@ public Map<String, String> getBasicStatistics(Partish partish) {
org.apache.hadoop.hive.ql.metadata.Table hmsTable = partish.getTable();
// For write queries where rows got modified, don't fetch from cache as values could have changed.
Table table = getTable(hmsTable);
- String statsSource = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_USE_STATS_FROM).toLowerCase();
+ String statsSource = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_ICEBERG_STATS_SOURCE).toLowerCase();
Review Comment:
We changed the config name used for col stats and basic stats.
As a result, we had to be updated in the place it was previously used.
--
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] deniskuzZ commented on a diff in pull request #4131: HIVE-27158: Store hive columns stats in puffin files for iceberg tables
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #4131:
URL: https://github.com/apache/hive/pull/4131#discussion_r1148999605
##########
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java:
##########
@@ -349,6 +365,96 @@ public Map<String, String> getBasicStatistics(Partish partish) {
return stats;
}
+
+ @Override
+ public boolean canSetColStatistics() {
+ String statsSource = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_USE_STATS_FROM).toLowerCase();
+ return statsSource.equals(PUFFIN);
+ }
+
+ @Override
+ public boolean canProvideColStatistics(org.apache.hadoop.hive.ql.metadata.Table tbl) {
+
+ org.apache.hadoop.hive.ql.metadata.Table hmsTable = tbl;
+ TableDesc tableDesc = Utilities.getTableDesc(hmsTable);
+ Table table = Catalogs.loadTable(conf, tableDesc.getProperties());
+ if (table.currentSnapshot() != null) {
+ String statsSource = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_COL_STATS_SOURCE).toLowerCase();
+ String statsPath = table.location() + STATS + table.name() + table.currentSnapshot().snapshotId();
Review Comment:
should we wrap table name with some delimiters like '-'?
--
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] ayushtkn commented on a diff in pull request #4131: HIVE-27158: Store hive columns stats in puffin files for iceberg tables
Posted by "ayushtkn (via GitHub)" <gi...@apache.org>.
ayushtkn commented on code in PR #4131:
URL: https://github.com/apache/hive/pull/4131#discussion_r1161623163
##########
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java:
##########
@@ -2205,9 +2205,8 @@ public static enum ConfVars {
"padding tolerance config (hive.exec.orc.block.padding.tolerance)."),
HIVE_ORC_CODEC_POOL("hive.use.orc.codec.pool", false,
"Whether to use codec pool in ORC. Disable if there are bugs with codec reuse."),
- HIVE_USE_STATS_FROM("hive.use.stats.from","iceberg","Use stats from iceberg table snapshot for query " +
- "planning. This has three values metastore, puffin and iceberg"),
-
+ HIVE_ICEBERG_STATS_SOURCE("hive.iceberg.stats.source","iceberg","Use stats from iceberg table snapshot for query " +
+ "planning. This has three values metastore and iceberg"),
Review Comment:
> This has three values metastore and iceberg
what is the third value,?
##########
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java:
##########
@@ -361,6 +378,83 @@ private Table getTable(org.apache.hadoop.hive.ql.metadata.Table hmsTable) {
return table;
}
+
+ @Override
+ public boolean canSetColStatistics() {
+ return getStatsSource().equals(ICEBERG);
+ }
+
+ @Override
+ public boolean canProvideColStatistics(org.apache.hadoop.hive.ql.metadata.Table hmsTable) {
+ Table table = Catalogs.loadTable(conf, Utilities.getTableDesc(hmsTable).getProperties());
+ if (table.currentSnapshot() != null) {
+ Path statsPath = getStatsPath(table);
+ if (getStatsSource().equals(ICEBERG)) {
Review Comment:
can use ```canSetColStatistics()```
##########
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java:
##########
@@ -361,6 +378,83 @@ private Table getTable(org.apache.hadoop.hive.ql.metadata.Table hmsTable) {
return table;
}
+
+ @Override
+ public boolean canSetColStatistics() {
+ return getStatsSource().equals(ICEBERG);
+ }
+
+ @Override
+ public boolean canProvideColStatistics(org.apache.hadoop.hive.ql.metadata.Table hmsTable) {
+ Table table = Catalogs.loadTable(conf, Utilities.getTableDesc(hmsTable).getProperties());
+ if (table.currentSnapshot() != null) {
+ Path statsPath = getStatsPath(table);
+ if (getStatsSource().equals(ICEBERG)) {
+ try (FileSystem fs = statsPath.getFileSystem(conf)) {
+ if (fs.exists(statsPath)) {
+ return true;
+ }
+ } catch (IOException e) {
+ LOG.warn(e.getMessage());
+ }
+ }
+ }
+ return false;
+ }
+
+ @Override
+ public List<ColumnStatisticsObj> getColStatistics(org.apache.hadoop.hive.ql.metadata.Table hmsTable) {
+ Table table = Catalogs.loadTable(conf, Utilities.getTableDesc(hmsTable).getProperties());
+ String statsPath = getStatsPath(table).toString();
+ LOG.info("Using stats from puffin file at:" + statsPath);
Review Comment:
Logger format:
```
LOG.info("Using stats from puffin file at: {}", statsPath);
```
##########
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java:
##########
@@ -361,6 +378,83 @@ private Table getTable(org.apache.hadoop.hive.ql.metadata.Table hmsTable) {
return table;
}
+
+ @Override
+ public boolean canSetColStatistics() {
+ return getStatsSource().equals(ICEBERG);
+ }
+
+ @Override
+ public boolean canProvideColStatistics(org.apache.hadoop.hive.ql.metadata.Table hmsTable) {
+ Table table = Catalogs.loadTable(conf, Utilities.getTableDesc(hmsTable).getProperties());
+ if (table.currentSnapshot() != null) {
+ Path statsPath = getStatsPath(table);
+ if (getStatsSource().equals(ICEBERG)) {
+ try (FileSystem fs = statsPath.getFileSystem(conf)) {
+ if (fs.exists(statsPath)) {
+ return true;
+ }
+ } catch (IOException e) {
+ LOG.warn(e.getMessage());
+ }
+ }
+ }
+ return false;
+ }
+
+ @Override
+ public List<ColumnStatisticsObj> getColStatistics(org.apache.hadoop.hive.ql.metadata.Table hmsTable) {
+ Table table = Catalogs.loadTable(conf, Utilities.getTableDesc(hmsTable).getProperties());
+ String statsPath = getStatsPath(table).toString();
+ LOG.info("Using stats from puffin file at:" + statsPath);
+ try (PuffinReader reader = Puffin.read(table.io().newInputFile(statsPath)).build()) {
+ List<BlobMetadata> blobMetadata = reader.fileMetadata().blobs();
+ Map<BlobMetadata, List<ColumnStatistics>> collect =
+ Streams.stream(reader.readAll(blobMetadata)).collect(Collectors.toMap(Pair::first,
+ blobMetadataByteBufferPair -> SerializationUtils.deserialize(
+ ByteBuffers.toByteArray(blobMetadataByteBufferPair.second()))));
+ return collect.get(blobMetadata.get(0)).get(0).getStatsObj();
+ } catch (IOException e) {
+ LOG.error(String.valueOf(e));
+ }
+ return null;
+ }
+
+
+ @Override
+ public boolean setColStatistics(org.apache.hadoop.hive.ql.metadata.Table table,
+ List<ColumnStatistics> colStats) {
+ TableDesc tableDesc = Utilities.getTableDesc(table);
+ Table tbl = Catalogs.loadTable(conf, tableDesc.getProperties());
+ String snapshotId = tbl.name() + tbl.currentSnapshot().snapshotId();
+ byte[] serializeColStats = SerializationUtils.serialize((Serializable) colStats);
+
+ try (PuffinWriter writer = Puffin.write(tbl.io().newOutputFile(getStatsPath(tbl).toString()))
+ .createdBy("Hive").build()) {
+ writer.add(
+ new Blob(
+ tbl.name() + "-" + snapshotId,
+ ImmutableList.of(1),
+ tbl.currentSnapshot().snapshotId(),
+ tbl.currentSnapshot().sequenceNumber(),
+ ByteBuffer.wrap(serializeColStats),
+ PuffinCompressionCodec.NONE,
+ ImmutableMap.of()));
+ writer.finish();
+ } catch (IOException e) {
+ LOG.error(String.valueOf(e));
+ }
+ return false;
+ }
+
+ private String getStatsSource() {
+ return HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_ICEBERG_STATS_SOURCE, "metastore").toLowerCase();
+ }
Review Comment:
I don't understand this, why the default is here ``metastore``? when the config has default set as Iceberg. Who is using that default then
```
HIVE_ICEBERG_STATS_SOURCE("hive.iceberg.stats.source","iceberg",
```
##########
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java:
##########
@@ -361,6 +378,83 @@ private Table getTable(org.apache.hadoop.hive.ql.metadata.Table hmsTable) {
return table;
}
+
+ @Override
+ public boolean canSetColStatistics() {
+ return getStatsSource().equals(ICEBERG);
+ }
+
+ @Override
+ public boolean canProvideColStatistics(org.apache.hadoop.hive.ql.metadata.Table hmsTable) {
+ Table table = Catalogs.loadTable(conf, Utilities.getTableDesc(hmsTable).getProperties());
+ if (table.currentSnapshot() != null) {
+ Path statsPath = getStatsPath(table);
+ if (getStatsSource().equals(ICEBERG)) {
+ try (FileSystem fs = statsPath.getFileSystem(conf)) {
+ if (fs.exists(statsPath)) {
+ return true;
+ }
+ } catch (IOException e) {
+ LOG.warn(e.getMessage());
+ }
+ }
+ }
+ return false;
+ }
+
+ @Override
+ public List<ColumnStatisticsObj> getColStatistics(org.apache.hadoop.hive.ql.metadata.Table hmsTable) {
+ Table table = Catalogs.loadTable(conf, Utilities.getTableDesc(hmsTable).getProperties());
Review Comment:
Change to ``Table table = IcebergTableUtil.getTable(conf, hmsTable.getTTable());``
##########
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java:
##########
@@ -361,6 +378,83 @@ private Table getTable(org.apache.hadoop.hive.ql.metadata.Table hmsTable) {
return table;
}
+
+ @Override
+ public boolean canSetColStatistics() {
+ return getStatsSource().equals(ICEBERG);
+ }
+
+ @Override
+ public boolean canProvideColStatistics(org.apache.hadoop.hive.ql.metadata.Table hmsTable) {
+ Table table = Catalogs.loadTable(conf, Utilities.getTableDesc(hmsTable).getProperties());
+ if (table.currentSnapshot() != null) {
+ Path statsPath = getStatsPath(table);
+ if (getStatsSource().equals(ICEBERG)) {
+ try (FileSystem fs = statsPath.getFileSystem(conf)) {
+ if (fs.exists(statsPath)) {
+ return true;
+ }
+ } catch (IOException e) {
+ LOG.warn(e.getMessage());
Review Comment:
Log the trace as well, rather than just the message. Along with the table name and the stats path and the snapshot id
##########
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java:
##########
@@ -361,6 +378,83 @@ private Table getTable(org.apache.hadoop.hive.ql.metadata.Table hmsTable) {
return table;
}
+
+ @Override
+ public boolean canSetColStatistics() {
+ return getStatsSource().equals(ICEBERG);
+ }
+
+ @Override
+ public boolean canProvideColStatistics(org.apache.hadoop.hive.ql.metadata.Table hmsTable) {
+ Table table = Catalogs.loadTable(conf, Utilities.getTableDesc(hmsTable).getProperties());
+ if (table.currentSnapshot() != null) {
+ Path statsPath = getStatsPath(table);
+ if (getStatsSource().equals(ICEBERG)) {
+ try (FileSystem fs = statsPath.getFileSystem(conf)) {
+ if (fs.exists(statsPath)) {
+ return true;
+ }
+ } catch (IOException e) {
+ LOG.warn(e.getMessage());
+ }
+ }
+ }
+ return false;
+ }
+
+ @Override
+ public List<ColumnStatisticsObj> getColStatistics(org.apache.hadoop.hive.ql.metadata.Table hmsTable) {
+ Table table = Catalogs.loadTable(conf, Utilities.getTableDesc(hmsTable).getProperties());
+ String statsPath = getStatsPath(table).toString();
+ LOG.info("Using stats from puffin file at:" + statsPath);
+ try (PuffinReader reader = Puffin.read(table.io().newInputFile(statsPath)).build()) {
+ List<BlobMetadata> blobMetadata = reader.fileMetadata().blobs();
+ Map<BlobMetadata, List<ColumnStatistics>> collect =
+ Streams.stream(reader.readAll(blobMetadata)).collect(Collectors.toMap(Pair::first,
+ blobMetadataByteBufferPair -> SerializationUtils.deserialize(
+ ByteBuffers.toByteArray(blobMetadataByteBufferPair.second()))));
+ return collect.get(blobMetadata.get(0)).get(0).getStatsObj();
+ } catch (IOException e) {
+ LOG.error(String.valueOf(e));
+ }
+ return null;
+ }
+
+
+ @Override
+ public boolean setColStatistics(org.apache.hadoop.hive.ql.metadata.Table table,
+ List<ColumnStatistics> colStats) {
+ TableDesc tableDesc = Utilities.getTableDesc(table);
+ Table tbl = Catalogs.loadTable(conf, tableDesc.getProperties());
+ String snapshotId = tbl.name() + tbl.currentSnapshot().snapshotId();
Review Comment:
can currentSnapshot be ``null``? like empty table and then somebody shoots a CLI command to compute statistics?
##########
ql/src/java/org/apache/hadoop/hive/ql/stats/StatsUtils.java:
##########
@@ -1069,8 +1069,12 @@ public static List<ColStatistics> getTableColumnStats(
}
if (fetchColStats && !colStatsToRetrieve.isEmpty()) {
try {
- List<ColumnStatisticsObj> colStat = Hive.get().getTableColumnStatistics(
- dbName, tabName, colStatsToRetrieve, false);
+ List<ColumnStatisticsObj> colStat;
+ if (table != null && table.isNonNative() && table.getStorageHandler().canProvideColStatistics(table)) {
Review Comment:
table can not be `null` at this point
##########
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java:
##########
@@ -361,6 +378,83 @@ private Table getTable(org.apache.hadoop.hive.ql.metadata.Table hmsTable) {
return table;
}
+
+ @Override
+ public boolean canSetColStatistics() {
+ return getStatsSource().equals(ICEBERG);
+ }
+
+ @Override
+ public boolean canProvideColStatistics(org.apache.hadoop.hive.ql.metadata.Table hmsTable) {
+ Table table = Catalogs.loadTable(conf, Utilities.getTableDesc(hmsTable).getProperties());
Review Comment:
Change to `` Table table = IcebergTableUtil.getTable(conf, hmsTable.getTTable());``
##########
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java:
##########
@@ -361,6 +378,83 @@ private Table getTable(org.apache.hadoop.hive.ql.metadata.Table hmsTable) {
return table;
}
+
+ @Override
+ public boolean canSetColStatistics() {
+ return getStatsSource().equals(ICEBERG);
+ }
+
+ @Override
+ public boolean canProvideColStatistics(org.apache.hadoop.hive.ql.metadata.Table hmsTable) {
+ Table table = Catalogs.loadTable(conf, Utilities.getTableDesc(hmsTable).getProperties());
+ if (table.currentSnapshot() != null) {
+ Path statsPath = getStatsPath(table);
+ if (getStatsSource().equals(ICEBERG)) {
+ try (FileSystem fs = statsPath.getFileSystem(conf)) {
+ if (fs.exists(statsPath)) {
+ return true;
+ }
+ } catch (IOException e) {
+ LOG.warn(e.getMessage());
+ }
+ }
+ }
+ return false;
+ }
+
+ @Override
+ public List<ColumnStatisticsObj> getColStatistics(org.apache.hadoop.hive.ql.metadata.Table hmsTable) {
+ Table table = Catalogs.loadTable(conf, Utilities.getTableDesc(hmsTable).getProperties());
+ String statsPath = getStatsPath(table).toString();
+ LOG.info("Using stats from puffin file at:" + statsPath);
+ try (PuffinReader reader = Puffin.read(table.io().newInputFile(statsPath)).build()) {
+ List<BlobMetadata> blobMetadata = reader.fileMetadata().blobs();
+ Map<BlobMetadata, List<ColumnStatistics>> collect =
+ Streams.stream(reader.readAll(blobMetadata)).collect(Collectors.toMap(Pair::first,
+ blobMetadataByteBufferPair -> SerializationUtils.deserialize(
+ ByteBuffers.toByteArray(blobMetadataByteBufferPair.second()))));
+ return collect.get(blobMetadata.get(0)).get(0).getStatsObj();
+ } catch (IOException e) {
+ LOG.error(String.valueOf(e));
+ }
+ return null;
+ }
+
+
+ @Override
+ public boolean setColStatistics(org.apache.hadoop.hive.ql.metadata.Table table,
+ List<ColumnStatistics> colStats) {
+ TableDesc tableDesc = Utilities.getTableDesc(table);
+ Table tbl = Catalogs.loadTable(conf, tableDesc.getProperties());
+ String snapshotId = tbl.name() + tbl.currentSnapshot().snapshotId();
+ byte[] serializeColStats = SerializationUtils.serialize((Serializable) colStats);
+
+ try (PuffinWriter writer = Puffin.write(tbl.io().newOutputFile(getStatsPath(tbl).toString()))
+ .createdBy("Hive").build()) {
Review Comment:
Use constant
```
Constants.HIVE_ENGINE
```
##########
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java:
##########
@@ -361,6 +378,83 @@ private Table getTable(org.apache.hadoop.hive.ql.metadata.Table hmsTable) {
return table;
}
+
+ @Override
+ public boolean canSetColStatistics() {
+ return getStatsSource().equals(ICEBERG);
+ }
+
+ @Override
+ public boolean canProvideColStatistics(org.apache.hadoop.hive.ql.metadata.Table hmsTable) {
+ Table table = Catalogs.loadTable(conf, Utilities.getTableDesc(hmsTable).getProperties());
+ if (table.currentSnapshot() != null) {
+ Path statsPath = getStatsPath(table);
+ if (getStatsSource().equals(ICEBERG)) {
+ try (FileSystem fs = statsPath.getFileSystem(conf)) {
+ if (fs.exists(statsPath)) {
+ return true;
+ }
+ } catch (IOException e) {
+ LOG.warn(e.getMessage());
+ }
+ }
+ }
+ return false;
+ }
+
+ @Override
+ public List<ColumnStatisticsObj> getColStatistics(org.apache.hadoop.hive.ql.metadata.Table hmsTable) {
+ Table table = Catalogs.loadTable(conf, Utilities.getTableDesc(hmsTable).getProperties());
+ String statsPath = getStatsPath(table).toString();
+ LOG.info("Using stats from puffin file at:" + statsPath);
+ try (PuffinReader reader = Puffin.read(table.io().newInputFile(statsPath)).build()) {
+ List<BlobMetadata> blobMetadata = reader.fileMetadata().blobs();
+ Map<BlobMetadata, List<ColumnStatistics>> collect =
+ Streams.stream(reader.readAll(blobMetadata)).collect(Collectors.toMap(Pair::first,
+ blobMetadataByteBufferPair -> SerializationUtils.deserialize(
+ ByteBuffers.toByteArray(blobMetadataByteBufferPair.second()))));
+ return collect.get(blobMetadata.get(0)).get(0).getStatsObj();
+ } catch (IOException e) {
+ LOG.error(String.valueOf(e));
+ }
+ return null;
+ }
+
+
+ @Override
+ public boolean setColStatistics(org.apache.hadoop.hive.ql.metadata.Table table,
+ List<ColumnStatistics> colStats) {
+ TableDesc tableDesc = Utilities.getTableDesc(table);
+ Table tbl = Catalogs.loadTable(conf, tableDesc.getProperties());
Review Comment:
Use ``IcebergTableUtil`` to fetch the table, It has cache, fetching and reading the table metadata multiple times have severe performance penalties, cache the table and use it from cache, unless necessary
##########
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java:
##########
@@ -361,6 +378,83 @@ private Table getTable(org.apache.hadoop.hive.ql.metadata.Table hmsTable) {
return table;
}
+
+ @Override
+ public boolean canSetColStatistics() {
+ return getStatsSource().equals(ICEBERG);
+ }
+
+ @Override
+ public boolean canProvideColStatistics(org.apache.hadoop.hive.ql.metadata.Table hmsTable) {
+ Table table = Catalogs.loadTable(conf, Utilities.getTableDesc(hmsTable).getProperties());
+ if (table.currentSnapshot() != null) {
+ Path statsPath = getStatsPath(table);
+ if (getStatsSource().equals(ICEBERG)) {
+ try (FileSystem fs = statsPath.getFileSystem(conf)) {
+ if (fs.exists(statsPath)) {
+ return true;
+ }
+ } catch (IOException e) {
+ LOG.warn(e.getMessage());
+ }
+ }
+ }
+ return false;
+ }
+
+ @Override
+ public List<ColumnStatisticsObj> getColStatistics(org.apache.hadoop.hive.ql.metadata.Table hmsTable) {
+ Table table = Catalogs.loadTable(conf, Utilities.getTableDesc(hmsTable).getProperties());
+ String statsPath = getStatsPath(table).toString();
+ LOG.info("Using stats from puffin file at:" + statsPath);
+ try (PuffinReader reader = Puffin.read(table.io().newInputFile(statsPath)).build()) {
+ List<BlobMetadata> blobMetadata = reader.fileMetadata().blobs();
+ Map<BlobMetadata, List<ColumnStatistics>> collect =
+ Streams.stream(reader.readAll(blobMetadata)).collect(Collectors.toMap(Pair::first,
+ blobMetadataByteBufferPair -> SerializationUtils.deserialize(
+ ByteBuffers.toByteArray(blobMetadataByteBufferPair.second()))));
+ return collect.get(blobMetadata.get(0)).get(0).getStatsObj();
+ } catch (IOException e) {
+ LOG.error(String.valueOf(e));
+ }
+ return null;
Review Comment:
Why don't we throw exception here?
--
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 #4131: HIVE-27158: Store hive columns stats in puffin files for iceberg tables
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #4131:
URL: https://github.com/apache/hive/pull/4131#issuecomment-1476983951
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=4131)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4131&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4131&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4131&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=CODE_SMELL) [8 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4131&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4131&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] deniskuzZ commented on a diff in pull request #4131: HIVE-27158: Store hive columns stats in puffin files for iceberg tables
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #4131:
URL: https://github.com/apache/hive/pull/4131#discussion_r1149123408
##########
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java:
##########
@@ -349,6 +365,96 @@ public Map<String, String> getBasicStatistics(Partish partish) {
return stats;
}
+
+ @Override
+ public boolean canSetColStatistics() {
+ String statsSource = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_USE_STATS_FROM).toLowerCase();
+ return statsSource.equals(PUFFIN);
+ }
+
+ @Override
+ public boolean canProvideColStatistics(org.apache.hadoop.hive.ql.metadata.Table tbl) {
+
+ org.apache.hadoop.hive.ql.metadata.Table hmsTable = tbl;
+ TableDesc tableDesc = Utilities.getTableDesc(hmsTable);
+ Table table = Catalogs.loadTable(conf, tableDesc.getProperties());
+ if (table.currentSnapshot() != null) {
+ String statsSource = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_COL_STATS_SOURCE).toLowerCase();
+ String statsPath = table.location() + STATS + table.name() + table.currentSnapshot().snapshotId();
+ if (statsSource.equals(PUFFIN)) {
+ try (FileSystem fs = new Path(table.location()).getFileSystem(conf)) {
+ if (fs.exists(new Path(statsPath))) {
+ return true;
+ }
+ } catch (IOException e) {
+ LOG.warn(e.getMessage());
+ }
+ }
+ }
+ return false;
+ }
+
+ @Override
+ public List<ColumnStatisticsObj> getColStatistics(org.apache.hadoop.hive.ql.metadata.Table tbl) {
+
+ org.apache.hadoop.hive.ql.metadata.Table hmsTable = tbl;
+ TableDesc tableDesc = Utilities.getTableDesc(hmsTable);
+ Table table = Catalogs.loadTable(conf, tableDesc.getProperties());
+ String statsSource = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_COL_STATS_SOURCE).toLowerCase();
+ switch (statsSource) {
+ case ICEBERG:
+ // Place holder for iceberg stats
+ break;
+ case PUFFIN:
+ String snapshotId = table.name() + table.currentSnapshot().snapshotId();
+ String statsPath = table.location() + STATS + snapshotId;
+ LOG.info("Using stats from puffin file at:" + statsPath);
+ try (PuffinReader reader = Puffin.read(table.io().newInputFile(statsPath)).build()) {
+ BlobMetadata blobMetadata = reader.fileMetadata().blobs().get(0);
+ Map<BlobMetadata, List<ColumnStatistics>> collect =
+ Streams.stream(reader.readAll(ImmutableList.of(blobMetadata))).collect(Collectors.toMap(Pair::first,
+ blobMetadataByteBufferPair -> SerializationUtils.deserialize(
+ ByteBuffers.toByteArray(blobMetadataByteBufferPair.second()))));
+
+ return collect.entrySet().stream().iterator().next().getValue().get(0).getStatsObj();
+ } catch (IOException e) {
+ LOG.info(String.valueOf(e));
+ }
+ break;
+ default:
+ // fall back to metastore
+ }
+ return null;
+ }
+
+
+ @Override
+ public boolean setColStatistics(org.apache.hadoop.hive.ql.metadata.Table table,
+ List<ColumnStatistics> colStats) {
+ TableDesc tableDesc = Utilities.getTableDesc(table);
+ Table tbl = Catalogs.loadTable(conf, tableDesc.getProperties());
+ String snapshotId = tbl.name() + tbl.currentSnapshot().snapshotId();
+ byte[] serializeColStats = SerializationUtils.serialize((Serializable) colStats);
+
+ try (PuffinWriter writer = Puffin.write(tbl.io().newOutputFile(tbl.location() + STATS + snapshotId))
Review Comment:
looks like not, per `getColStatsForPartCol` comment: currently, metastore does not store column stats for the partition column. Also in the HMS column stats table is called `TAB_COL_STATS` that doesn't track the per-partition 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] deniskuzZ commented on a diff in pull request #4131: HIVE-27158: Store hive columns stats in puffin files for iceberg tables
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #4131:
URL: https://github.com/apache/hive/pull/4131#discussion_r1148993843
##########
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java:
##########
@@ -349,6 +365,96 @@ public Map<String, String> getBasicStatistics(Partish partish) {
return stats;
}
+
+ @Override
+ public boolean canSetColStatistics() {
+ String statsSource = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_USE_STATS_FROM).toLowerCase();
+ return statsSource.equals(PUFFIN);
Review Comment:
what if statsSource is undefined, could we get NPE?
--
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] deniskuzZ commented on a diff in pull request #4131: HIVE-27158: Store hive columns stats in puffin files for iceberg tables
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #4131:
URL: https://github.com/apache/hive/pull/4131#discussion_r1149035181
##########
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java:
##########
@@ -349,6 +365,96 @@ public Map<String, String> getBasicStatistics(Partish partish) {
return stats;
}
+
+ @Override
+ public boolean canSetColStatistics() {
+ String statsSource = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_USE_STATS_FROM).toLowerCase();
+ return statsSource.equals(PUFFIN);
+ }
+
+ @Override
+ public boolean canProvideColStatistics(org.apache.hadoop.hive.ql.metadata.Table tbl) {
+
+ org.apache.hadoop.hive.ql.metadata.Table hmsTable = tbl;
+ TableDesc tableDesc = Utilities.getTableDesc(hmsTable);
+ Table table = Catalogs.loadTable(conf, tableDesc.getProperties());
+ if (table.currentSnapshot() != null) {
+ String statsSource = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_COL_STATS_SOURCE).toLowerCase();
+ String statsPath = table.location() + STATS + table.name() + table.currentSnapshot().snapshotId();
+ if (statsSource.equals(PUFFIN)) {
+ try (FileSystem fs = new Path(table.location()).getFileSystem(conf)) {
+ if (fs.exists(new Path(statsPath))) {
+ return true;
+ }
+ } catch (IOException e) {
+ LOG.warn(e.getMessage());
+ }
+ }
+ }
+ return false;
+ }
+
+ @Override
+ public List<ColumnStatisticsObj> getColStatistics(org.apache.hadoop.hive.ql.metadata.Table tbl) {
+
+ org.apache.hadoop.hive.ql.metadata.Table hmsTable = tbl;
+ TableDesc tableDesc = Utilities.getTableDesc(hmsTable);
+ Table table = Catalogs.loadTable(conf, tableDesc.getProperties());
+ String statsSource = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_COL_STATS_SOURCE).toLowerCase();
+ switch (statsSource) {
+ case ICEBERG:
+ // Place holder for iceberg stats
+ break;
+ case PUFFIN:
+ String snapshotId = table.name() + table.currentSnapshot().snapshotId();
+ String statsPath = table.location() + STATS + snapshotId;
+ LOG.info("Using stats from puffin file at:" + statsPath);
+ try (PuffinReader reader = Puffin.read(table.io().newInputFile(statsPath)).build()) {
+ BlobMetadata blobMetadata = reader.fileMetadata().blobs().get(0);
+ Map<BlobMetadata, List<ColumnStatistics>> collect =
+ Streams.stream(reader.readAll(ImmutableList.of(blobMetadata))).collect(Collectors.toMap(Pair::first,
+ blobMetadataByteBufferPair -> SerializationUtils.deserialize(
+ ByteBuffers.toByteArray(blobMetadataByteBufferPair.second()))));
+
+ return collect.entrySet().stream().iterator().next().getValue().get(0).getStatsObj();
+ } catch (IOException e) {
+ LOG.info(String.valueOf(e));
+ }
+ break;
+ default:
+ // fall back to metastore
+ }
+ return null;
+ }
+
+
+ @Override
+ public boolean setColStatistics(org.apache.hadoop.hive.ql.metadata.Table table,
+ List<ColumnStatistics> colStats) {
+ TableDesc tableDesc = Utilities.getTableDesc(table);
+ Table tbl = Catalogs.loadTable(conf, tableDesc.getProperties());
+ String snapshotId = tbl.name() + tbl.currentSnapshot().snapshotId();
+ byte[] serializeColStats = SerializationUtils.serialize((Serializable) colStats);
+
+ try (PuffinWriter writer = Puffin.write(tbl.io().newOutputFile(tbl.location() + STATS + snapshotId))
+ .createdBy("Hive").build()) {
+ writer.add(
+ new Blob(
+ tbl.name() + "-" + snapshotId,
+ ImmutableList.of(1),
+ tbl.currentSnapshot().snapshotId(),
+ tbl.currentSnapshot().sequenceNumber(),
+ ByteBuffer.wrap(serializeColStats),
+ PuffinCompressionCodec.NONE,
+ ImmutableMap.of()));
+ writer.finish();
+ } catch (IOException e) {
+ LOG.info(String.valueOf(e));
Review Comment:
do not swallow exception
--
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 #4131: HIVE-27158: Store hive columns stats in puffin files for iceberg tables
Posted by "simhadri-g (via GitHub)" <gi...@apache.org>.
simhadri-g commented on code in PR #4131:
URL: https://github.com/apache/hive/pull/4131#discussion_r1161856054
##########
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java:
##########
@@ -361,6 +378,83 @@ private Table getTable(org.apache.hadoop.hive.ql.metadata.Table hmsTable) {
return table;
}
+
+ @Override
+ public boolean canSetColStatistics() {
+ return getStatsSource().equals(ICEBERG);
+ }
+
+ @Override
+ public boolean canProvideColStatistics(org.apache.hadoop.hive.ql.metadata.Table hmsTable) {
+ Table table = Catalogs.loadTable(conf, Utilities.getTableDesc(hmsTable).getProperties());
Review Comment:
Done
##########
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java:
##########
@@ -361,6 +378,83 @@ private Table getTable(org.apache.hadoop.hive.ql.metadata.Table hmsTable) {
return table;
}
+
+ @Override
+ public boolean canSetColStatistics() {
+ return getStatsSource().equals(ICEBERG);
+ }
+
+ @Override
+ public boolean canProvideColStatistics(org.apache.hadoop.hive.ql.metadata.Table hmsTable) {
+ Table table = Catalogs.loadTable(conf, Utilities.getTableDesc(hmsTable).getProperties());
+ if (table.currentSnapshot() != null) {
+ Path statsPath = getStatsPath(table);
+ if (getStatsSource().equals(ICEBERG)) {
Review Comment:
Fixed.
--
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] TuroczyX commented on pull request #4131: HIVE-27158: Store hive columns stats in puffin files for iceberg tables
Posted by "TuroczyX (via GitHub)" <gi...@apache.org>.
TuroczyX commented on PR #4131:
URL: https://github.com/apache/hive/pull/4131#issuecomment-1514469677
Juhuu! :)
--
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 #4131: HIVE-27158: Store hive columns stats in puffin files for iceberg tables
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #4131:
URL: https://github.com/apache/hive/pull/4131#issuecomment-1482423092
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=4131)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4131&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4131&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4131&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=CODE_SMELL) [6 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4131&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4131&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 #4131: HIVE-27158: Store hive columns stats in puffin files for iceberg tables
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #4131:
URL: https://github.com/apache/hive/pull/4131#issuecomment-1484679758
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=4131)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4131&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4131&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4131&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=CODE_SMELL) [6 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4131&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4131&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] deniskuzZ commented on a diff in pull request #4131: HIVE-27158: Store hive columns stats in puffin files for iceberg tables
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #4131:
URL: https://github.com/apache/hive/pull/4131#discussion_r1149022350
##########
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java:
##########
@@ -349,6 +365,96 @@ public Map<String, String> getBasicStatistics(Partish partish) {
return stats;
}
+
+ @Override
+ public boolean canSetColStatistics() {
+ String statsSource = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_USE_STATS_FROM).toLowerCase();
+ return statsSource.equals(PUFFIN);
+ }
+
+ @Override
+ public boolean canProvideColStatistics(org.apache.hadoop.hive.ql.metadata.Table tbl) {
+
+ org.apache.hadoop.hive.ql.metadata.Table hmsTable = tbl;
+ TableDesc tableDesc = Utilities.getTableDesc(hmsTable);
+ Table table = Catalogs.loadTable(conf, tableDesc.getProperties());
+ if (table.currentSnapshot() != null) {
+ String statsSource = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_COL_STATS_SOURCE).toLowerCase();
+ String statsPath = table.location() + STATS + table.name() + table.currentSnapshot().snapshotId();
+ if (statsSource.equals(PUFFIN)) {
+ try (FileSystem fs = new Path(table.location()).getFileSystem(conf)) {
+ if (fs.exists(new Path(statsPath))) {
+ return true;
+ }
+ } catch (IOException e) {
+ LOG.warn(e.getMessage());
+ }
+ }
+ }
+ return false;
+ }
+
+ @Override
+ public List<ColumnStatisticsObj> getColStatistics(org.apache.hadoop.hive.ql.metadata.Table tbl) {
+
+ org.apache.hadoop.hive.ql.metadata.Table hmsTable = tbl;
+ TableDesc tableDesc = Utilities.getTableDesc(hmsTable);
+ Table table = Catalogs.loadTable(conf, tableDesc.getProperties());
+ String statsSource = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_COL_STATS_SOURCE).toLowerCase();
+ switch (statsSource) {
+ case ICEBERG:
+ // Place holder for iceberg stats
+ break;
+ case PUFFIN:
+ String snapshotId = table.name() + table.currentSnapshot().snapshotId();
+ String statsPath = table.location() + STATS + snapshotId;
Review Comment:
could we extract path construction into a helper method?
##########
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java:
##########
@@ -349,6 +365,96 @@ public Map<String, String> getBasicStatistics(Partish partish) {
return stats;
}
+
+ @Override
+ public boolean canSetColStatistics() {
+ String statsSource = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_USE_STATS_FROM).toLowerCase();
+ return statsSource.equals(PUFFIN);
+ }
+
+ @Override
+ public boolean canProvideColStatistics(org.apache.hadoop.hive.ql.metadata.Table tbl) {
+
+ org.apache.hadoop.hive.ql.metadata.Table hmsTable = tbl;
+ TableDesc tableDesc = Utilities.getTableDesc(hmsTable);
+ Table table = Catalogs.loadTable(conf, tableDesc.getProperties());
+ if (table.currentSnapshot() != null) {
+ String statsSource = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_COL_STATS_SOURCE).toLowerCase();
+ String statsPath = table.location() + STATS + table.name() + table.currentSnapshot().snapshotId();
+ if (statsSource.equals(PUFFIN)) {
+ try (FileSystem fs = new Path(table.location()).getFileSystem(conf)) {
+ if (fs.exists(new Path(statsPath))) {
+ return true;
+ }
+ } catch (IOException e) {
+ LOG.warn(e.getMessage());
+ }
+ }
+ }
+ return false;
+ }
+
+ @Override
+ public List<ColumnStatisticsObj> getColStatistics(org.apache.hadoop.hive.ql.metadata.Table tbl) {
+
+ org.apache.hadoop.hive.ql.metadata.Table hmsTable = tbl;
+ TableDesc tableDesc = Utilities.getTableDesc(hmsTable);
+ Table table = Catalogs.loadTable(conf, tableDesc.getProperties());
+ String statsSource = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_COL_STATS_SOURCE).toLowerCase();
+ switch (statsSource) {
+ case ICEBERG:
+ // Place holder for iceberg stats
+ break;
+ case PUFFIN:
+ String snapshotId = table.name() + table.currentSnapshot().snapshotId();
+ String statsPath = table.location() + STATS + snapshotId;
Review Comment:
could we extract path construction into a helper method and reuse?
--
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] deniskuzZ merged pull request #4131: HIVE-27158: Store hive columns stats in puffin files for iceberg tables
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ merged PR #4131:
URL: https://github.com/apache/hive/pull/4131
--
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 a diff in pull request #4131: HIVE-27158: Store hive columns stats in puffin files for iceberg tables
Posted by "InvisibleProgrammer (via GitHub)" <gi...@apache.org>.
InvisibleProgrammer commented on code in PR #4131:
URL: https://github.com/apache/hive/pull/4131#discussion_r1165171624
##########
ql/src/java/org/apache/hadoop/hive/ql/metadata/HiveStorageHandler.java:
##########
@@ -245,6 +248,44 @@ default boolean canProvideBasicStatistics() {
return false;
}
+ /**
+ * Return some col statistics (Lower bounds, Upper bounds, Null value counts, NaN, total counts) calculated by
+ * the underlying storage handler implementation.
+ * @param table
+ * @return A List of Column Statistics Objects, can be null
+ */
+ default List<ColumnStatisticsObj>getColStatistics(org.apache.hadoop.hive.ql.metadata.Table table) {
+ return null;
+ }
+
+ /**
+ * Set column stats for non-native tables
+ * @param table
+ * @param colStats
+ * @return boolean
+ */
+ default boolean setColStatistics(org.apache.hadoop.hive.ql.metadata.Table table,
+ List<ColumnStatistics> colStats) {
+ return false;
+ }
+
+ /**
+ * Check if the storage handler can provide col statistics.
+ * @param tbl
+ * @return true if the storage handler can supply the col statistics
+ */
+ default boolean canProvideColStatistics(org.apache.hadoop.hive.ql.metadata.Table tbl) {
+ return false;
+ }
+
+ /**
+ * Check if the storage handler can set col statistics.
+ * @return true if the storage handler can set the col statistics
+ */
+ default boolean canSetColStatistics(org.apache.hadoop.hive.ql.metadata.Table tbl) {
Review Comment:
I don't know the good answer, I'm just thinking:
If we have a pair of methods like `canSetColStatistics` and `setColStatistics`. Can we do that in a way that doesn't allow to call `setColStatistics` if they cannot be set?
--
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 #4131: HIVE-27158: Store hive columns stats in puffin files for iceberg tables
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #4131:
URL: https://github.com/apache/hive/pull/4131#issuecomment-1505941900
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=4131)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4131&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4131&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4131&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=CODE_SMELL) [6 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4131&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4131&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 #4131: HIVE-27158: Store hive columns stats in puffin files for iceberg tables
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #4131:
URL: https://github.com/apache/hive/pull/4131#issuecomment-1506524618
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=4131)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4131&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4131&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4131&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=CODE_SMELL) [6 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4131&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4131&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 #4131: HIVE-27158: Store hive columns stats in puffin files for iceberg tables
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #4131:
URL: https://github.com/apache/hive/pull/4131#issuecomment-1481637666
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=4131)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=BUG) [1 Bug](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4131&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4131&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4131&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=CODE_SMELL) [23 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4131&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4131&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] deniskuzZ commented on a diff in pull request #4131: HIVE-27158: Store hive columns stats in puffin files for iceberg tables
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #4131:
URL: https://github.com/apache/hive/pull/4131#discussion_r1149037383
##########
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java:
##########
@@ -349,6 +365,96 @@ public Map<String, String> getBasicStatistics(Partish partish) {
return stats;
}
+
+ @Override
+ public boolean canSetColStatistics() {
+ String statsSource = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_USE_STATS_FROM).toLowerCase();
+ return statsSource.equals(PUFFIN);
+ }
+
+ @Override
+ public boolean canProvideColStatistics(org.apache.hadoop.hive.ql.metadata.Table tbl) {
+
+ org.apache.hadoop.hive.ql.metadata.Table hmsTable = tbl;
+ TableDesc tableDesc = Utilities.getTableDesc(hmsTable);
+ Table table = Catalogs.loadTable(conf, tableDesc.getProperties());
+ if (table.currentSnapshot() != null) {
+ String statsSource = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_COL_STATS_SOURCE).toLowerCase();
+ String statsPath = table.location() + STATS + table.name() + table.currentSnapshot().snapshotId();
+ if (statsSource.equals(PUFFIN)) {
+ try (FileSystem fs = new Path(table.location()).getFileSystem(conf)) {
+ if (fs.exists(new Path(statsPath))) {
+ return true;
+ }
+ } catch (IOException e) {
+ LOG.warn(e.getMessage());
+ }
+ }
+ }
+ return false;
+ }
+
+ @Override
+ public List<ColumnStatisticsObj> getColStatistics(org.apache.hadoop.hive.ql.metadata.Table tbl) {
+
+ org.apache.hadoop.hive.ql.metadata.Table hmsTable = tbl;
+ TableDesc tableDesc = Utilities.getTableDesc(hmsTable);
+ Table table = Catalogs.loadTable(conf, tableDesc.getProperties());
+ String statsSource = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_COL_STATS_SOURCE).toLowerCase();
+ switch (statsSource) {
+ case ICEBERG:
+ // Place holder for iceberg stats
+ break;
+ case PUFFIN:
+ String snapshotId = table.name() + table.currentSnapshot().snapshotId();
+ String statsPath = table.location() + STATS + snapshotId;
+ LOG.info("Using stats from puffin file at:" + statsPath);
+ try (PuffinReader reader = Puffin.read(table.io().newInputFile(statsPath)).build()) {
+ BlobMetadata blobMetadata = reader.fileMetadata().blobs().get(0);
+ Map<BlobMetadata, List<ColumnStatistics>> collect =
+ Streams.stream(reader.readAll(ImmutableList.of(blobMetadata))).collect(Collectors.toMap(Pair::first,
+ blobMetadataByteBufferPair -> SerializationUtils.deserialize(
+ ByteBuffers.toByteArray(blobMetadataByteBufferPair.second()))));
+
+ return collect.entrySet().stream().iterator().next().getValue().get(0).getStatsObj();
+ } catch (IOException e) {
+ LOG.info(String.valueOf(e));
+ }
+ break;
+ default:
+ // fall back to metastore
+ }
+ return null;
+ }
+
+
+ @Override
+ public boolean setColStatistics(org.apache.hadoop.hive.ql.metadata.Table table,
+ List<ColumnStatistics> colStats) {
+ TableDesc tableDesc = Utilities.getTableDesc(table);
+ Table tbl = Catalogs.loadTable(conf, tableDesc.getProperties());
+ String snapshotId = tbl.name() + tbl.currentSnapshot().snapshotId();
+ byte[] serializeColStats = SerializationUtils.serialize((Serializable) colStats);
+
+ try (PuffinWriter writer = Puffin.write(tbl.io().newOutputFile(tbl.location() + STATS + snapshotId))
Review Comment:
what about partition-level stats? if the table is partitioned you need to append the partition stats as well
--
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] deniskuzZ commented on a diff in pull request #4131: HIVE-27158: Store hive columns stats in puffin files for iceberg tables
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #4131:
URL: https://github.com/apache/hive/pull/4131#discussion_r1149109172
##########
ql/src/java/org/apache/hadoop/hive/ql/stats/ColStatsProcessor.java:
##########
@@ -218,6 +218,9 @@ public int persistColumnStats(Hive db, Table tbl) throws HiveException, MetaExce
}
start = System. currentTimeMillis();
+ if(tbl != null && tbl.isNonNative() && tbl.getStorageHandler().canSetColStatistics()){
Review Comment:
nit: space
--
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 #4131: HIVE-27158: Store hive columns stats in puffin files for iceberg tables
Posted by "simhadri-g (via GitHub)" <gi...@apache.org>.
simhadri-g commented on code in PR #4131:
URL: https://github.com/apache/hive/pull/4131#discussion_r1146105589
##########
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java:
##########
@@ -349,6 +364,98 @@ public Map<String, String> getBasicStatistics(Partish partish) {
return stats;
}
+
+ @Override
+ public boolean canSetColStatistics() {
+ String statsSource = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_USE_STATS_FROM).toLowerCase();
+ return statsSource.equals(PUFFIN) ? true : false;
+ }
+
+ @Override
+ public boolean canProvideColStatistics(org.apache.hadoop.hive.ql.metadata.Table tbl) {
+
+ org.apache.hadoop.hive.ql.metadata.Table hmsTable = tbl;
+ TableDesc tableDesc = Utilities.getTableDesc(hmsTable);
+ Table table = Catalogs.loadTable(conf, tableDesc.getProperties());
+ if (table.currentSnapshot() != null) {
+ String statsSource = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_USE_STATS_FROM).toLowerCase();
+ String statsPath = table.location() + "/stats/" + table.name() + table.currentSnapshot().snapshotId();
+ if (statsSource.equals(PUFFIN)) {
+ try (FileSystem fs = new Path(table.location()).getFileSystem(conf)) {
+ if (fs.exists(new Path(statsPath))) {
+ return true;
+ }
+ } catch (IOException e) {
+ LOG.warn(e.getMessage());
+ }
+ }
+ }
+ return false;
+ }
+
+ @Override
+ public List<ColumnStatisticsObj> getColStatistics(org.apache.hadoop.hive.ql.metadata.Table tbl) {
+
+ org.apache.hadoop.hive.ql.metadata.Table hmsTable = tbl;
+ TableDesc tableDesc = Utilities.getTableDesc(hmsTable);
+ Table table = Catalogs.loadTable(conf, tableDesc.getProperties());
+ String statsSource = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_USE_STATS_FROM).toLowerCase();
+ switch (statsSource) {
+ case ICEBERG:
+ // Place holder for iceberg stats
+ break;
+ case PUFFIN:
+ String snapshotId = table.name() + table.currentSnapshot().snapshotId();
Review Comment:
Hi Rajesh,
Thanks for the review! :)
Currently, the scope of this patch is for column stats. We can include basic stats in this puffin file as well if needed.
But currently, basics stats for iceberg tables are obtained from `table.currentSnapshot().summary() ` here : https://github.com/apache/hive/blob/master/iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java#L316 .
I do not think we are querying the metastore for basic stats. Please correct me if i am missing something.
--
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] deniskuzZ commented on a diff in pull request #4131: HIVE-27158: Store hive columns stats in puffin files for iceberg tables
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #4131:
URL: https://github.com/apache/hive/pull/4131#discussion_r1148999605
##########
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java:
##########
@@ -349,6 +365,96 @@ public Map<String, String> getBasicStatistics(Partish partish) {
return stats;
}
+
+ @Override
+ public boolean canSetColStatistics() {
+ String statsSource = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_USE_STATS_FROM).toLowerCase();
+ return statsSource.equals(PUFFIN);
+ }
+
+ @Override
+ public boolean canProvideColStatistics(org.apache.hadoop.hive.ql.metadata.Table tbl) {
+
+ org.apache.hadoop.hive.ql.metadata.Table hmsTable = tbl;
+ TableDesc tableDesc = Utilities.getTableDesc(hmsTable);
+ Table table = Catalogs.loadTable(conf, tableDesc.getProperties());
+ if (table.currentSnapshot() != null) {
+ String statsSource = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_COL_STATS_SOURCE).toLowerCase();
+ String statsPath = table.location() + STATS + table.name() + table.currentSnapshot().snapshotId();
Review Comment:
should we wrap table name with some delimiters like '-'? `STATS-customers-122121` is there some extension for the puffin file?
--
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] deniskuzZ commented on a diff in pull request #4131: HIVE-27158: Store hive columns stats in puffin files for iceberg tables
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #4131:
URL: https://github.com/apache/hive/pull/4131#discussion_r1149127885
##########
ql/src/java/org/apache/hadoop/hive/ql/stats/StatsUtils.java:
##########
@@ -1069,8 +1069,12 @@ public static List<ColStatistics> getTableColumnStats(
}
if (fetchColStats && !colStatsToRetrieve.isEmpty()) {
try {
- List<ColumnStatisticsObj> colStat = Hive.get().getTableColumnStatistics(
- dbName, tabName, colStatsToRetrieve, false);
+ List<ColumnStatisticsObj> colStat;
+ if (table != null && table.isNonNative() && table.getStorageHandler().canProvideColStatistics(table)) {
Review Comment:
metastore mode for Iceberg is no longer supported, right? have we disabled to relevant stats persist logic?
--
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 #4131: HIVE-27158: Store hive columns stats in puffin files for iceberg tables
Posted by "simhadri-g (via GitHub)" <gi...@apache.org>.
simhadri-g commented on code in PR #4131:
URL: https://github.com/apache/hive/pull/4131#discussion_r1165323507
##########
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java:
##########
@@ -2205,9 +2205,8 @@ public static enum ConfVars {
"padding tolerance config (hive.exec.orc.block.padding.tolerance)."),
HIVE_ORC_CODEC_POOL("hive.use.orc.codec.pool", false,
"Whether to use codec pool in ORC. Disable if there are bugs with codec reuse."),
- HIVE_USE_STATS_FROM("hive.use.stats.from","iceberg","Use stats from iceberg table snapshot for query " +
- "planning. This has three values metastore, puffin and iceberg"),
-
+ HIVE_ICEBERG_STATS_SOURCE("hive.iceberg.stats.source","iceberg","Use stats from iceberg " +
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] simhadri-g commented on a diff in pull request #4131: HIVE-27158: Store hive columns stats in puffin files for iceberg tables
Posted by "simhadri-g (via GitHub)" <gi...@apache.org>.
simhadri-g commented on code in PR #4131:
URL: https://github.com/apache/hive/pull/4131#discussion_r1161518466
##########
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java:
##########
@@ -349,6 +365,96 @@ public Map<String, String> getBasicStatistics(Partish partish) {
return stats;
}
+
+ @Override
+ public boolean canSetColStatistics() {
+ String statsSource = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_USE_STATS_FROM).toLowerCase();
+ return statsSource.equals(PUFFIN);
Review Comment:
Fixed. Added a default fall back to metastore.
##########
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java:
##########
@@ -349,6 +365,96 @@ public Map<String, String> getBasicStatistics(Partish partish) {
return stats;
}
+
+ @Override
+ public boolean canSetColStatistics() {
+ String statsSource = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_USE_STATS_FROM).toLowerCase();
+ return statsSource.equals(PUFFIN);
+ }
+
+ @Override
+ public boolean canProvideColStatistics(org.apache.hadoop.hive.ql.metadata.Table tbl) {
+
+ org.apache.hadoop.hive.ql.metadata.Table hmsTable = tbl;
Review Comment:
Fixed.
--
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 #4131: HIVE-27158: Store hive columns stats in puffin files for iceberg tables
Posted by "simhadri-g (via GitHub)" <gi...@apache.org>.
simhadri-g commented on code in PR #4131:
URL: https://github.com/apache/hive/pull/4131#discussion_r1161855860
##########
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java:
##########
@@ -2205,9 +2205,8 @@ public static enum ConfVars {
"padding tolerance config (hive.exec.orc.block.padding.tolerance)."),
HIVE_ORC_CODEC_POOL("hive.use.orc.codec.pool", false,
"Whether to use codec pool in ORC. Disable if there are bugs with codec reuse."),
- HIVE_USE_STATS_FROM("hive.use.stats.from","iceberg","Use stats from iceberg table snapshot for query " +
- "planning. This has three values metastore, puffin and iceberg"),
-
+ HIVE_ICEBERG_STATS_SOURCE("hive.iceberg.stats.source","iceberg","Use stats from iceberg table snapshot for query " +
+ "planning. This has three values metastore and iceberg"),
Review Comment:
Fixed. There will be only 2 values.
--
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 #4131: HIVE-27158: Store hive columns stats in puffin files for iceberg tables
Posted by "InvisibleProgrammer (via GitHub)" <gi...@apache.org>.
InvisibleProgrammer commented on PR #4131:
URL: https://github.com/apache/hive/pull/4131#issuecomment-1506552227
I wonder, why it is necessary to check if the tables are not native tables before getting or setting column statistics?
The interface already provides canSet... and canProvide... methods. Isn't it enough to call those methods to decide if it can be done?
```java
if (tbl != null && tbl.isNonNative() && tbl.getStorageHandler().canSetColStatistics(tbl)) {
tbl.getStorageHandler().setColStatistics(tbl, colStats);
}
if (table.isNonNative() && table.getStorageHandler().canProvideColStatistics(table)) {
```
--
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 #4131: HIVE-27158: Store hive columns stats in puffin files for iceberg tables
Posted by "InvisibleProgrammer (via GitHub)" <gi...@apache.org>.
InvisibleProgrammer commented on PR #4131:
URL: https://github.com/apache/hive/pull/4131#issuecomment-1506556000
On overall, LGTM, thank you, @simhadri-g
--
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] deniskuzZ commented on a diff in pull request #4131: HIVE-27158: Store hive columns stats in puffin files for iceberg tables
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #4131:
URL: https://github.com/apache/hive/pull/4131#discussion_r1149015944
##########
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java:
##########
@@ -349,6 +365,96 @@ public Map<String, String> getBasicStatistics(Partish partish) {
return stats;
}
+
+ @Override
+ public boolean canSetColStatistics() {
+ String statsSource = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_USE_STATS_FROM).toLowerCase();
+ return statsSource.equals(PUFFIN);
+ }
+
+ @Override
+ public boolean canProvideColStatistics(org.apache.hadoop.hive.ql.metadata.Table tbl) {
+
+ org.apache.hadoop.hive.ql.metadata.Table hmsTable = tbl;
+ TableDesc tableDesc = Utilities.getTableDesc(hmsTable);
+ Table table = Catalogs.loadTable(conf, tableDesc.getProperties());
+ if (table.currentSnapshot() != null) {
+ String statsSource = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_COL_STATS_SOURCE).toLowerCase();
+ String statsPath = table.location() + STATS + table.name() + table.currentSnapshot().snapshotId();
+ if (statsSource.equals(PUFFIN)) {
+ try (FileSystem fs = new Path(table.location()).getFileSystem(conf)) {
+ if (fs.exists(new Path(statsPath))) {
+ return true;
+ }
+ } catch (IOException e) {
+ LOG.warn(e.getMessage());
+ }
+ }
+ }
+ return false;
+ }
+
+ @Override
+ public List<ColumnStatisticsObj> getColStatistics(org.apache.hadoop.hive.ql.metadata.Table tbl) {
+
+ org.apache.hadoop.hive.ql.metadata.Table hmsTable = tbl;
+ TableDesc tableDesc = Utilities.getTableDesc(hmsTable);
+ Table table = Catalogs.loadTable(conf, tableDesc.getProperties());
+ String statsSource = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_COL_STATS_SOURCE).toLowerCase();
Review Comment:
enum?
--
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] deniskuzZ commented on a diff in pull request #4131: HIVE-27158: Store hive columns stats in puffin files for iceberg tables
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #4131:
URL: https://github.com/apache/hive/pull/4131#discussion_r1148999605
##########
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java:
##########
@@ -349,6 +365,96 @@ public Map<String, String> getBasicStatistics(Partish partish) {
return stats;
}
+
+ @Override
+ public boolean canSetColStatistics() {
+ String statsSource = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_USE_STATS_FROM).toLowerCase();
+ return statsSource.equals(PUFFIN);
+ }
+
+ @Override
+ public boolean canProvideColStatistics(org.apache.hadoop.hive.ql.metadata.Table tbl) {
+
+ org.apache.hadoop.hive.ql.metadata.Table hmsTable = tbl;
+ TableDesc tableDesc = Utilities.getTableDesc(hmsTable);
+ Table table = Catalogs.loadTable(conf, tableDesc.getProperties());
+ if (table.currentSnapshot() != null) {
+ String statsSource = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_COL_STATS_SOURCE).toLowerCase();
+ String statsPath = table.location() + STATS + table.name() + table.currentSnapshot().snapshotId();
Review Comment:
should we wrap table name with some delimiters like '-'? `STATS-customers-122121` is there some extension for the puffin file?
btw, declare this vars under the PUFFIN code block
--
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 a diff in pull request #4131: HIVE-27158: Store hive columns stats in puffin files for iceberg tables
Posted by "InvisibleProgrammer (via GitHub)" <gi...@apache.org>.
InvisibleProgrammer commented on code in PR #4131:
URL: https://github.com/apache/hive/pull/4131#discussion_r1165162320
##########
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java:
##########
@@ -318,7 +335,7 @@ public Map<String, String> getBasicStatistics(Partish partish) {
org.apache.hadoop.hive.ql.metadata.Table hmsTable = partish.getTable();
// For write queries where rows got modified, don't fetch from cache as values could have changed.
Table table = getTable(hmsTable);
- String statsSource = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_USE_STATS_FROM).toLowerCase();
+ String statsSource = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_ICEBERG_STATS_SOURCE).toLowerCase();
Review Comment:
I wonder, what that change exactly changes? As I see, it gets the exact same value now as before the change:
```java
HIVE_USE_STATS_FROM("hive.use.stats.from","iceberg","Use stats from iceberg table snapshot for query " +
"planning. This has three values metastore, puffin and iceberg"),
```
--
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 #4131: HIVE-27158: Store hive columns stats in puffin files for iceberg tables
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #4131:
URL: https://github.com/apache/hive/pull/4131#issuecomment-1502282051
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=4131)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4131&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4131&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4131&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=CODE_SMELL) [12 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4131&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4131&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 #4131: HIVE-27158: Store hive columns stats in puffin files for iceberg tables
Posted by "simhadri-g (via GitHub)" <gi...@apache.org>.
simhadri-g commented on code in PR #4131:
URL: https://github.com/apache/hive/pull/4131#discussion_r1161519355
##########
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java:
##########
@@ -349,6 +365,96 @@ public Map<String, String> getBasicStatistics(Partish partish) {
return stats;
}
+
+ @Override
+ public boolean canSetColStatistics() {
+ String statsSource = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_USE_STATS_FROM).toLowerCase();
+ return statsSource.equals(PUFFIN);
+ }
+
+ @Override
+ public boolean canProvideColStatistics(org.apache.hadoop.hive.ql.metadata.Table tbl) {
+
+ org.apache.hadoop.hive.ql.metadata.Table hmsTable = tbl;
+ TableDesc tableDesc = Utilities.getTableDesc(hmsTable);
+ Table table = Catalogs.loadTable(conf, tableDesc.getProperties());
+ if (table.currentSnapshot() != null) {
+ String statsSource = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_COL_STATS_SOURCE).toLowerCase();
+ String statsPath = table.location() + STATS + table.name() + table.currentSnapshot().snapshotId();
+ if (statsSource.equals(PUFFIN)) {
+ try (FileSystem fs = new Path(table.location()).getFileSystem(conf)) {
+ if (fs.exists(new Path(statsPath))) {
+ return true;
+ }
+ } catch (IOException e) {
+ LOG.warn(e.getMessage());
+ }
+ }
+ }
+ return false;
+ }
+
+ @Override
+ public List<ColumnStatisticsObj> getColStatistics(org.apache.hadoop.hive.ql.metadata.Table tbl) {
+
+ org.apache.hadoop.hive.ql.metadata.Table hmsTable = tbl;
+ TableDesc tableDesc = Utilities.getTableDesc(hmsTable);
+ Table table = Catalogs.loadTable(conf, tableDesc.getProperties());
+ String statsSource = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_COL_STATS_SOURCE).toLowerCase();
+ switch (statsSource) {
+ case ICEBERG:
+ // Place holder for iceberg stats
+ break;
+ case PUFFIN:
+ String snapshotId = table.name() + table.currentSnapshot().snapshotId();
+ String statsPath = table.location() + STATS + snapshotId;
+ LOG.info("Using stats from puffin file at:" + statsPath);
+ try (PuffinReader reader = Puffin.read(table.io().newInputFile(statsPath)).build()) {
+ BlobMetadata blobMetadata = reader.fileMetadata().blobs().get(0);
+ Map<BlobMetadata, List<ColumnStatistics>> collect =
+ Streams.stream(reader.readAll(ImmutableList.of(blobMetadata))).collect(Collectors.toMap(Pair::first,
Review Comment:
Fixed.
--
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 #4131: HIVE-27158: Store hive columns stats in puffin files for iceberg tables
Posted by "simhadri-g (via GitHub)" <gi...@apache.org>.
simhadri-g commented on code in PR #4131:
URL: https://github.com/apache/hive/pull/4131#discussion_r1161518779
##########
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java:
##########
@@ -349,6 +365,96 @@ public Map<String, String> getBasicStatistics(Partish partish) {
return stats;
}
+
+ @Override
+ public boolean canSetColStatistics() {
+ String statsSource = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_USE_STATS_FROM).toLowerCase();
+ return statsSource.equals(PUFFIN);
+ }
+
+ @Override
+ public boolean canProvideColStatistics(org.apache.hadoop.hive.ql.metadata.Table tbl) {
+
+ org.apache.hadoop.hive.ql.metadata.Table hmsTable = tbl;
+ TableDesc tableDesc = Utilities.getTableDesc(hmsTable);
+ Table table = Catalogs.loadTable(conf, tableDesc.getProperties());
+ if (table.currentSnapshot() != null) {
+ String statsSource = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_COL_STATS_SOURCE).toLowerCase();
+ String statsPath = table.location() + STATS + table.name() + table.currentSnapshot().snapshotId();
+ if (statsSource.equals(PUFFIN)) {
Review Comment:
Fixed.
##########
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java:
##########
@@ -349,6 +365,96 @@ public Map<String, String> getBasicStatistics(Partish partish) {
return stats;
}
+
+ @Override
+ public boolean canSetColStatistics() {
+ String statsSource = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_USE_STATS_FROM).toLowerCase();
+ return statsSource.equals(PUFFIN);
+ }
+
+ @Override
+ public boolean canProvideColStatistics(org.apache.hadoop.hive.ql.metadata.Table tbl) {
+
+ org.apache.hadoop.hive.ql.metadata.Table hmsTable = tbl;
+ TableDesc tableDesc = Utilities.getTableDesc(hmsTable);
+ Table table = Catalogs.loadTable(conf, tableDesc.getProperties());
+ if (table.currentSnapshot() != null) {
+ String statsSource = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_COL_STATS_SOURCE).toLowerCase();
+ String statsPath = table.location() + STATS + table.name() + table.currentSnapshot().snapshotId();
+ if (statsSource.equals(PUFFIN)) {
+ try (FileSystem fs = new Path(table.location()).getFileSystem(conf)) {
+ if (fs.exists(new Path(statsPath))) {
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] simhadri-g commented on a diff in pull request #4131: HIVE-27158: Store hive columns stats in puffin files for iceberg tables
Posted by "simhadri-g (via GitHub)" <gi...@apache.org>.
simhadri-g commented on code in PR #4131:
URL: https://github.com/apache/hive/pull/4131#discussion_r1161518101
##########
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java:
##########
@@ -2207,6 +2207,8 @@ public static enum ConfVars {
"Whether to use codec pool in ORC. Disable if there are bugs with codec reuse."),
HIVE_USE_STATS_FROM("hive.use.stats.from","iceberg","Use stats from iceberg table snapshot for query " +
"planning. This has three values metastore, puffin and iceberg"),
+ HIVE_COL_STATS_SOURCE("hive.col.stats.source","metastore","Use stats from puffin file for query " +
Review Comment:
Fixed, merged the confs to a single conf.
--
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 #4131: HIVE-27158: Store hive columns stats in puffin files for iceberg tables
Posted by "simhadri-g (via GitHub)" <gi...@apache.org>.
simhadri-g commented on code in PR #4131:
URL: https://github.com/apache/hive/pull/4131#discussion_r1161857489
##########
ql/src/java/org/apache/hadoop/hive/ql/stats/StatsUtils.java:
##########
@@ -1069,8 +1069,12 @@ public static List<ColStatistics> getTableColumnStats(
}
if (fetchColStats && !colStatsToRetrieve.isEmpty()) {
try {
- List<ColumnStatisticsObj> colStat = Hive.get().getTableColumnStatistics(
- dbName, tabName, colStatsToRetrieve, false);
+ List<ColumnStatisticsObj> colStat;
+ if (table != null && table.isNonNative() && table.getStorageHandler().canProvideColStatistics(table)) {
Review Comment:
Fixed
--
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 #4131: HIVE-27158: Store hive columns stats in puffin files for iceberg tables
Posted by "simhadri-g (via GitHub)" <gi...@apache.org>.
simhadri-g commented on code in PR #4131:
URL: https://github.com/apache/hive/pull/4131#discussion_r1161530815
##########
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java:
##########
@@ -349,6 +365,96 @@ public Map<String, String> getBasicStatistics(Partish partish) {
return stats;
}
+
+ @Override
+ public boolean canSetColStatistics() {
+ String statsSource = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_USE_STATS_FROM).toLowerCase();
+ return statsSource.equals(PUFFIN);
+ }
+
+ @Override
+ public boolean canProvideColStatistics(org.apache.hadoop.hive.ql.metadata.Table tbl) {
+
+ org.apache.hadoop.hive.ql.metadata.Table hmsTable = tbl;
+ TableDesc tableDesc = Utilities.getTableDesc(hmsTable);
+ Table table = Catalogs.loadTable(conf, tableDesc.getProperties());
+ if (table.currentSnapshot() != null) {
+ String statsSource = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_COL_STATS_SOURCE).toLowerCase();
+ String statsPath = table.location() + STATS + table.name() + table.currentSnapshot().snapshotId();
+ if (statsSource.equals(PUFFIN)) {
+ try (FileSystem fs = new Path(table.location()).getFileSystem(conf)) {
+ if (fs.exists(new Path(statsPath))) {
+ return true;
+ }
+ } catch (IOException e) {
+ LOG.warn(e.getMessage());
+ }
+ }
+ }
+ return false;
+ }
+
+ @Override
+ public List<ColumnStatisticsObj> getColStatistics(org.apache.hadoop.hive.ql.metadata.Table tbl) {
+
+ org.apache.hadoop.hive.ql.metadata.Table hmsTable = tbl;
+ TableDesc tableDesc = Utilities.getTableDesc(hmsTable);
+ Table table = Catalogs.loadTable(conf, tableDesc.getProperties());
+ String statsSource = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_COL_STATS_SOURCE).toLowerCase();
+ switch (statsSource) {
+ case ICEBERG:
+ // Place holder for iceberg stats
+ break;
+ case PUFFIN:
+ String snapshotId = table.name() + table.currentSnapshot().snapshotId();
+ String statsPath = table.location() + STATS + snapshotId;
+ LOG.info("Using stats from puffin file at:" + statsPath);
+ try (PuffinReader reader = Puffin.read(table.io().newInputFile(statsPath)).build()) {
+ BlobMetadata blobMetadata = reader.fileMetadata().blobs().get(0);
+ Map<BlobMetadata, List<ColumnStatistics>> collect =
+ Streams.stream(reader.readAll(ImmutableList.of(blobMetadata))).collect(Collectors.toMap(Pair::first,
+ blobMetadataByteBufferPair -> SerializationUtils.deserialize(
+ ByteBuffers.toByteArray(blobMetadataByteBufferPair.second()))));
+
+ return collect.entrySet().stream().iterator().next().getValue().get(0).getStatsObj();
+ } catch (IOException e) {
+ LOG.info(String.valueOf(e));
+ }
+ break;
+ default:
+ // fall back to metastore
+ }
+ return null;
+ }
+
+
+ @Override
+ public boolean setColStatistics(org.apache.hadoop.hive.ql.metadata.Table table,
+ List<ColumnStatistics> colStats) {
+ TableDesc tableDesc = Utilities.getTableDesc(table);
+ Table tbl = Catalogs.loadTable(conf, tableDesc.getProperties());
+ String snapshotId = tbl.name() + tbl.currentSnapshot().snapshotId();
+ byte[] serializeColStats = SerializationUtils.serialize((Serializable) colStats);
Review Comment:
We are checking if the colStats is empty here.
https://github.com/apache/hive/blob/master/ql/src/java/org/apache/hadoop/hive/ql/stats/ColStatsProcessor.java#L208
--
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 #4131: HIVE-27158: Store hive columns stats in puffin files for iceberg tables
Posted by "simhadri-g (via GitHub)" <gi...@apache.org>.
simhadri-g commented on code in PR #4131:
URL: https://github.com/apache/hive/pull/4131#discussion_r1161519017
##########
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java:
##########
@@ -349,6 +365,96 @@ public Map<String, String> getBasicStatistics(Partish partish) {
return stats;
}
+
+ @Override
+ public boolean canSetColStatistics() {
+ String statsSource = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_USE_STATS_FROM).toLowerCase();
+ return statsSource.equals(PUFFIN);
+ }
+
+ @Override
+ public boolean canProvideColStatistics(org.apache.hadoop.hive.ql.metadata.Table tbl) {
+
+ org.apache.hadoop.hive.ql.metadata.Table hmsTable = tbl;
+ TableDesc tableDesc = Utilities.getTableDesc(hmsTable);
+ Table table = Catalogs.loadTable(conf, tableDesc.getProperties());
+ if (table.currentSnapshot() != null) {
+ String statsSource = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_COL_STATS_SOURCE).toLowerCase();
+ String statsPath = table.location() + STATS + table.name() + table.currentSnapshot().snapshotId();
+ if (statsSource.equals(PUFFIN)) {
+ try (FileSystem fs = new Path(table.location()).getFileSystem(conf)) {
+ if (fs.exists(new Path(statsPath))) {
+ return true;
+ }
+ } catch (IOException e) {
+ LOG.warn(e.getMessage());
+ }
+ }
+ }
+ return false;
+ }
+
+ @Override
+ public List<ColumnStatisticsObj> getColStatistics(org.apache.hadoop.hive.ql.metadata.Table tbl) {
+
+ org.apache.hadoop.hive.ql.metadata.Table hmsTable = tbl;
Review Comment:
fixed
##########
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java:
##########
@@ -349,6 +365,96 @@ public Map<String, String> getBasicStatistics(Partish partish) {
return stats;
}
+
+ @Override
+ public boolean canSetColStatistics() {
+ String statsSource = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_USE_STATS_FROM).toLowerCase();
+ return statsSource.equals(PUFFIN);
+ }
+
+ @Override
+ public boolean canProvideColStatistics(org.apache.hadoop.hive.ql.metadata.Table tbl) {
+
+ org.apache.hadoop.hive.ql.metadata.Table hmsTable = tbl;
+ TableDesc tableDesc = Utilities.getTableDesc(hmsTable);
+ Table table = Catalogs.loadTable(conf, tableDesc.getProperties());
+ if (table.currentSnapshot() != null) {
+ String statsSource = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_COL_STATS_SOURCE).toLowerCase();
+ String statsPath = table.location() + STATS + table.name() + table.currentSnapshot().snapshotId();
+ if (statsSource.equals(PUFFIN)) {
+ try (FileSystem fs = new Path(table.location()).getFileSystem(conf)) {
+ if (fs.exists(new Path(statsPath))) {
+ return true;
+ }
+ } catch (IOException e) {
+ LOG.warn(e.getMessage());
+ }
+ }
+ }
+ return false;
+ }
+
+ @Override
+ public List<ColumnStatisticsObj> getColStatistics(org.apache.hadoop.hive.ql.metadata.Table tbl) {
+
+ org.apache.hadoop.hive.ql.metadata.Table hmsTable = tbl;
+ TableDesc tableDesc = Utilities.getTableDesc(hmsTable);
+ Table table = Catalogs.loadTable(conf, tableDesc.getProperties());
+ String statsSource = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_COL_STATS_SOURCE).toLowerCase();
+ switch (statsSource) {
+ case ICEBERG:
+ // Place holder for iceberg stats
+ break;
+ case PUFFIN:
+ String snapshotId = table.name() + table.currentSnapshot().snapshotId();
+ String statsPath = table.location() + STATS + snapshotId;
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 a diff in pull request #4131: HIVE-27158: Store hive columns stats in puffin files for iceberg tables
Posted by "InvisibleProgrammer (via GitHub)" <gi...@apache.org>.
InvisibleProgrammer commented on code in PR #4131:
URL: https://github.com/apache/hive/pull/4131#discussion_r1165152436
##########
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java:
##########
@@ -2205,9 +2205,8 @@ public static enum ConfVars {
"padding tolerance config (hive.exec.orc.block.padding.tolerance)."),
HIVE_ORC_CODEC_POOL("hive.use.orc.codec.pool", false,
"Whether to use codec pool in ORC. Disable if there are bugs with codec reuse."),
- HIVE_USE_STATS_FROM("hive.use.stats.from","iceberg","Use stats from iceberg table snapshot for query " +
- "planning. This has three values metastore, puffin and iceberg"),
-
+ HIVE_ICEBERG_STATS_SOURCE("hive.iceberg.stats.source","iceberg","Use stats from iceberg " +
Review Comment:
Is it worth adding a PatternSet as it is used for `HIVESTATSDBCLASS("hive.stats.dbclass", "fs", new PatternSet("custom", "fs"),`?
In that way, it would be easy to understand what are the accepted values, for the first look at the code.
Also, I wonder, if is there any other way to do similar stuff with settings?
--
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] deniskuzZ commented on a diff in pull request #4131: HIVE-27158: Store hive columns stats in puffin files for iceberg tables
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #4131:
URL: https://github.com/apache/hive/pull/4131#discussion_r1149008394
##########
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java:
##########
@@ -349,6 +365,96 @@ public Map<String, String> getBasicStatistics(Partish partish) {
return stats;
}
+
+ @Override
+ public boolean canSetColStatistics() {
+ String statsSource = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_USE_STATS_FROM).toLowerCase();
+ return statsSource.equals(PUFFIN);
+ }
+
+ @Override
+ public boolean canProvideColStatistics(org.apache.hadoop.hive.ql.metadata.Table tbl) {
+
+ org.apache.hadoop.hive.ql.metadata.Table hmsTable = tbl;
+ TableDesc tableDesc = Utilities.getTableDesc(hmsTable);
+ Table table = Catalogs.loadTable(conf, tableDesc.getProperties());
+ if (table.currentSnapshot() != null) {
+ String statsSource = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_COL_STATS_SOURCE).toLowerCase();
+ String statsPath = table.location() + STATS + table.name() + table.currentSnapshot().snapshotId();
+ if (statsSource.equals(PUFFIN)) {
+ try (FileSystem fs = new Path(table.location()).getFileSystem(conf)) {
+ if (fs.exists(new Path(statsPath))) {
Review Comment:
can we create a path variable just once?
````
Path statsPath = new Path(...);
try (FileSystem fs = statsPath.getFileSystem(conf)) {
return fs.exists(statsPath));
}
````
--
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] deniskuzZ commented on a diff in pull request #4131: HIVE-27158: Store hive columns stats in puffin files for iceberg tables
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #4131:
URL: https://github.com/apache/hive/pull/4131#discussion_r1148989152
##########
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java:
##########
@@ -2207,6 +2207,8 @@ public static enum ConfVars {
"Whether to use codec pool in ORC. Disable if there are bugs with codec reuse."),
HIVE_USE_STATS_FROM("hive.use.stats.from","iceberg","Use stats from iceberg table snapshot for query " +
"planning. This has three values metastore, puffin and iceberg"),
+ HIVE_COL_STATS_SOURCE("hive.col.stats.source","metastore","Use stats from puffin file for query " +
Review Comment:
- do we support `iceberg` mode?
- please update the description - "Use stats from the selected source for query planning"
what's the difference between `HIVE_USE_STATS_FROM` && `HIVE_COL_STATS_SOURCE `? it doesn't seem to be a generic config, should we add an ICEBERG prefix?
--
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] deniskuzZ commented on a diff in pull request #4131: HIVE-27158: Store hive columns stats in puffin files for iceberg tables
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #4131:
URL: https://github.com/apache/hive/pull/4131#discussion_r1149033188
##########
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java:
##########
@@ -349,6 +365,96 @@ public Map<String, String> getBasicStatistics(Partish partish) {
return stats;
}
+
+ @Override
+ public boolean canSetColStatistics() {
+ String statsSource = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_USE_STATS_FROM).toLowerCase();
+ return statsSource.equals(PUFFIN);
+ }
+
+ @Override
+ public boolean canProvideColStatistics(org.apache.hadoop.hive.ql.metadata.Table tbl) {
+
+ org.apache.hadoop.hive.ql.metadata.Table hmsTable = tbl;
+ TableDesc tableDesc = Utilities.getTableDesc(hmsTable);
+ Table table = Catalogs.loadTable(conf, tableDesc.getProperties());
+ if (table.currentSnapshot() != null) {
+ String statsSource = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_COL_STATS_SOURCE).toLowerCase();
+ String statsPath = table.location() + STATS + table.name() + table.currentSnapshot().snapshotId();
+ if (statsSource.equals(PUFFIN)) {
+ try (FileSystem fs = new Path(table.location()).getFileSystem(conf)) {
+ if (fs.exists(new Path(statsPath))) {
+ return true;
+ }
+ } catch (IOException e) {
+ LOG.warn(e.getMessage());
+ }
+ }
+ }
+ return false;
+ }
+
+ @Override
+ public List<ColumnStatisticsObj> getColStatistics(org.apache.hadoop.hive.ql.metadata.Table tbl) {
+
+ org.apache.hadoop.hive.ql.metadata.Table hmsTable = tbl;
+ TableDesc tableDesc = Utilities.getTableDesc(hmsTable);
+ Table table = Catalogs.loadTable(conf, tableDesc.getProperties());
+ String statsSource = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_COL_STATS_SOURCE).toLowerCase();
+ switch (statsSource) {
+ case ICEBERG:
+ // Place holder for iceberg stats
+ break;
+ case PUFFIN:
+ String snapshotId = table.name() + table.currentSnapshot().snapshotId();
+ String statsPath = table.location() + STATS + snapshotId;
+ LOG.info("Using stats from puffin file at:" + statsPath);
+ try (PuffinReader reader = Puffin.read(table.io().newInputFile(statsPath)).build()) {
+ BlobMetadata blobMetadata = reader.fileMetadata().blobs().get(0);
+ Map<BlobMetadata, List<ColumnStatistics>> collect =
+ Streams.stream(reader.readAll(ImmutableList.of(blobMetadata))).collect(Collectors.toMap(Pair::first,
+ blobMetadataByteBufferPair -> SerializationUtils.deserialize(
+ ByteBuffers.toByteArray(blobMetadataByteBufferPair.second()))));
+
+ return collect.entrySet().stream().iterator().next().getValue().get(0).getStatsObj();
+ } catch (IOException e) {
+ LOG.info(String.valueOf(e));
+ }
+ break;
+ default:
+ // fall back to metastore
+ }
+ return null;
+ }
+
+
+ @Override
+ public boolean setColStatistics(org.apache.hadoop.hive.ql.metadata.Table table,
+ List<ColumnStatistics> colStats) {
+ TableDesc tableDesc = Utilities.getTableDesc(table);
+ Table tbl = Catalogs.loadTable(conf, tableDesc.getProperties());
+ String snapshotId = tbl.name() + tbl.currentSnapshot().snapshotId();
+ byte[] serializeColStats = SerializationUtils.serialize((Serializable) colStats);
Review Comment:
should we check if for NULL here?
--
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] deniskuzZ commented on a diff in pull request #4131: HIVE-27158: Store hive columns stats in puffin files for iceberg tables
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #4131:
URL: https://github.com/apache/hive/pull/4131#discussion_r1149031483
##########
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java:
##########
@@ -349,6 +365,96 @@ public Map<String, String> getBasicStatistics(Partish partish) {
return stats;
}
+
+ @Override
+ public boolean canSetColStatistics() {
+ String statsSource = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_USE_STATS_FROM).toLowerCase();
+ return statsSource.equals(PUFFIN);
+ }
+
+ @Override
+ public boolean canProvideColStatistics(org.apache.hadoop.hive.ql.metadata.Table tbl) {
+
+ org.apache.hadoop.hive.ql.metadata.Table hmsTable = tbl;
+ TableDesc tableDesc = Utilities.getTableDesc(hmsTable);
+ Table table = Catalogs.loadTable(conf, tableDesc.getProperties());
+ if (table.currentSnapshot() != null) {
+ String statsSource = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_COL_STATS_SOURCE).toLowerCase();
+ String statsPath = table.location() + STATS + table.name() + table.currentSnapshot().snapshotId();
+ if (statsSource.equals(PUFFIN)) {
+ try (FileSystem fs = new Path(table.location()).getFileSystem(conf)) {
+ if (fs.exists(new Path(statsPath))) {
+ return true;
+ }
+ } catch (IOException e) {
+ LOG.warn(e.getMessage());
+ }
+ }
+ }
+ return false;
+ }
+
+ @Override
+ public List<ColumnStatisticsObj> getColStatistics(org.apache.hadoop.hive.ql.metadata.Table tbl) {
+
+ org.apache.hadoop.hive.ql.metadata.Table hmsTable = tbl;
+ TableDesc tableDesc = Utilities.getTableDesc(hmsTable);
+ Table table = Catalogs.loadTable(conf, tableDesc.getProperties());
+ String statsSource = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_COL_STATS_SOURCE).toLowerCase();
+ switch (statsSource) {
+ case ICEBERG:
+ // Place holder for iceberg stats
+ break;
+ case PUFFIN:
+ String snapshotId = table.name() + table.currentSnapshot().snapshotId();
+ String statsPath = table.location() + STATS + snapshotId;
+ LOG.info("Using stats from puffin file at:" + statsPath);
+ try (PuffinReader reader = Puffin.read(table.io().newInputFile(statsPath)).build()) {
+ BlobMetadata blobMetadata = reader.fileMetadata().blobs().get(0);
+ Map<BlobMetadata, List<ColumnStatistics>> collect =
+ Streams.stream(reader.readAll(ImmutableList.of(blobMetadata))).collect(Collectors.toMap(Pair::first,
+ blobMetadataByteBufferPair -> SerializationUtils.deserialize(
+ ByteBuffers.toByteArray(blobMetadataByteBufferPair.second()))));
+
+ return collect.entrySet().stream().iterator().next().getValue().get(0).getStatsObj();
+ } catch (IOException e) {
+ LOG.info(String.valueOf(e));
Review Comment:
why are we swallowing exception here?
--
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] deniskuzZ commented on a diff in pull request #4131: HIVE-27158: Store hive columns stats in puffin files for iceberg tables
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #4131:
URL: https://github.com/apache/hive/pull/4131#discussion_r1149030014
##########
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java:
##########
@@ -349,6 +365,96 @@ public Map<String, String> getBasicStatistics(Partish partish) {
return stats;
}
+
+ @Override
+ public boolean canSetColStatistics() {
+ String statsSource = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_USE_STATS_FROM).toLowerCase();
+ return statsSource.equals(PUFFIN);
+ }
+
+ @Override
+ public boolean canProvideColStatistics(org.apache.hadoop.hive.ql.metadata.Table tbl) {
+
+ org.apache.hadoop.hive.ql.metadata.Table hmsTable = tbl;
+ TableDesc tableDesc = Utilities.getTableDesc(hmsTable);
+ Table table = Catalogs.loadTable(conf, tableDesc.getProperties());
+ if (table.currentSnapshot() != null) {
+ String statsSource = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_COL_STATS_SOURCE).toLowerCase();
+ String statsPath = table.location() + STATS + table.name() + table.currentSnapshot().snapshotId();
+ if (statsSource.equals(PUFFIN)) {
+ try (FileSystem fs = new Path(table.location()).getFileSystem(conf)) {
+ if (fs.exists(new Path(statsPath))) {
+ return true;
+ }
+ } catch (IOException e) {
+ LOG.warn(e.getMessage());
+ }
+ }
+ }
+ return false;
+ }
+
+ @Override
+ public List<ColumnStatisticsObj> getColStatistics(org.apache.hadoop.hive.ql.metadata.Table tbl) {
+
+ org.apache.hadoop.hive.ql.metadata.Table hmsTable = tbl;
+ TableDesc tableDesc = Utilities.getTableDesc(hmsTable);
+ Table table = Catalogs.loadTable(conf, tableDesc.getProperties());
+ String statsSource = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_COL_STATS_SOURCE).toLowerCase();
+ switch (statsSource) {
+ case ICEBERG:
+ // Place holder for iceberg stats
+ break;
+ case PUFFIN:
+ String snapshotId = table.name() + table.currentSnapshot().snapshotId();
+ String statsPath = table.location() + STATS + snapshotId;
+ LOG.info("Using stats from puffin file at:" + statsPath);
+ try (PuffinReader reader = Puffin.read(table.io().newInputFile(statsPath)).build()) {
+ BlobMetadata blobMetadata = reader.fileMetadata().blobs().get(0);
+ Map<BlobMetadata, List<ColumnStatistics>> collect =
+ Streams.stream(reader.readAll(ImmutableList.of(blobMetadata))).collect(Collectors.toMap(Pair::first,
+ blobMetadataByteBufferPair -> SerializationUtils.deserialize(
+ ByteBuffers.toByteArray(blobMetadataByteBufferPair.second()))));
+
+ return collect.entrySet().stream().iterator().next().getValue().get(0).getStatsObj();
Review Comment:
why do you need a map in the first place?
--
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] deniskuzZ commented on a diff in pull request #4131: HIVE-27158: Store hive columns stats in puffin files for iceberg tables
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #4131:
URL: https://github.com/apache/hive/pull/4131#discussion_r1149027518
##########
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java:
##########
@@ -349,6 +365,96 @@ public Map<String, String> getBasicStatistics(Partish partish) {
return stats;
}
+
+ @Override
+ public boolean canSetColStatistics() {
+ String statsSource = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_USE_STATS_FROM).toLowerCase();
+ return statsSource.equals(PUFFIN);
+ }
+
+ @Override
+ public boolean canProvideColStatistics(org.apache.hadoop.hive.ql.metadata.Table tbl) {
+
+ org.apache.hadoop.hive.ql.metadata.Table hmsTable = tbl;
+ TableDesc tableDesc = Utilities.getTableDesc(hmsTable);
+ Table table = Catalogs.loadTable(conf, tableDesc.getProperties());
+ if (table.currentSnapshot() != null) {
+ String statsSource = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_COL_STATS_SOURCE).toLowerCase();
+ String statsPath = table.location() + STATS + table.name() + table.currentSnapshot().snapshotId();
+ if (statsSource.equals(PUFFIN)) {
+ try (FileSystem fs = new Path(table.location()).getFileSystem(conf)) {
+ if (fs.exists(new Path(statsPath))) {
+ return true;
+ }
+ } catch (IOException e) {
+ LOG.warn(e.getMessage());
+ }
+ }
+ }
+ return false;
+ }
+
+ @Override
+ public List<ColumnStatisticsObj> getColStatistics(org.apache.hadoop.hive.ql.metadata.Table tbl) {
+
+ org.apache.hadoop.hive.ql.metadata.Table hmsTable = tbl;
+ TableDesc tableDesc = Utilities.getTableDesc(hmsTable);
+ Table table = Catalogs.loadTable(conf, tableDesc.getProperties());
+ String statsSource = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_COL_STATS_SOURCE).toLowerCase();
+ switch (statsSource) {
+ case ICEBERG:
+ // Place holder for iceberg stats
+ break;
+ case PUFFIN:
+ String snapshotId = table.name() + table.currentSnapshot().snapshotId();
+ String statsPath = table.location() + STATS + snapshotId;
+ LOG.info("Using stats from puffin file at:" + statsPath);
+ try (PuffinReader reader = Puffin.read(table.io().newInputFile(statsPath)).build()) {
+ BlobMetadata blobMetadata = reader.fileMetadata().blobs().get(0);
+ Map<BlobMetadata, List<ColumnStatistics>> collect =
+ Streams.stream(reader.readAll(ImmutableList.of(blobMetadata))).collect(Collectors.toMap(Pair::first,
Review Comment:
why do you need to wrap with ImmutableList.of(blobMetadata) ?
--
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 a diff in pull request #4131: HIVE-27158: Store hive columns stats in puffin files for iceberg tables
Posted by "InvisibleProgrammer (via GitHub)" <gi...@apache.org>.
InvisibleProgrammer commented on code in PR #4131:
URL: https://github.com/apache/hive/pull/4131#discussion_r1165148424
##########
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java:
##########
@@ -2205,9 +2205,8 @@ public static enum ConfVars {
"padding tolerance config (hive.exec.orc.block.padding.tolerance)."),
HIVE_ORC_CODEC_POOL("hive.use.orc.codec.pool", false,
"Whether to use codec pool in ORC. Disable if there are bugs with codec reuse."),
- HIVE_USE_STATS_FROM("hive.use.stats.from","iceberg","Use stats from iceberg table snapshot for query " +
- "planning. This has three values metastore, puffin and iceberg"),
-
+ HIVE_ICEBERG_STATS_SOURCE("hive.iceberg.stats.source","iceberg","Use stats from iceberg " +
Review Comment:
Can I ask you to put an extra line after the default value? I read it about three times trying to find what is the default. It would be easier to understand it when you look at it the first time.
--
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 #4131: HIVE-27158: Store hive columns stats in puffin files for iceberg tables
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #4131:
URL: https://github.com/apache/hive/pull/4131#issuecomment-1506912828
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=4131)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4131&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4131&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4131&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=CODE_SMELL) [6 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4131&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4131&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 #4131: HIVE-27158: Store hive columns stats in puffin files for iceberg tables
Posted by "simhadri-g (via GitHub)" <gi...@apache.org>.
simhadri-g commented on code in PR #4131:
URL: https://github.com/apache/hive/pull/4131#discussion_r1161857242
##########
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java:
##########
@@ -361,6 +378,83 @@ private Table getTable(org.apache.hadoop.hive.ql.metadata.Table hmsTable) {
return table;
}
+
+ @Override
+ public boolean canSetColStatistics() {
+ return getStatsSource().equals(ICEBERG);
+ }
+
+ @Override
+ public boolean canProvideColStatistics(org.apache.hadoop.hive.ql.metadata.Table hmsTable) {
+ Table table = Catalogs.loadTable(conf, Utilities.getTableDesc(hmsTable).getProperties());
+ if (table.currentSnapshot() != null) {
+ Path statsPath = getStatsPath(table);
+ if (getStatsSource().equals(ICEBERG)) {
+ try (FileSystem fs = statsPath.getFileSystem(conf)) {
+ if (fs.exists(statsPath)) {
+ return true;
+ }
+ } catch (IOException e) {
+ LOG.warn(e.getMessage());
+ }
+ }
+ }
+ return false;
+ }
+
+ @Override
+ public List<ColumnStatisticsObj> getColStatistics(org.apache.hadoop.hive.ql.metadata.Table hmsTable) {
+ Table table = Catalogs.loadTable(conf, Utilities.getTableDesc(hmsTable).getProperties());
+ String statsPath = getStatsPath(table).toString();
+ LOG.info("Using stats from puffin file at:" + statsPath);
+ try (PuffinReader reader = Puffin.read(table.io().newInputFile(statsPath)).build()) {
+ List<BlobMetadata> blobMetadata = reader.fileMetadata().blobs();
+ Map<BlobMetadata, List<ColumnStatistics>> collect =
+ Streams.stream(reader.readAll(blobMetadata)).collect(Collectors.toMap(Pair::first,
+ blobMetadataByteBufferPair -> SerializationUtils.deserialize(
+ ByteBuffers.toByteArray(blobMetadataByteBufferPair.second()))));
+ return collect.get(blobMetadata.get(0)).get(0).getStatsObj();
+ } catch (IOException e) {
+ LOG.error(String.valueOf(e));
+ }
+ return null;
+ }
+
+
+ @Override
+ public boolean setColStatistics(org.apache.hadoop.hive.ql.metadata.Table table,
+ List<ColumnStatistics> colStats) {
+ TableDesc tableDesc = Utilities.getTableDesc(table);
+ Table tbl = Catalogs.loadTable(conf, tableDesc.getProperties());
+ String snapshotId = tbl.name() + tbl.currentSnapshot().snapshotId();
+ byte[] serializeColStats = SerializationUtils.serialize((Serializable) colStats);
+
+ try (PuffinWriter writer = Puffin.write(tbl.io().newOutputFile(getStatsPath(tbl).toString()))
+ .createdBy("Hive").build()) {
Review Comment:
Done
##########
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java:
##########
@@ -361,6 +378,83 @@ private Table getTable(org.apache.hadoop.hive.ql.metadata.Table hmsTable) {
return table;
}
+
+ @Override
+ public boolean canSetColStatistics() {
+ return getStatsSource().equals(ICEBERG);
+ }
+
+ @Override
+ public boolean canProvideColStatistics(org.apache.hadoop.hive.ql.metadata.Table hmsTable) {
+ Table table = Catalogs.loadTable(conf, Utilities.getTableDesc(hmsTable).getProperties());
+ if (table.currentSnapshot() != null) {
+ Path statsPath = getStatsPath(table);
+ if (getStatsSource().equals(ICEBERG)) {
+ try (FileSystem fs = statsPath.getFileSystem(conf)) {
+ if (fs.exists(statsPath)) {
+ return true;
+ }
+ } catch (IOException e) {
+ LOG.warn(e.getMessage());
+ }
+ }
+ }
+ return false;
+ }
+
+ @Override
+ public List<ColumnStatisticsObj> getColStatistics(org.apache.hadoop.hive.ql.metadata.Table hmsTable) {
+ Table table = Catalogs.loadTable(conf, Utilities.getTableDesc(hmsTable).getProperties());
+ String statsPath = getStatsPath(table).toString();
+ LOG.info("Using stats from puffin file at:" + statsPath);
+ try (PuffinReader reader = Puffin.read(table.io().newInputFile(statsPath)).build()) {
+ List<BlobMetadata> blobMetadata = reader.fileMetadata().blobs();
+ Map<BlobMetadata, List<ColumnStatistics>> collect =
+ Streams.stream(reader.readAll(blobMetadata)).collect(Collectors.toMap(Pair::first,
+ blobMetadataByteBufferPair -> SerializationUtils.deserialize(
+ ByteBuffers.toByteArray(blobMetadataByteBufferPair.second()))));
+ return collect.get(blobMetadata.get(0)).get(0).getStatsObj();
+ } catch (IOException e) {
+ LOG.error(String.valueOf(e));
+ }
+ return null;
+ }
+
+
+ @Override
+ public boolean setColStatistics(org.apache.hadoop.hive.ql.metadata.Table table,
+ List<ColumnStatistics> colStats) {
+ TableDesc tableDesc = Utilities.getTableDesc(table);
+ Table tbl = Catalogs.loadTable(conf, tableDesc.getProperties());
+ String snapshotId = tbl.name() + tbl.currentSnapshot().snapshotId();
+ byte[] serializeColStats = SerializationUtils.serialize((Serializable) colStats);
+
+ try (PuffinWriter writer = Puffin.write(tbl.io().newOutputFile(getStatsPath(tbl).toString()))
+ .createdBy("Hive").build()) {
+ writer.add(
+ new Blob(
+ tbl.name() + "-" + snapshotId,
+ ImmutableList.of(1),
+ tbl.currentSnapshot().snapshotId(),
+ tbl.currentSnapshot().sequenceNumber(),
+ ByteBuffer.wrap(serializeColStats),
+ PuffinCompressionCodec.NONE,
+ ImmutableMap.of()));
+ writer.finish();
+ } catch (IOException e) {
+ LOG.error(String.valueOf(e));
+ }
+ return false;
+ }
+
+ private String getStatsSource() {
+ return HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_ICEBERG_STATS_SOURCE, "metastore").toLowerCase();
+ }
Review Comment:
Fixed
--
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 #4131: HIVE-27158: Store hive columns stats in puffin files for iceberg tables
Posted by "simhadri-g (via GitHub)" <gi...@apache.org>.
simhadri-g commented on code in PR #4131:
URL: https://github.com/apache/hive/pull/4131#discussion_r1161857030
##########
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java:
##########
@@ -361,6 +378,83 @@ private Table getTable(org.apache.hadoop.hive.ql.metadata.Table hmsTable) {
return table;
}
+
+ @Override
+ public boolean canSetColStatistics() {
+ return getStatsSource().equals(ICEBERG);
+ }
+
+ @Override
+ public boolean canProvideColStatistics(org.apache.hadoop.hive.ql.metadata.Table hmsTable) {
+ Table table = Catalogs.loadTable(conf, Utilities.getTableDesc(hmsTable).getProperties());
+ if (table.currentSnapshot() != null) {
+ Path statsPath = getStatsPath(table);
+ if (getStatsSource().equals(ICEBERG)) {
+ try (FileSystem fs = statsPath.getFileSystem(conf)) {
+ if (fs.exists(statsPath)) {
+ return true;
+ }
+ } catch (IOException e) {
+ LOG.warn(e.getMessage());
+ }
+ }
+ }
+ return false;
+ }
+
+ @Override
+ public List<ColumnStatisticsObj> getColStatistics(org.apache.hadoop.hive.ql.metadata.Table hmsTable) {
+ Table table = Catalogs.loadTable(conf, Utilities.getTableDesc(hmsTable).getProperties());
+ String statsPath = getStatsPath(table).toString();
+ LOG.info("Using stats from puffin file at:" + statsPath);
+ try (PuffinReader reader = Puffin.read(table.io().newInputFile(statsPath)).build()) {
+ List<BlobMetadata> blobMetadata = reader.fileMetadata().blobs();
+ Map<BlobMetadata, List<ColumnStatistics>> collect =
+ Streams.stream(reader.readAll(blobMetadata)).collect(Collectors.toMap(Pair::first,
+ blobMetadataByteBufferPair -> SerializationUtils.deserialize(
+ ByteBuffers.toByteArray(blobMetadataByteBufferPair.second()))));
+ return collect.get(blobMetadata.get(0)).get(0).getStatsObj();
+ } catch (IOException e) {
+ LOG.error(String.valueOf(e));
+ }
+ return null;
+ }
+
+
+ @Override
+ public boolean setColStatistics(org.apache.hadoop.hive.ql.metadata.Table table,
+ List<ColumnStatistics> colStats) {
+ TableDesc tableDesc = Utilities.getTableDesc(table);
+ Table tbl = Catalogs.loadTable(conf, tableDesc.getProperties());
+ String snapshotId = tbl.name() + tbl.currentSnapshot().snapshotId();
Review Comment:
Fixed, the null check is moved to canSetColStatistics.
--
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 #4131: HIVE-27158: Store hive columns stats in puffin files for iceberg tables
Posted by "simhadri-g (via GitHub)" <gi...@apache.org>.
simhadri-g commented on PR #4131:
URL: https://github.com/apache/hive/pull/4131#issuecomment-1512659990
Thanks everyone for the reviews! :)
--
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] deniskuzZ commented on a diff in pull request #4131: HIVE-27158: Store hive columns stats in puffin files for iceberg tables
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #4131:
URL: https://github.com/apache/hive/pull/4131#discussion_r1149111504
##########
ql/src/java/org/apache/hadoop/hive/ql/stats/ColStatsProcessor.java:
##########
@@ -218,6 +218,9 @@ public int persistColumnStats(Hive db, Table tbl) throws HiveException, MetaExce
}
start = System. currentTimeMillis();
+ if(tbl != null && tbl.isNonNative() && tbl.getStorageHandler().canSetColStatistics()){
+ tbl.getStorageHandler().setColStatistics(tbl, colStats);
Review Comment:
when this code is getting invoked, is it by auto-gather thread? if yes, what if there was several snapshots generated between the runs?
##########
ql/src/java/org/apache/hadoop/hive/ql/stats/ColStatsProcessor.java:
##########
@@ -218,6 +218,9 @@ public int persistColumnStats(Hive db, Table tbl) throws HiveException, MetaExce
}
start = System. currentTimeMillis();
+ if(tbl != null && tbl.isNonNative() && tbl.getStorageHandler().canSetColStatistics()){
+ tbl.getStorageHandler().setColStatistics(tbl, colStats);
Review Comment:
when this code is getting invoked, is it by auto-gather thread? if yes, what if there were several snapshots generated between the runs?
--
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] deniskuzZ commented on a diff in pull request #4131: HIVE-27158: Store hive columns stats in puffin files for iceberg tables
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #4131:
URL: https://github.com/apache/hive/pull/4131#discussion_r1149123408
##########
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java:
##########
@@ -349,6 +365,96 @@ public Map<String, String> getBasicStatistics(Partish partish) {
return stats;
}
+
+ @Override
+ public boolean canSetColStatistics() {
+ String statsSource = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_USE_STATS_FROM).toLowerCase();
+ return statsSource.equals(PUFFIN);
+ }
+
+ @Override
+ public boolean canProvideColStatistics(org.apache.hadoop.hive.ql.metadata.Table tbl) {
+
+ org.apache.hadoop.hive.ql.metadata.Table hmsTable = tbl;
+ TableDesc tableDesc = Utilities.getTableDesc(hmsTable);
+ Table table = Catalogs.loadTable(conf, tableDesc.getProperties());
+ if (table.currentSnapshot() != null) {
+ String statsSource = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_COL_STATS_SOURCE).toLowerCase();
+ String statsPath = table.location() + STATS + table.name() + table.currentSnapshot().snapshotId();
+ if (statsSource.equals(PUFFIN)) {
+ try (FileSystem fs = new Path(table.location()).getFileSystem(conf)) {
+ if (fs.exists(new Path(statsPath))) {
+ return true;
+ }
+ } catch (IOException e) {
+ LOG.warn(e.getMessage());
+ }
+ }
+ }
+ return false;
+ }
+
+ @Override
+ public List<ColumnStatisticsObj> getColStatistics(org.apache.hadoop.hive.ql.metadata.Table tbl) {
+
+ org.apache.hadoop.hive.ql.metadata.Table hmsTable = tbl;
+ TableDesc tableDesc = Utilities.getTableDesc(hmsTable);
+ Table table = Catalogs.loadTable(conf, tableDesc.getProperties());
+ String statsSource = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_COL_STATS_SOURCE).toLowerCase();
+ switch (statsSource) {
+ case ICEBERG:
+ // Place holder for iceberg stats
+ break;
+ case PUFFIN:
+ String snapshotId = table.name() + table.currentSnapshot().snapshotId();
+ String statsPath = table.location() + STATS + snapshotId;
+ LOG.info("Using stats from puffin file at:" + statsPath);
+ try (PuffinReader reader = Puffin.read(table.io().newInputFile(statsPath)).build()) {
+ BlobMetadata blobMetadata = reader.fileMetadata().blobs().get(0);
+ Map<BlobMetadata, List<ColumnStatistics>> collect =
+ Streams.stream(reader.readAll(ImmutableList.of(blobMetadata))).collect(Collectors.toMap(Pair::first,
+ blobMetadataByteBufferPair -> SerializationUtils.deserialize(
+ ByteBuffers.toByteArray(blobMetadataByteBufferPair.second()))));
+
+ return collect.entrySet().stream().iterator().next().getValue().get(0).getStatsObj();
+ } catch (IOException e) {
+ LOG.info(String.valueOf(e));
+ }
+ break;
+ default:
+ // fall back to metastore
+ }
+ return null;
+ }
+
+
+ @Override
+ public boolean setColStatistics(org.apache.hadoop.hive.ql.metadata.Table table,
+ List<ColumnStatistics> colStats) {
+ TableDesc tableDesc = Utilities.getTableDesc(table);
+ Table tbl = Catalogs.loadTable(conf, tableDesc.getProperties());
+ String snapshotId = tbl.name() + tbl.currentSnapshot().snapshotId();
+ byte[] serializeColStats = SerializationUtils.serialize((Serializable) colStats);
+
+ try (PuffinWriter writer = Puffin.write(tbl.io().newOutputFile(tbl.location() + STATS + snapshotId))
Review Comment:
looks like not, per `getColStatsForPartCol` comment: currently, metastore does not store column stats for the partition column. Also in the HMS column stats table is called `TAB_COL_STATS` which doesn't track the per-partition 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] rbalamohan commented on a diff in pull request #4131: HIVE-27158: Store hive columns stats in puffin files for iceberg tables
Posted by "rbalamohan (via GitHub)" <gi...@apache.org>.
rbalamohan commented on code in PR #4131:
URL: https://github.com/apache/hive/pull/4131#discussion_r1142898325
##########
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java:
##########
@@ -349,6 +364,98 @@ public Map<String, String> getBasicStatistics(Partish partish) {
return stats;
}
+
+ @Override
+ public boolean canSetColStatistics() {
+ String statsSource = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_USE_STATS_FROM).toLowerCase();
+ return statsSource.equals(PUFFIN) ? true : false;
+ }
+
+ @Override
+ public boolean canProvideColStatistics(org.apache.hadoop.hive.ql.metadata.Table tbl) {
+
+ org.apache.hadoop.hive.ql.metadata.Table hmsTable = tbl;
+ TableDesc tableDesc = Utilities.getTableDesc(hmsTable);
+ Table table = Catalogs.loadTable(conf, tableDesc.getProperties());
+ if (table.currentSnapshot() != null) {
+ String statsSource = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_USE_STATS_FROM).toLowerCase();
+ String statsPath = table.location() + "/stats/" + table.name() + table.currentSnapshot().snapshotId();
+ if (statsSource.equals(PUFFIN)) {
+ try (FileSystem fs = new Path(table.location()).getFileSystem(conf)) {
+ if (fs.exists(new Path(statsPath))) {
+ return true;
+ }
+ } catch (IOException e) {
+ LOG.warn(e.getMessage());
+ }
+ }
+ }
+ return false;
+ }
+
+ @Override
+ public List<ColumnStatisticsObj> getColStatistics(org.apache.hadoop.hive.ql.metadata.Table tbl) {
+
+ org.apache.hadoop.hive.ql.metadata.Table hmsTable = tbl;
+ TableDesc tableDesc = Utilities.getTableDesc(hmsTable);
+ Table table = Catalogs.loadTable(conf, tableDesc.getProperties());
+ String statsSource = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_USE_STATS_FROM).toLowerCase();
+ switch (statsSource) {
+ case ICEBERG:
+ // Place holder for iceberg stats
+ break;
+ case PUFFIN:
+ String snapshotId = table.name() + table.currentSnapshot().snapshotId();
Review Comment:
Is this mainly for col stats? As in, do we plan to standardize this for iceberg?
As of today, it seems to be querying HMS for basic stats and with this, it may have a mix of HMS + FileReading.
--
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 #4131: HIVE-27158: Store hive columns stats in puffin files for iceberg tables
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #4131:
URL: https://github.com/apache/hive/pull/4131#issuecomment-1482786217
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=4131)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4131&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4131&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4131&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=CODE_SMELL) [6 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4131&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4131&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4131&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] deniskuzZ commented on a diff in pull request #4131: HIVE-27158: Store hive columns stats in puffin files for iceberg tables
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #4131:
URL: https://github.com/apache/hive/pull/4131#discussion_r1149008394
##########
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java:
##########
@@ -349,6 +365,96 @@ public Map<String, String> getBasicStatistics(Partish partish) {
return stats;
}
+
+ @Override
+ public boolean canSetColStatistics() {
+ String statsSource = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_USE_STATS_FROM).toLowerCase();
+ return statsSource.equals(PUFFIN);
+ }
+
+ @Override
+ public boolean canProvideColStatistics(org.apache.hadoop.hive.ql.metadata.Table tbl) {
+
+ org.apache.hadoop.hive.ql.metadata.Table hmsTable = tbl;
+ TableDesc tableDesc = Utilities.getTableDesc(hmsTable);
+ Table table = Catalogs.loadTable(conf, tableDesc.getProperties());
+ if (table.currentSnapshot() != null) {
+ String statsSource = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_COL_STATS_SOURCE).toLowerCase();
+ String statsPath = table.location() + STATS + table.name() + table.currentSnapshot().snapshotId();
+ if (statsSource.equals(PUFFIN)) {
+ try (FileSystem fs = new Path(table.location()).getFileSystem(conf)) {
+ if (fs.exists(new Path(statsPath))) {
Review Comment:
can we create a path variable just once?
````
Path statsPath = new Path(...);
FileSystem fs = statsPath.getFileSystem(conf);
return fs.exists(statsPath));
````
--
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] deniskuzZ commented on a diff in pull request #4131: HIVE-27158: Store hive columns stats in puffin files for iceberg tables
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #4131:
URL: https://github.com/apache/hive/pull/4131#discussion_r1149003439
##########
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java:
##########
@@ -349,6 +365,96 @@ public Map<String, String> getBasicStatistics(Partish partish) {
return stats;
}
+
+ @Override
+ public boolean canSetColStatistics() {
+ String statsSource = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_USE_STATS_FROM).toLowerCase();
+ return statsSource.equals(PUFFIN);
+ }
+
+ @Override
+ public boolean canProvideColStatistics(org.apache.hadoop.hive.ql.metadata.Table tbl) {
+
+ org.apache.hadoop.hive.ql.metadata.Table hmsTable = tbl;
+ TableDesc tableDesc = Utilities.getTableDesc(hmsTable);
+ Table table = Catalogs.loadTable(conf, tableDesc.getProperties());
+ if (table.currentSnapshot() != null) {
+ String statsSource = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_COL_STATS_SOURCE).toLowerCase();
+ String statsPath = table.location() + STATS + table.name() + table.currentSnapshot().snapshotId();
+ if (statsSource.equals(PUFFIN)) {
Review Comment:
PUFFIN.equals(statsSource), maybe even better to create enum
--
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] deniskuzZ commented on a diff in pull request #4131: HIVE-27158: Store hive columns stats in puffin files for iceberg tables
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #4131:
URL: https://github.com/apache/hive/pull/4131#discussion_r1149030014
##########
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java:
##########
@@ -349,6 +365,96 @@ public Map<String, String> getBasicStatistics(Partish partish) {
return stats;
}
+
+ @Override
+ public boolean canSetColStatistics() {
+ String statsSource = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_USE_STATS_FROM).toLowerCase();
+ return statsSource.equals(PUFFIN);
+ }
+
+ @Override
+ public boolean canProvideColStatistics(org.apache.hadoop.hive.ql.metadata.Table tbl) {
+
+ org.apache.hadoop.hive.ql.metadata.Table hmsTable = tbl;
+ TableDesc tableDesc = Utilities.getTableDesc(hmsTable);
+ Table table = Catalogs.loadTable(conf, tableDesc.getProperties());
+ if (table.currentSnapshot() != null) {
+ String statsSource = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_COL_STATS_SOURCE).toLowerCase();
+ String statsPath = table.location() + STATS + table.name() + table.currentSnapshot().snapshotId();
+ if (statsSource.equals(PUFFIN)) {
+ try (FileSystem fs = new Path(table.location()).getFileSystem(conf)) {
+ if (fs.exists(new Path(statsPath))) {
+ return true;
+ }
+ } catch (IOException e) {
+ LOG.warn(e.getMessage());
+ }
+ }
+ }
+ return false;
+ }
+
+ @Override
+ public List<ColumnStatisticsObj> getColStatistics(org.apache.hadoop.hive.ql.metadata.Table tbl) {
+
+ org.apache.hadoop.hive.ql.metadata.Table hmsTable = tbl;
+ TableDesc tableDesc = Utilities.getTableDesc(hmsTable);
+ Table table = Catalogs.loadTable(conf, tableDesc.getProperties());
+ String statsSource = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_COL_STATS_SOURCE).toLowerCase();
+ switch (statsSource) {
+ case ICEBERG:
+ // Place holder for iceberg stats
+ break;
+ case PUFFIN:
+ String snapshotId = table.name() + table.currentSnapshot().snapshotId();
+ String statsPath = table.location() + STATS + snapshotId;
+ LOG.info("Using stats from puffin file at:" + statsPath);
+ try (PuffinReader reader = Puffin.read(table.io().newInputFile(statsPath)).build()) {
+ BlobMetadata blobMetadata = reader.fileMetadata().blobs().get(0);
+ Map<BlobMetadata, List<ColumnStatistics>> collect =
+ Streams.stream(reader.readAll(ImmutableList.of(blobMetadata))).collect(Collectors.toMap(Pair::first,
+ blobMetadataByteBufferPair -> SerializationUtils.deserialize(
+ ByteBuffers.toByteArray(blobMetadataByteBufferPair.second()))));
+
+ return collect.entrySet().stream().iterator().next().getValue().get(0).getStatsObj();
Review Comment:
why do you need a map in the first place if you are only interested in the first value?
--
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] deniskuzZ commented on a diff in pull request #4131: HIVE-27158: Store hive columns stats in puffin files for iceberg tables
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #4131:
URL: https://github.com/apache/hive/pull/4131#discussion_r1149037383
##########
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java:
##########
@@ -349,6 +365,96 @@ public Map<String, String> getBasicStatistics(Partish partish) {
return stats;
}
+
+ @Override
+ public boolean canSetColStatistics() {
+ String statsSource = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_USE_STATS_FROM).toLowerCase();
+ return statsSource.equals(PUFFIN);
+ }
+
+ @Override
+ public boolean canProvideColStatistics(org.apache.hadoop.hive.ql.metadata.Table tbl) {
+
+ org.apache.hadoop.hive.ql.metadata.Table hmsTable = tbl;
+ TableDesc tableDesc = Utilities.getTableDesc(hmsTable);
+ Table table = Catalogs.loadTable(conf, tableDesc.getProperties());
+ if (table.currentSnapshot() != null) {
+ String statsSource = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_COL_STATS_SOURCE).toLowerCase();
+ String statsPath = table.location() + STATS + table.name() + table.currentSnapshot().snapshotId();
+ if (statsSource.equals(PUFFIN)) {
+ try (FileSystem fs = new Path(table.location()).getFileSystem(conf)) {
+ if (fs.exists(new Path(statsPath))) {
+ return true;
+ }
+ } catch (IOException e) {
+ LOG.warn(e.getMessage());
+ }
+ }
+ }
+ return false;
+ }
+
+ @Override
+ public List<ColumnStatisticsObj> getColStatistics(org.apache.hadoop.hive.ql.metadata.Table tbl) {
+
+ org.apache.hadoop.hive.ql.metadata.Table hmsTable = tbl;
+ TableDesc tableDesc = Utilities.getTableDesc(hmsTable);
+ Table table = Catalogs.loadTable(conf, tableDesc.getProperties());
+ String statsSource = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_COL_STATS_SOURCE).toLowerCase();
+ switch (statsSource) {
+ case ICEBERG:
+ // Place holder for iceberg stats
+ break;
+ case PUFFIN:
+ String snapshotId = table.name() + table.currentSnapshot().snapshotId();
+ String statsPath = table.location() + STATS + snapshotId;
+ LOG.info("Using stats from puffin file at:" + statsPath);
+ try (PuffinReader reader = Puffin.read(table.io().newInputFile(statsPath)).build()) {
+ BlobMetadata blobMetadata = reader.fileMetadata().blobs().get(0);
+ Map<BlobMetadata, List<ColumnStatistics>> collect =
+ Streams.stream(reader.readAll(ImmutableList.of(blobMetadata))).collect(Collectors.toMap(Pair::first,
+ blobMetadataByteBufferPair -> SerializationUtils.deserialize(
+ ByteBuffers.toByteArray(blobMetadataByteBufferPair.second()))));
+
+ return collect.entrySet().stream().iterator().next().getValue().get(0).getStatsObj();
+ } catch (IOException e) {
+ LOG.info(String.valueOf(e));
+ }
+ break;
+ default:
+ // fall back to metastore
+ }
+ return null;
+ }
+
+
+ @Override
+ public boolean setColStatistics(org.apache.hadoop.hive.ql.metadata.Table table,
+ List<ColumnStatistics> colStats) {
+ TableDesc tableDesc = Utilities.getTableDesc(table);
+ Table tbl = Catalogs.loadTable(conf, tableDesc.getProperties());
+ String snapshotId = tbl.name() + tbl.currentSnapshot().snapshotId();
+ byte[] serializeColStats = SerializationUtils.serialize((Serializable) colStats);
+
+ try (PuffinWriter writer = Puffin.write(tbl.io().newOutputFile(tbl.location() + STATS + snapshotId))
Review Comment:
what about partition-level 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] deniskuzZ commented on a diff in pull request #4131: HIVE-27158: Store hive columns stats in puffin files for iceberg tables
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #4131:
URL: https://github.com/apache/hive/pull/4131#discussion_r1148996651
##########
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java:
##########
@@ -349,6 +365,96 @@ public Map<String, String> getBasicStatistics(Partish partish) {
return stats;
}
+
+ @Override
+ public boolean canSetColStatistics() {
+ String statsSource = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_USE_STATS_FROM).toLowerCase();
+ return statsSource.equals(PUFFIN);
+ }
+
+ @Override
+ public boolean canProvideColStatistics(org.apache.hadoop.hive.ql.metadata.Table tbl) {
+
+ org.apache.hadoop.hive.ql.metadata.Table hmsTable = tbl;
Review Comment:
why do we need local var?
--
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] deniskuzZ commented on a diff in pull request #4131: HIVE-27158: Store hive columns stats in puffin files for iceberg tables
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #4131:
URL: https://github.com/apache/hive/pull/4131#discussion_r1149023043
##########
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java:
##########
@@ -349,6 +365,96 @@ public Map<String, String> getBasicStatistics(Partish partish) {
return stats;
}
+
+ @Override
+ public boolean canSetColStatistics() {
+ String statsSource = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_USE_STATS_FROM).toLowerCase();
+ return statsSource.equals(PUFFIN);
+ }
+
+ @Override
+ public boolean canProvideColStatistics(org.apache.hadoop.hive.ql.metadata.Table tbl) {
+
+ org.apache.hadoop.hive.ql.metadata.Table hmsTable = tbl;
+ TableDesc tableDesc = Utilities.getTableDesc(hmsTable);
+ Table table = Catalogs.loadTable(conf, tableDesc.getProperties());
+ if (table.currentSnapshot() != null) {
+ String statsSource = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_COL_STATS_SOURCE).toLowerCase();
+ String statsPath = table.location() + STATS + table.name() + table.currentSnapshot().snapshotId();
+ if (statsSource.equals(PUFFIN)) {
+ try (FileSystem fs = new Path(table.location()).getFileSystem(conf)) {
+ if (fs.exists(new Path(statsPath))) {
+ return true;
+ }
+ } catch (IOException e) {
+ LOG.warn(e.getMessage());
+ }
+ }
+ }
+ return false;
+ }
+
+ @Override
+ public List<ColumnStatisticsObj> getColStatistics(org.apache.hadoop.hive.ql.metadata.Table tbl) {
+
+ org.apache.hadoop.hive.ql.metadata.Table hmsTable = tbl;
+ TableDesc tableDesc = Utilities.getTableDesc(hmsTable);
+ Table table = Catalogs.loadTable(conf, tableDesc.getProperties());
+ String statsSource = HiveConf.getVar(conf, HiveConf.ConfVars.HIVE_COL_STATS_SOURCE).toLowerCase();
+ switch (statsSource) {
+ case ICEBERG:
+ // Place holder for iceberg stats
+ break;
+ case PUFFIN:
+ String snapshotId = table.name() + table.currentSnapshot().snapshotId();
+ String statsPath = table.location() + STATS + snapshotId;
+ LOG.info("Using stats from puffin file at:" + statsPath);
Review Comment:
debug level?
--
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