You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hive.apache.org by "ASF GitHub Bot (Jira)" <ji...@apache.org> on 2023/02/03 15:47:00 UTC
[jira] [Work logged] (HIVE-26832) Implement SHOW PARTITIONS for Iceberg
[ https://issues.apache.org/jira/browse/HIVE-26832?focusedWorklogId=843518&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-843518 ]
ASF GitHub Bot logged work on HIVE-26832:
-----------------------------------------
Author: ASF GitHub Bot
Created on: 03/Feb/23 15:46
Start Date: 03/Feb/23 15:46
Worklog Time Spent: 10m
Work Description: InvisibleProgrammer commented on code in PR #3849:
URL: https://github.com/apache/hive/pull/3849#discussion_r1095947711
##########
ql/src/java/org/apache/hadoop/hive/ql/metadata/HiveStorageHandler.java:
##########
@@ -521,4 +523,7 @@ default SnapshotContext getCurrentSnapshotContext(org.apache.hadoop.hive.ql.meta
default void prepareAlterTableEnvironmentContext(AbstractAlterTableDesc alterTableDesc,
EnvironmentContext environmentContext) {
}
+ default List<String> getIcebergPartitions(Configuration conf, org.apache.hadoop.hive.ql.metadata.Table metaTable){
Review Comment:
What 'nit' means?
##########
ql/src/java/org/apache/hadoop/hive/ql/ddl/table/partition/show/ShowPartitionsOperation.java:
##########
@@ -116,4 +120,22 @@ private List<String> getPartitionNames(Table tbl) throws HiveException {
desc.getOrder(), desc.getLimit());
return partNames;
}
+
+ private boolean isPartitionedIcebergTable(Table tbl) {
+ String t = tbl.getParameters().get("table_type");
+ return t != null && t.equalsIgnoreCase("ICEBERG") &&
+ (tbl.getStorageHandler().getPartitionTransformSpec(tbl).size() > 0);
+ }
+
+ private List<String> getIcebergParts(Table tbl) {
+ List<String> parts = new ArrayList<>();
+ try {
+ Table metaDataPartTable = context.getDb().getTable(tbl.getDbName(), tbl.getTableName(), "partitions", true);
+ parts = tbl.getStorageHandler().getIcebergPartitions( context.getConf(), metaDataPartTable);
+ return parts;
+ } catch (Exception e) {
Review Comment:
Do we have any more specific than a general Exception? And also, is that Warn message just a leftover?
##########
ql/src/java/org/apache/hadoop/hive/ql/ddl/table/partition/show/ShowPartitionsOperation.java:
##########
@@ -116,4 +120,22 @@ private List<String> getPartitionNames(Table tbl) throws HiveException {
desc.getOrder(), desc.getLimit());
return partNames;
}
+
+ private boolean isPartitionedIcebergTable(Table tbl) {
+ String t = tbl.getParameters().get("table_type");
+ return t != null && t.equalsIgnoreCase("ICEBERG") &&
Review Comment:
What about `"ICEBERG".equalsIgnoreCase(tbl.getParameters().get("table_type"))` ?
##########
ql/src/java/org/apache/hadoop/hive/ql/metadata/HiveStorageHandler.java:
##########
@@ -521,4 +523,7 @@ default SnapshotContext getCurrentSnapshotContext(org.apache.hadoop.hive.ql.meta
default void prepareAlterTableEnvironmentContext(AbstractAlterTableDesc alterTableDesc,
EnvironmentContext environmentContext) {
}
+ default List<String> getIcebergPartitions(Configuration conf, org.apache.hadoop.hive.ql.metadata.Table metaTable){
Review Comment:
Or what about giving back an optional to avoid exceptions?
##########
ql/src/java/org/apache/hadoop/hive/ql/ddl/table/partition/show/ShowPartitionsOperation.java:
##########
@@ -116,4 +120,22 @@ private List<String> getPartitionNames(Table tbl) throws HiveException {
desc.getOrder(), desc.getLimit());
return partNames;
}
+
+ private boolean isPartitionedIcebergTable(Table tbl) {
+ String t = tbl.getParameters().get("table_type");
+ return t != null && t.equalsIgnoreCase("ICEBERG") &&
Review Comment:
Also, I'm thinking (not in this ticket) but is it worth adding an extra method to Table to get parameter by name?
E.g: getParameter(String name)
<img width="1519" alt="image" src="https://user-images.githubusercontent.com/1486749/216643120-c99f7715-a643-47ed-ace7-885fc18825fa.png">
##########
ql/src/java/org/apache/hadoop/hive/ql/ddl/table/partition/show/ShowPartitionsOperation.java:
##########
@@ -54,12 +54,16 @@ public ShowPartitionsOperation(DDLOperationContext context, ShowPartitionsDesc d
@Override
public int execute() throws HiveException {
Table tbl = context.getDb().getTable(desc.getTabName());
- if (!tbl.isPartitioned()) {
- throw new HiveException(ErrorMsg.TABLE_NOT_PARTITIONED, desc.getTabName());
+ boolean isPartitionedIcebergTable = isPartitionedIcebergTable(tbl);
+ if (!(tbl.isPartitioned() || isPartitionedIcebergTable)) {
Review Comment:
And also, there is an `IcebergTableUtil` class as well. It is in the `iceberg.mr.hive` package. I don't know if we can use it here.
Issue Time Tracking
-------------------
Worklog Id: (was: 843518)
Time Spent: 2h 50m (was: 2h 40m)
> Implement SHOW PARTITIONS for Iceberg
> -------------------------------------
>
> Key: HIVE-26832
> URL: https://issues.apache.org/jira/browse/HIVE-26832
> Project: Hive
> Issue Type: New Feature
> Reporter: Simhadri Govindappa
> Assignee: Simhadri Govindappa
> Priority: Major
> Labels: pull-request-available
> Time Spent: 2h 50m
> Remaining Estimate: 0h
>
> Show partition command for iceberg tables should reflect the partition info from the iceberg.partition metadata table based on the default-spec-id .
--
This message was sent by Atlassian Jira
(v8.20.10#820010)