Skip to content

Conversation

@dengzhhu653
Copy link
Member

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Copy link
Contributor

@okumin okumin left a comment

Choose a reason for hiding this comment

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

What change(in Postgres?) makes it possible to remove the flag?

We can delete the same parameters in HiveConf and some test cases as well.

common/src/java/org/apache/hadoop/hive/conf/HiveConf.java:    METASTORE_TRY_DIRECT_SQL_DDL("hive.metastore.try.direct.sql.ddl", true,
ql/src/test/queries/clientpositive/alter_table_column_stats.q:-- Test for external tables with metastore.try.direct.sql.ddl as false
ql/src/test/queries/clientpositive/alter_table_column_stats.q:set metaconf:metastore.try.direct.sql.ddl=false;
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java:    TRY_DIRECT_SQL_DDL("metastore.try.direct.sql.ddl", "hive.metastore.try.direct.sql.ddl", true,

@dengzhhu653
Copy link
Member Author

What change(in Postgres?) makes it possible to remove the flag?

We can delete the same parameters in HiveConf and some test cases as well.

common/src/java/org/apache/hadoop/hive/conf/HiveConf.java:    METASTORE_TRY_DIRECT_SQL_DDL("hive.metastore.try.direct.sql.ddl", true,
ql/src/test/queries/clientpositive/alter_table_column_stats.q:-- Test for external tables with metastore.try.direct.sql.ddl as false
ql/src/test/queries/clientpositive/alter_table_column_stats.q:set metaconf:metastore.try.direct.sql.ddl=false;
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java:    TRY_DIRECT_SQL_DDL("metastore.try.direct.sql.ddl", "hive.metastore.try.direct.sql.ddl", true,

With the help of HIVE-26976, it's possible to recover from the direct sql by rolling back to the savepoint before sql execution,

@okumin
Copy link
Contributor

okumin commented Jan 14, 2026

Makes sense for me if we update the three files. The test error looks related.
https://ci.hive.apache.org/blue/organizations/jenkins/hive-precommit/detail/PR-6265/1/tests

@dengzhhu653
Copy link
Member Author

Makes sense for me if we update the three files. The test error looks related. https://ci.hive.apache.org/blue/organizations/jenkins/hive-precommit/detail/PR-6265/1/tests

Done, looks the latest failure isn't related

@Aggarwal-Raghav
Copy link
Contributor

@dengzhhu653 , HiveConf changes in this PR are big +1 , I need some input on how to deal with the deprecated Metastore configs present in HiveConf. For example:

METASTORE_AGGREGATE_STATS_CACHE_MAX_FULL
METASTORE_AGGREGATE_STATS_CACHE_CLEAN_UNTIL
...

I had filed HIVE-29289 and the goal was to remove these configurations from HiveConf. Mostly these configs are used in test code. I was thinking of moving those tests in itests module to prevent cyclic depenedency if caused by adding metastore-comomon as dependency. Can you provide your thoughts on this?

@dengzhhu653
Copy link
Member Author

@dengzhhu653 , HiveConf changes in this PR are big +1 , I need some input on how to deal with the deprecated Metastore configs present in HiveConf. For example:

METASTORE_AGGREGATE_STATS_CACHE_MAX_FULL
METASTORE_AGGREGATE_STATS_CACHE_CLEAN_UNTIL
...

I had filed HIVE-29289 and the goal was to remove these configurations from HiveConf. Mostly these configs are used in test code. I was thinking of moving those tests in itests module to prevent cyclic depenedency if caused by adding metastore-comomon as dependency. Can you provide your thoughts on this?

Not sure how it could introduce the cyclic depenedency, the hive-common has already contained the dependency on metastore-common, can you give an example?

@sonarqubecloud
Copy link

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