HIVE-29396: Drop the property hive.metastore.try.direct.sql.ddl

[dengzhhu653]

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?

[okumin]

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]

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]

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]

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]

@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]

@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]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
1.1% Duplication on New Code

See analysis details on SonarQube Cloud

[Aggarwal-Raghav]

LGTM +1

[Aggarwal-Raghav]

@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?

I'm will work on HIVE-29289 and will discuss more under that following JIRA.

[okumin]

LGTM!

[dengzhhu653]

Thanks for the review @okumin, @Aggarwal-Raghav!