wypoon commented on code in PR #12721: URL: https://github.com/apache/iceberg/pull/12721#discussion_r2034029619
########## hive-metastore/src/test/java/org/apache/iceberg/hive/TestHiveMetastore.java: ########## @@ -186,9 +189,9 @@ public void stop() throws Exception { if (executorService != null) { executorService.shutdown(); } - if (baseHandler != null) { - baseHandler.shutdown(); - } + // if (baseHandler != null) { + // baseHandler.shutdown(); + // } Review Comment: @pvary I made two mistakes above. First, I made a mistake about the type hierarchies. _I had missed that_ `IHMSHandler` **is** a `FacebookService.Iface`. So I can use `FacebookService.Iface` for `Foo`. I.e., I can declare the `baseHandler` field to be of type `FacebookService.Iface`. (No wonder you didn't understand my thinking before, as it was wrong!) Second, I was thinking about what the actual static method (either `RetryingHMSHandler.getProxy` in the case of Hive 2/3 or `HMSHandlerProxyFactory.getProxy` in the case of Hive 4) needs as parameters. It needs an `IHMSHandler` as its second parameter, not a `FacebookService.Iface`. However, when using reflection (`GET_BASE_HMS_HANDLER.invoke`), the parameters (to the `invoke`) are `Object`s, so there is no type checking. That is why the code compiles (and at runtime, fortunately `HMSHandler` is the correct type of `Object`). I have pushed a new commit to fix this issue. -- 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: issues-unsubscr...@iceberg.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org For additional commands, e-mail: issues-h...@iceberg.apache.org