abstractdog commented on code in PR #5859: URL: https://github.com/apache/hive/pull/5859#discussion_r2158204496
########## ql/pom.xml: ########## @@ -1142,6 +1143,12 @@ <exclude>META-INF/licenses/slf4j*/**</exclude> </excludes> </filter> + <filter> + <artifact>org.apache.tez:tez-protobuf-history-plugin</artifact> Review Comment: @deniskuzZ : let me respond here the quesions above > Why is the tez-protobuf-history-plugin needed in schematool to begin with? What purpose does it serve? schematool runs hive-ql codepath, so it can hit anything which is used around the Driver, that's how query hooks came into the picture, see this trace below: ``` at org.apache.hadoop.hive.ql.parse.BaseSemanticAnalyzer.getDatabase(BaseSemanticAnalyzer.java:1877) at org.apache.hadoop.hive.ql.ddl.database.use.SwitchDatabaseAnalyzer.analyzeInternal(SwitchDatabaseAnalyzer.java:45) at org.apache.hadoop.hive.ql.parse.BaseSemanticAnalyzer.analyze(BaseSemanticAnalyzer.java:324) at org.apache.hadoop.hive.ql.Compiler.analyze(Compiler.java:227) at org.apache.hadoop.hive.ql.Compiler.compile(Compiler.java:108) at org.apache.hadoop.hive.ql.Driver.compile(Driver.java:203) at org.apache.hadoop.hive.ql.Driver.compileInternal(Driver.java:659) at org.apache.hadoop.hive.ql.Driver.compileAndRespond(Driver.java:604) at org.apache.hadoop.hive.ql.Driver.compileAndRespond(Driver.java:598) at org.apache.hadoop.hive.ql.reexec.ReExecDriver.compileAndRespond(ReExecDriver.java:126) at org.apache.hive.service.cli.operation.SQLOperation.prepare(SQLOperation.java:210) at org.apache.hive.service.cli.operation.SQLOperation.runInternal(SQLOperation.java:274) at org.apache.hive.service.cli.operation.Operation.run(Operation.java:288) at org.apache.hive.service.cli.session.HiveSessionImpl.executeStatementInternal(HiveSessionImpl.java:574) at org.apache.hive.service.cli.session.HiveSessionImpl.executeStatementAsync(HiveSessionImpl.java:560) at org.apache.hive.service.cli.CLIService.executeStatementAsync(CLIService.java:315) at org.apache.hive.service.cli.thrift.ThriftCLIService.ExecuteStatement(ThriftCLIService.java:583) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.lang.reflect.Method.invoke(Method.java:498) at org.apache.hive.jdbc.HiveConnection$SynchronizedHandler.invoke(HiveConnection.java:2168) at com.sun.proxy.$Proxy47.ExecuteStatement(Unknown Source) at org.apache.hive.jdbc.HiveStatement.runAsyncOnServer(HiveStatement.java:374) at org.apache.hive.jdbc.HiveStatement.execute(HiveStatement.java:316) at org.apache.hive.jdbc.HiveConnection.setSchema(HiveConnection.java:2084) at org.apache.hadoop.hive.metastore.tools.schematool.HiveSchemaHelper.getConnectionToMetastore(HiveSchemaHelper.java:90) at org.apache.hadoop.hive.metastore.tools.schematool.HiveSchemaHelper.getConnectionToMetastore(HiveSchemaHelper.java:103) at org.apache.hadoop.hive.metastore.CDHMetaStoreSchemaInfo.getMetaStoreSchemaVersion(CDHMetaStoreSchemaInfo.java:323) at org.apache.hadoop.hive.metastore.tools.schematool.SchemaToolTaskInitOrUpgrade.execute(SchemaToolTaskInitOrUpgrade.java:41) at org.apache.hadoop.hive.metastore.tools.schematool.MetastoreSchemaTool.run(MetastoreSchemaTool.java:486) at org.apache.hive.beeline.schematool.HiveSchemaTool.main(HiveSchemaTool.java:143) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.lang.reflect.Method.invoke(Method.java:498) at org.apache.hadoop.util.RunJar.run(RunJar.java:321) at org.apache.hadoop.util.RunJar.main(RunJar.java:237) ``` > in a root pom we define multiple exclusions for tez-protobuf-history-plugin, can't we exclude all the transitive dependencies like let me check > I noticed those classes were already part of hive-exec before [HIVE-28028](https://issues.apache.org/jira/browse/HIVE-28028), which removed them and replaced with the tez-protobuf-history-plugin. So, basically, this PR doesn't add any extra classes. before HIVE-28028 hive had these classes as sources copied, so it's a major difference that now we shade the binaries (I didn't like that copied and version controlled them independently, which is something we only do in crazy situations when there is no time for waiting for a tez release or something) > why do we need filter? that's the only package in that artifact good catch, I haven't checked, I'm removing the include -- 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