kasakrisz commented on code in PR #4440:
URL: https://github.com/apache/hive/pull/4440#discussion_r1247559021
##########
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java:
##########
@@ -428,10 +428,11 @@ public boolean
setColStatistics(org.apache.hadoop.hive.ql.metadata.Table hmsTabl
PuffinCompressionCodec.NONE,
ImmutableMap.of()));
writer.finish();
+ return true;
} catch (IOException e) {
LOG.error(String.valueOf(e));
+ return false;
Review Comment:
What is the purpose of this boolean return value? If it is indicating
whether the set operation failed can we throw an exception instead and change
the return type to void?
nit: I know it is not changed by this PR but the method name
`setColStatistics` is confusing a bit because we are not just simply setting
some fields of an object but this is an I/O operation.
##########
ql/src/test/results/clientpositive/llap/autoColumnStats_10.q.out:
##########
@@ -205,7 +205,7 @@ Retention: 0
#### A masked pattern was here ####
Table Type: MANAGED_TABLE
Table Parameters:
- COLUMN_STATS_ACCURATE
{\"BASIC_STATS\":\"true\",\"COLUMN_STATS\":{\"insert_num\":\"true\"}}
+ COLUMN_STATS_ACCURATE
{\"BASIC_STATS\":\"true\",\"COLUMN_STATS\":{\"c1\":\"true\",\"c2\":\"true\",\"insert_num\":\"true\"}}
Review Comment:
In this test an alter table statement changes the type of columns `c1` and
`c2` and it removes the column stats entries from HMS backend.
https://github.com/apache/hive/blob/e6c6029439f831545fe5fa267452b662c3a71403/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java#L1085-L1089
from `tinyint` to `string`
```
alter table p_n1 replace columns (insert_num int, c1 STRING, c2 STRING);
```
The insert follows the alter
```
insert into p_n1 values (1,22,333);
```
can not merge the stats for `c1` and `c2` columns because those are not
exists.
So the
```
\"c1\":\"true\",\"c2\":\"true\"
```
is not true.
--
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-unsubscr...@hive.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
