Skip to content

HIVE-28953: Remove HiveMetaStoreClientPreCatalog - #5812

Merged
dengzhhu653 merged 2 commits into
apache:masterfrom
dengzhhu653:HIVE-28953
May 15, 2025
Merged

HIVE-28953: Remove HiveMetaStoreClientPreCatalog#5812
dengzhhu653 merged 2 commits into
apache:masterfrom
dengzhhu653:HIVE-28953

Conversation

@dengzhhu653

Copy link
Copy Markdown
Member

What changes were proposed in this pull request?

Why are the changes needed?

HiveMetaStoreClient is aware of the default catalog set in the configuration, making HiveMetaStoreClientPreCatalog unnecessary in the test, and each time we introduce a new method in IMetaStoreClient, we have to implement the body in HiveMetaStoreClientPreCatalog, which is meaningless.

Does this PR introduce any user-facing change?

No

How was this patch tested?

@zhangbutao

Copy link
Copy Markdown
Contributor

Can we confirm that HiveMetaStoreClientPreCatalog will not be used by any other downstream open-source components?

IMO, HiveMetaStoreClientPreCatalog to be of no use, so I think it can be safely deleted.

@dengzhhu653

Copy link
Copy Markdown
Member Author

Can we confirm that HiveMetaStoreClientPreCatalog will not be used by any other downstream open-source components?

IMO, HiveMetaStoreClientPreCatalog to be of no use, so I think it can be safely deleted.

This class resides in our test package, which I think no downstream project should use this in their base, will check Impala and Iceberg.

@okumin

okumin commented May 13, 2025

Copy link
Copy Markdown
Contributor

I assume the class is really not needed.

I'd like to remove the name also from the comment in HiveMetaStore.java.

% git grep HiveMetaStoreClientPreCatalog
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java:   * This method is called indirectly by HiveMetastoreClient and HiveMetaStoreClientPreCatalog
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java:   * change in HiveMetastoreClient and HiveMetaStoreClientPreCatalog.
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClientPreCatalog.java:public class HiveMetaStoreClientPreCatalog implements IMetaStoreClient, AutoCloseable {
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClientPreCatalog.java:  static final protected Logger LOG = LoggerFactory.getLogger(HiveMetaStoreClientPreCatalog.class);
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClientPreCatalog.java:  public HiveMetaStoreClientPreCatalog(Configuration conf) throws MetaException {
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClientPreCatalog.java:  public HiveMetaStoreClientPreCatalog(Configuration conf, HiveMetaHookLoader hookLoader) throws MetaException {
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClientPreCatalog.java:  public HiveMetaStoreClientPreCatalog(Configuration conf, HiveMetaHookLoader hookLoader, Boolean allowEmbedded)
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClientPreCatalog.java:      HiveMetaStoreClientPreCatalog.class.getClassLoader(),
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestCatalogNonDefaultSvr.java:    return new HiveMetaStoreClientPreCatalog(conf);
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestCatalogOldClient.java:    return new HiveMetaStoreClientPreCatalog(conf);

@dengzhhu653

Copy link
Copy Markdown
Member Author

I checked the master/main branch of Iceberg and Impala, they are not using this class.

@sonarqubecloud

Copy link
Copy Markdown

@zhangbutao zhangbutao left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 LGTM

@okumin okumin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@dengzhhu653
dengzhhu653 merged commit 88cc6fd into apache:master May 15, 2025
@dengzhhu653
dengzhhu653 deleted the HIVE-28953 branch May 15, 2025 00:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants