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.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
![https://user-images.githubusercontent.com/1486749/216643120-c99f7715-a643-47ed-ace7-885fc18825fa.png"](https://user-images.githubusercontent.com/1486749/216643120-c99f7715-a643-47ed-ace7-885fc18825fa.png"){kind=link}