Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
zhangbutao commented on code in PR #6088:
URL: https://github.com/apache/hive/pull/6088#discussion_r2641660434
##
parser/src/java/org/apache/hadoop/hive/ql/parse/FromClauseParser.g:
##
@@ -231,6 +231,17 @@ uniqueJoinTableSource
-> ^(TOK_TABREF $tabname $ts? $alias?)
;
+databaseName
+@init { gParent.pushMsg("database name", state); }
+@after { gParent.popMsg(state); }
+:
+catalog=identifier DOT db=identifier?
Review Comment:
@ayushtkn Thanks for finding this issue.
Yes, `? ` is not needed. We can remove it.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
ayushtkn commented on code in PR #6088:
URL: https://github.com/apache/hive/pull/6088#discussion_r2640723349
##
parser/src/java/org/apache/hadoop/hive/ql/parse/FromClauseParser.g:
##
@@ -231,6 +231,17 @@ uniqueJoinTableSource
-> ^(TOK_TABREF $tabname $ts? $alias?)
;
+databaseName
+@init { gParent.pushMsg("database name", state); }
+@after { gParent.popMsg(state); }
+:
+catalog=identifier DOT db=identifier?
Review Comment:
why was ? needed here?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
zhangbutao commented on PR #6088: URL: https://github.com/apache/hive/pull/6088#issuecomment-3673895143 Merged. Thanks @deniskuzZ @dengzhhu653 @harshal-16 @Neer393 -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
zhangbutao merged PR #6088: URL: https://github.com/apache/hive/pull/6088 -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
zhangbutao commented on PR #6088: URL: https://github.com/apache/hive/pull/6088#issuecomment-3663212548 This PR currently introduces only a minor incompatibility issue: when using ZooKeeperHiveLockManager, the znode path for locks stored in ZooKeeper changes from `/database` to `/catalog/database`. This will cause older versions of Hive to be unable to properly read the lock information stored in ZooKeeper. However, to my knowledge, we typically use DbLockManager for ACID operations rather than ZooKeeperHiveLockManager, so this incompatibility risk is negligible. Aside from this, the PR does not break any other default functionalities. I will proceed to merge this PR shortly and continue with the subsequent related development work. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
sonarqubecloud[bot] commented on PR #6088: URL: https://github.com/apache/hive/pull/6088#issuecomment-3649737408 ## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=6088) **Quality Gate passed** Issues  [161 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=6088&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=6088&issueStatuses=ACCEPTED) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=6088&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=6088&metric=new_coverage&view=list)  [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=6088&metric=new_duplicated_lines_density&view=list) [See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=6088) -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
zhangbutao commented on PR #6088: URL: https://github.com/apache/hive/pull/6088#issuecomment-3649459186 > +1, Thanks, @zhangbutao. If anything was missed, we can fix it in a follow-up Sure. We still have lots of works to do. :) -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
zhangbutao commented on code in PR #6088: URL: https://github.com/apache/hive/pull/6088#discussion_r2616323224 ## ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java: ## @@ -1297,7 +1303,12 @@ public void renamePartition(Table tbl, Map oldPartSpec, Partitio } } - // TODO: this whole path won't work with catalogs + /** + * When you call this method, you need to ensure that the catalog has been set in the db object. Review Comment: Fixed in https://github.com/apache/hive/pull/6088/commits/97e8af0dadfd36703499e72d9ecbfd3c9f84da0f Thanks! -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
deniskuzZ commented on code in PR #6088: URL: https://github.com/apache/hive/pull/6088#discussion_r2616314322 ## ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java: ## @@ -1297,7 +1303,12 @@ public void renamePartition(Table tbl, Map oldPartSpec, Partitio } } - // TODO: this whole path won't work with catalogs + /** + * When you call this method, you need to ensure that the catalog has been set in the db object. Review Comment: @zhangbutao, looks like this comment slipped, but it's minor -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
deniskuzZ commented on code in PR #6088: URL: https://github.com/apache/hive/pull/6088#discussion_r2616313341 ## ql/src/test/queries/clientpositive/catalog_database.q: ## @@ -0,0 +1,69 @@ +-- SORT_QUERY_RESULTS + +-- CREATE DATABASE in default catalog 'hive' +CREATE DATABASE testdb; + +-- Check databases in default catalog 'hive', +-- The list of databases in the catalog 'hive' should only contain the default and the testdb. +SHOW DATABASES; + +-- CREATE a new catalog with comment +CREATE CATALOG testcat LOCATION '/tmp/testcat' COMMENT 'Hive test catalog'; + +-- Check catalogs list +SHOW CATALOGS; + +-- CREATE DATABASE in new catalog testcat by catalog.db pattern +CREATE DATABASE testcat.testdb_1; + +-- Switch the catalog from hive to 'testcat' +SET CATALOG testcat; + +-- Check the current catalog, should be testcat. +select current_catalog(); + +-- Switch database by catalog.db pattern, and the catalog also be changed. +USE hive.default; + +-- Check the current catalog, should be hive +select current_catalog(); + +-- CREATE DATABASE in new catalog testcat +SET CATALOG testcat; +CREATE DATABASE testdb_2; + +-- Check databases in catalog 'testcat', +-- The list of databases in the catalog 'testcat' should contain default and testdb_1 and testdb_2. +SHOW DATABASES; + +-- Switch database by catalog.db pattern +USE testcat.testdb_1; + +-- Drop database by catalog.db pattern +DROP DATABASE testcat.testdb_1; Review Comment: thanks! -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
zhangbutao commented on code in PR #6088: URL: https://github.com/apache/hive/pull/6088#discussion_r2616312177 ## ql/src/test/results/clientpositive/llap/lock1.q.out: ## @@ -98,26 +98,26 @@ PREHOOK: query: SHOW LOCKS PREHOOK: type: SHOWLOCKS POSTHOOK: query: SHOW LOCKS POSTHOOK: type: SHOWLOCKS -default@tstsrc_n1 SHARED -default@tstsrc_n1 SHARED +hive@default@tstsrc_n1 SHARED +hive@default@tstsrc_n1 SHARED PREHOOK: query: SHOW LOCKS extended PREHOOK: type: SHOWLOCKS POSTHOOK: query: SHOW LOCKS extended POSTHOOK: type: SHOWLOCKS -default@tstsrc_n1 SHARED +hive@default@tstsrc_n1 SHARED A masked pattern was here LOCK_MODE:EXPLICIT LOCK_QUERYSTRING:lock TABLE tstsrc_n1 SHARED -default@tstsrc_n1 SHARED +hive@default@tstsrc_n1 SHARED A masked pattern was here LOCK_MODE:EXPLICIT LOCK_QUERYSTRING:LOCK TABLE tstsrc_n1 SHARED PREHOOK: query: SHOW LOCKS tstsrc_n1 PREHOOK: type: SHOWLOCKS POSTHOOK: query: SHOW LOCKS tstsrc_n1 POSTHOOK: type: SHOWLOCKS -default@tstsrc_n1 SHARED -default@tstsrc_n1 SHARED +hive@default@tstsrc_n1 SHARED Review Comment: That's make sense. Fixed in https://github.com/apache/hive/pull/6088/commits/df94b419ed5c6e1017f3fd7eb53995a23848fbab -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
deniskuzZ commented on code in PR #6088: URL: https://github.com/apache/hive/pull/6088#discussion_r2616291437 ## ql/src/test/results/clientpositive/llap/lock1.q.out: ## @@ -98,26 +98,26 @@ PREHOOK: query: SHOW LOCKS PREHOOK: type: SHOWLOCKS POSTHOOK: query: SHOW LOCKS POSTHOOK: type: SHOWLOCKS -default@tstsrc_n1 SHARED -default@tstsrc_n1 SHARED +hive@default@tstsrc_n1 SHARED +hive@default@tstsrc_n1 SHARED PREHOOK: query: SHOW LOCKS extended PREHOOK: type: SHOWLOCKS POSTHOOK: query: SHOW LOCKS extended POSTHOOK: type: SHOWLOCKS -default@tstsrc_n1 SHARED +hive@default@tstsrc_n1 SHARED A masked pattern was here LOCK_MODE:EXPLICIT LOCK_QUERYSTRING:lock TABLE tstsrc_n1 SHARED -default@tstsrc_n1 SHARED +hive@default@tstsrc_n1 SHARED A masked pattern was here LOCK_MODE:EXPLICIT LOCK_QUERYSTRING:LOCK TABLE tstsrc_n1 SHARED PREHOOK: query: SHOW LOCKS tstsrc_n1 PREHOOK: type: SHOWLOCKS POSTHOOK: query: SHOW LOCKS tstsrc_n1 POSTHOOK: type: SHOWLOCKS -default@tstsrc_n1 SHARED -default@tstsrc_n1 SHARED +hive@default@tstsrc_n1 SHARED Review Comment: show locks should display locks only for the current catalog, similar to show tables for a specific database -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
deniskuzZ commented on code in PR #6088: URL: https://github.com/apache/hive/pull/6088#discussion_r2616291437 ## ql/src/test/results/clientpositive/llap/lock1.q.out: ## @@ -98,26 +98,26 @@ PREHOOK: query: SHOW LOCKS PREHOOK: type: SHOWLOCKS POSTHOOK: query: SHOW LOCKS POSTHOOK: type: SHOWLOCKS -default@tstsrc_n1 SHARED -default@tstsrc_n1 SHARED +hive@default@tstsrc_n1 SHARED +hive@default@tstsrc_n1 SHARED PREHOOK: query: SHOW LOCKS extended PREHOOK: type: SHOWLOCKS POSTHOOK: query: SHOW LOCKS extended POSTHOOK: type: SHOWLOCKS -default@tstsrc_n1 SHARED +hive@default@tstsrc_n1 SHARED A masked pattern was here LOCK_MODE:EXPLICIT LOCK_QUERYSTRING:lock TABLE tstsrc_n1 SHARED -default@tstsrc_n1 SHARED +hive@default@tstsrc_n1 SHARED A masked pattern was here LOCK_MODE:EXPLICIT LOCK_QUERYSTRING:LOCK TABLE tstsrc_n1 SHARED PREHOOK: query: SHOW LOCKS tstsrc_n1 PREHOOK: type: SHOWLOCKS POSTHOOK: query: SHOW LOCKS tstsrc_n1 POSTHOOK: type: SHOWLOCKS -default@tstsrc_n1 SHARED -default@tstsrc_n1 SHARED +hive@default@tstsrc_n1 SHARED Review Comment: show locks should display locks only for the current catalog, similar to show tables -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
deniskuzZ commented on code in PR #6088: URL: https://github.com/apache/hive/pull/6088#discussion_r2616291437 ## ql/src/test/results/clientpositive/llap/lock1.q.out: ## @@ -98,26 +98,26 @@ PREHOOK: query: SHOW LOCKS PREHOOK: type: SHOWLOCKS POSTHOOK: query: SHOW LOCKS POSTHOOK: type: SHOWLOCKS -default@tstsrc_n1 SHARED -default@tstsrc_n1 SHARED +hive@default@tstsrc_n1 SHARED +hive@default@tstsrc_n1 SHARED PREHOOK: query: SHOW LOCKS extended PREHOOK: type: SHOWLOCKS POSTHOOK: query: SHOW LOCKS extended POSTHOOK: type: SHOWLOCKS -default@tstsrc_n1 SHARED +hive@default@tstsrc_n1 SHARED A masked pattern was here LOCK_MODE:EXPLICIT LOCK_QUERYSTRING:lock TABLE tstsrc_n1 SHARED -default@tstsrc_n1 SHARED +hive@default@tstsrc_n1 SHARED A masked pattern was here LOCK_MODE:EXPLICIT LOCK_QUERYSTRING:LOCK TABLE tstsrc_n1 SHARED PREHOOK: query: SHOW LOCKS tstsrc_n1 PREHOOK: type: SHOWLOCKS POSTHOOK: query: SHOW LOCKS tstsrc_n1 POSTHOOK: type: SHOWLOCKS -default@tstsrc_n1 SHARED -default@tstsrc_n1 SHARED +hive@default@tstsrc_n1 SHARED Review Comment: show locks should display locks only for the current catalog or the provided one, similar to show tables -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
deniskuzZ commented on code in PR #6088: URL: https://github.com/apache/hive/pull/6088#discussion_r2616291437 ## ql/src/test/results/clientpositive/llap/lock1.q.out: ## @@ -98,26 +98,26 @@ PREHOOK: query: SHOW LOCKS PREHOOK: type: SHOWLOCKS POSTHOOK: query: SHOW LOCKS POSTHOOK: type: SHOWLOCKS -default@tstsrc_n1 SHARED -default@tstsrc_n1 SHARED +hive@default@tstsrc_n1 SHARED +hive@default@tstsrc_n1 SHARED PREHOOK: query: SHOW LOCKS extended PREHOOK: type: SHOWLOCKS POSTHOOK: query: SHOW LOCKS extended POSTHOOK: type: SHOWLOCKS -default@tstsrc_n1 SHARED +hive@default@tstsrc_n1 SHARED A masked pattern was here LOCK_MODE:EXPLICIT LOCK_QUERYSTRING:lock TABLE tstsrc_n1 SHARED -default@tstsrc_n1 SHARED +hive@default@tstsrc_n1 SHARED A masked pattern was here LOCK_MODE:EXPLICIT LOCK_QUERYSTRING:LOCK TABLE tstsrc_n1 SHARED PREHOOK: query: SHOW LOCKS tstsrc_n1 PREHOOK: type: SHOWLOCKS POSTHOOK: query: SHOW LOCKS tstsrc_n1 POSTHOOK: type: SHOWLOCKS -default@tstsrc_n1 SHARED -default@tstsrc_n1 SHARED +hive@default@tstsrc_n1 SHARED Review Comment: show locks should display locks only for the current catalog or the provided one -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
deniskuzZ commented on code in PR #6088: URL: https://github.com/apache/hive/pull/6088#discussion_r2616291437 ## ql/src/test/results/clientpositive/llap/lock1.q.out: ## @@ -98,26 +98,26 @@ PREHOOK: query: SHOW LOCKS PREHOOK: type: SHOWLOCKS POSTHOOK: query: SHOW LOCKS POSTHOOK: type: SHOWLOCKS -default@tstsrc_n1 SHARED -default@tstsrc_n1 SHARED +hive@default@tstsrc_n1 SHARED +hive@default@tstsrc_n1 SHARED PREHOOK: query: SHOW LOCKS extended PREHOOK: type: SHOWLOCKS POSTHOOK: query: SHOW LOCKS extended POSTHOOK: type: SHOWLOCKS -default@tstsrc_n1 SHARED +hive@default@tstsrc_n1 SHARED A masked pattern was here LOCK_MODE:EXPLICIT LOCK_QUERYSTRING:lock TABLE tstsrc_n1 SHARED -default@tstsrc_n1 SHARED +hive@default@tstsrc_n1 SHARED A masked pattern was here LOCK_MODE:EXPLICIT LOCK_QUERYSTRING:LOCK TABLE tstsrc_n1 SHARED PREHOOK: query: SHOW LOCKS tstsrc_n1 PREHOOK: type: SHOWLOCKS POSTHOOK: query: SHOW LOCKS tstsrc_n1 POSTHOOK: type: SHOWLOCKS -default@tstsrc_n1 SHARED -default@tstsrc_n1 SHARED +hive@default@tstsrc_n1 SHARED Review Comment: show locks should display locks for the current catalog or the provided one -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
deniskuzZ commented on code in PR #6088: URL: https://github.com/apache/hive/pull/6088#discussion_r2616291437 ## ql/src/test/results/clientpositive/llap/lock1.q.out: ## @@ -98,26 +98,26 @@ PREHOOK: query: SHOW LOCKS PREHOOK: type: SHOWLOCKS POSTHOOK: query: SHOW LOCKS POSTHOOK: type: SHOWLOCKS -default@tstsrc_n1 SHARED -default@tstsrc_n1 SHARED +hive@default@tstsrc_n1 SHARED +hive@default@tstsrc_n1 SHARED PREHOOK: query: SHOW LOCKS extended PREHOOK: type: SHOWLOCKS POSTHOOK: query: SHOW LOCKS extended POSTHOOK: type: SHOWLOCKS -default@tstsrc_n1 SHARED +hive@default@tstsrc_n1 SHARED A masked pattern was here LOCK_MODE:EXPLICIT LOCK_QUERYSTRING:lock TABLE tstsrc_n1 SHARED -default@tstsrc_n1 SHARED +hive@default@tstsrc_n1 SHARED A masked pattern was here LOCK_MODE:EXPLICIT LOCK_QUERYSTRING:LOCK TABLE tstsrc_n1 SHARED PREHOOK: query: SHOW LOCKS tstsrc_n1 PREHOOK: type: SHOWLOCKS POSTHOOK: query: SHOW LOCKS tstsrc_n1 POSTHOOK: type: SHOWLOCKS -default@tstsrc_n1 SHARED -default@tstsrc_n1 SHARED +hive@default@tstsrc_n1 SHARED Review Comment: show locks can only display locks for the current catalog or the provided one -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
zhangbutao commented on code in PR #6088: URL: https://github.com/apache/hive/pull/6088#discussion_r2616180778 ## ql/src/test/results/clientpositive/llap/lock1.q.out: ## @@ -98,26 +98,26 @@ PREHOOK: query: SHOW LOCKS PREHOOK: type: SHOWLOCKS POSTHOOK: query: SHOW LOCKS POSTHOOK: type: SHOWLOCKS -default@tstsrc_n1 SHARED -default@tstsrc_n1 SHARED +hive@default@tstsrc_n1 SHARED +hive@default@tstsrc_n1 SHARED PREHOOK: query: SHOW LOCKS extended PREHOOK: type: SHOWLOCKS POSTHOOK: query: SHOW LOCKS extended POSTHOOK: type: SHOWLOCKS -default@tstsrc_n1 SHARED +hive@default@tstsrc_n1 SHARED A masked pattern was here LOCK_MODE:EXPLICIT LOCK_QUERYSTRING:lock TABLE tstsrc_n1 SHARED -default@tstsrc_n1 SHARED +hive@default@tstsrc_n1 SHARED A masked pattern was here LOCK_MODE:EXPLICIT LOCK_QUERYSTRING:LOCK TABLE tstsrc_n1 SHARED PREHOOK: query: SHOW LOCKS tstsrc_n1 PREHOOK: type: SHOWLOCKS POSTHOOK: query: SHOW LOCKS tstsrc_n1 POSTHOOK: type: SHOWLOCKS -default@tstsrc_n1 SHARED -default@tstsrc_n1 SHARED +hive@default@tstsrc_n1 SHARED Review Comment: I think it's necessary to display catalog information in the "show locks" command. Since we will have multiple native Hive catalogs in the future, each catalog will have its own locks. Displaying catalog information will help users determine which catalog a lock belongs to. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
zhangbutao commented on code in PR #6088:
URL: https://github.com/apache/hive/pull/6088#discussion_r2616178952
##
ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java:
##
@@ -421,6 +421,33 @@ public static String charSetString(String charSetName,
String charSetString)
}
}
+ /**
+ *
+ * @param dbNameNode A root node that contains database fields
+ * @return Return a Pair object which includes catalogName and dbName
+ * @throws SemanticException
+ */
+ public static Pair getCatDbNamePair(ASTNode dbNameNode)
throws SemanticException {
Review Comment:
I'm not entirely sure if this method will be used in SA in the future. Given
that it's currently only used in DB for DDL operations, I'll move this code to
DDLUtils.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
zhangbutao commented on code in PR #6088: URL: https://github.com/apache/hive/pull/6088#discussion_r2616177913 ## ql/src/test/queries/clientpositive/catalog_database.q: ## @@ -0,0 +1,69 @@ +-- SORT_QUERY_RESULTS + +-- CREATE DATABASE in default catalog 'hive' +CREATE DATABASE testdb; + +-- Check databases in default catalog 'hive', +-- The list of databases in the catalog 'hive' should only contain the default and the testdb. +SHOW DATABASES; + +-- CREATE a new catalog with comment +CREATE CATALOG testcat LOCATION '/tmp/testcat' COMMENT 'Hive test catalog'; + +-- Check catalogs list +SHOW CATALOGS; + +-- CREATE DATABASE in new catalog testcat by catalog.db pattern +CREATE DATABASE testcat.testdb_1; + +-- Switch the catalog from hive to 'testcat' +SET CATALOG testcat; + +-- Check the current catalog, should be testcat. +select current_catalog(); + +-- Switch database by catalog.db pattern, and the catalog also be changed. +USE hive.default; + +-- Check the current catalog, should be hive +select current_catalog(); + +-- CREATE DATABASE in new catalog testcat +SET CATALOG testcat; +CREATE DATABASE testdb_2; + +-- Check databases in catalog 'testcat', +-- The list of databases in the catalog 'testcat' should contain default and testdb_1 and testdb_2. +SHOW DATABASES; + +-- Switch database by catalog.db pattern +USE testcat.testdb_1; + +-- Drop database by catalog.db pattern +DROP DATABASE testcat.testdb_1; Review Comment: Added qtest ql/src/test/results/clientnegative/drop_default_catalog.q.out -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
zhangbutao commented on code in PR #6088: URL: https://github.com/apache/hive/pull/6088#discussion_r2616178024 ## ql/src/java/org/apache/hadoop/hive/ql/parse/repl/load/message/CreateDatabaseHandler.java: ## @@ -55,22 +55,23 @@ public List> handle(Context context) Database db = metaData.getDatabase(); String destinationDBName = context.dbName == null ? db.getName() : context.dbName; +String destinationCatalogName = db.getCatalogName(); // TODO catalog. Need to double check the catalog here. Depend on HIVE-29278 Review Comment: Fixed. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
zhangbutao commented on code in PR #6088:
URL: https://github.com/apache/hive/pull/6088#discussion_r2616177626
##
ql/src/java/org/apache/hadoop/hive/ql/lockmgr/DummyTxnManager.java:
##
@@ -380,7 +389,7 @@ private HiveLockMode getWriteEntityLockMode (WriteEntity
we) {
}
}
- private List getLockObjects(QueryPlan plan, Database db,
+ private List getLockObjects(QueryPlan plan, Catalog catalog,
Database db,
Review Comment:
Fixed! Thx!
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
zhangbutao commented on code in PR #6088: URL: https://github.com/apache/hive/pull/6088#discussion_r2616177410 ## ql/src/java/org/apache/hadoop/hive/ql/lockmgr/HiveTxnManagerImpl.java: ## @@ -38,8 +39,10 @@ import org.apache.hadoop.hive.ql.lockmgr.HiveLockObject.HiveLockObjectData; import org.apache.hadoop.hive.ql.metadata.Hive; import org.apache.hadoop.hive.ql.metadata.HiveException; +import org.apache.hadoop.hive.ql.metadata.HiveUtils; import org.apache.hadoop.hive.ql.metadata.Partition; import org.apache.hadoop.hive.ql.metadata.Table; +import org.stringtemplate.v4.ST; Review Comment: Removed. ## parser/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g: ## @@ -379,10 +379,13 @@ TOK_CATALOGLOCATION; TOK_CATALOGCOMMENT; TOK_ALTERCATALOG_LOCATION; TOK_ALTERCATALOG_PROPERTIES; +TOK_SWITCHCATALOG; TOK_DESCDATABASE; TOK_DATABASELOCATION; TOK_DATABASE_MANAGEDLOCATION; TOK_PROPLIST; +TOK_DBPROPLIST; Review Comment: Removed. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
deniskuzZ commented on PR #6088: URL: https://github.com/apache/hive/pull/6088#issuecomment-3637250217 @zhangbutao, I've added few more comments , otherwise LGTM. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
deniskuzZ commented on code in PR #6088:
URL: https://github.com/apache/hive/pull/6088#discussion_r2606789910
##
ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java:
##
@@ -421,6 +421,33 @@ public static String charSetString(String charSetName,
String charSetString)
}
}
+ /**
+ *
+ * @param dbNameNode A root node that contains database fields
+ * @return Return a Pair object which includes catalogName and dbName
+ * @throws SemanticException
+ */
+ public static Pair getCatDbNamePair(ASTNode dbNameNode)
throws SemanticException {
Review Comment:
it doesn't belong in SA, maybe move to util class
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
deniskuzZ commented on code in PR #6088: URL: https://github.com/apache/hive/pull/6088#discussion_r2606782145 ## ql/src/java/org/apache/hadoop/hive/ql/parse/repl/load/message/CreateDatabaseHandler.java: ## @@ -55,22 +55,23 @@ public List> handle(Context context) Database db = metaData.getDatabase(); String destinationDBName = context.dbName == null ? db.getName() : context.dbName; +String destinationCatalogName = db.getCatalogName(); // TODO catalog. Need to double check the catalog here. Depend on HIVE-29278 Review Comment: could we shorten `destinationCatalogName` -> `destCatalogName`. same for destDBName -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
deniskuzZ commented on code in PR #6088: URL: https://github.com/apache/hive/pull/6088#discussion_r2606774937 ## ql/src/test/results/clientpositive/llap/lock1.q.out: ## @@ -98,26 +98,26 @@ PREHOOK: query: SHOW LOCKS PREHOOK: type: SHOWLOCKS POSTHOOK: query: SHOW LOCKS POSTHOOK: type: SHOWLOCKS -default@tstsrc_n1 SHARED -default@tstsrc_n1 SHARED +hive@default@tstsrc_n1 SHARED +hive@default@tstsrc_n1 SHARED PREHOOK: query: SHOW LOCKS extended PREHOOK: type: SHOWLOCKS POSTHOOK: query: SHOW LOCKS extended POSTHOOK: type: SHOWLOCKS -default@tstsrc_n1 SHARED +hive@default@tstsrc_n1 SHARED A masked pattern was here LOCK_MODE:EXPLICIT LOCK_QUERYSTRING:lock TABLE tstsrc_n1 SHARED -default@tstsrc_n1 SHARED +hive@default@tstsrc_n1 SHARED A masked pattern was here LOCK_MODE:EXPLICIT LOCK_QUERYSTRING:LOCK TABLE tstsrc_n1 SHARED PREHOOK: query: SHOW LOCKS tstsrc_n1 PREHOOK: type: SHOWLOCKS POSTHOOK: query: SHOW LOCKS tstsrc_n1 POSTHOOK: type: SHOWLOCKS -default@tstsrc_n1 SHARED -default@tstsrc_n1 SHARED +hive@default@tstsrc_n1 SHARED Review Comment: No need for catalog in show tables or show locks; -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
deniskuzZ commented on code in PR #6088: URL: https://github.com/apache/hive/pull/6088#discussion_r2606771681 ## ql/src/test/queries/clientpositive/catalog_database.q: ## @@ -0,0 +1,69 @@ +-- SORT_QUERY_RESULTS + +-- CREATE DATABASE in default catalog 'hive' +CREATE DATABASE testdb; + +-- Check databases in default catalog 'hive', +-- The list of databases in the catalog 'hive' should only contain the default and the testdb. +SHOW DATABASES; + +-- CREATE a new catalog with comment +CREATE CATALOG testcat LOCATION '/tmp/testcat' COMMENT 'Hive test catalog'; + +-- Check catalogs list +SHOW CATALOGS; + +-- CREATE DATABASE in new catalog testcat by catalog.db pattern +CREATE DATABASE testcat.testdb_1; + +-- Switch the catalog from hive to 'testcat' +SET CATALOG testcat; + +-- Check the current catalog, should be testcat. +select current_catalog(); + +-- Switch database by catalog.db pattern, and the catalog also be changed. +USE hive.default; + +-- Check the current catalog, should be hive +select current_catalog(); + +-- CREATE DATABASE in new catalog testcat +SET CATALOG testcat; +CREATE DATABASE testdb_2; + +-- Check databases in catalog 'testcat', +-- The list of databases in the catalog 'testcat' should contain default and testdb_1 and testdb_2. +SHOW DATABASES; + +-- Switch database by catalog.db pattern +USE testcat.testdb_1; + +-- Drop database by catalog.db pattern +DROP DATABASE testcat.testdb_1; Review Comment: Do we have tests ensuring that dropping the current catalog or the Hive default catalog is not allowed? -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
deniskuzZ commented on code in PR #6088:
URL: https://github.com/apache/hive/pull/6088#discussion_r2606744513
##
ql/src/java/org/apache/hadoop/hive/ql/lockmgr/DummyTxnManager.java:
##
@@ -380,7 +389,7 @@ private HiveLockMode getWriteEntityLockMode (WriteEntity
we) {
}
}
- private List getLockObjects(QueryPlan plan, Database db,
+ private List getLockObjects(QueryPlan plan, Catalog catalog,
Database db,
Review Comment:
could we make method generic:
private List getLockObjects(QueryPlan plan, T target,
HiveLockMode mode)
throws LockException {
List locks = new LinkedList();
switch (target) {
case Catalog catalog -> {
locks.add(new HiveLockObj(new HiveLockObject(catalog.getName(),
lockData), mode));
}
case Database db -> {
String catName = Objects.requireNonNullElse(db.getCatalogName(),
HiveUtils.getCurrentCatalogOrDefault(conf));
locks.add(new HiveLockObj(new HiveLockObject(catName, lockData), mode));
db.setCatalogName(catName);
locks.add(new HiveLockObj(new HiveLockObject(db, lockData), mode));
}
...
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
deniskuzZ commented on code in PR #6088:
URL: https://github.com/apache/hive/pull/6088#discussion_r2606744513
##
ql/src/java/org/apache/hadoop/hive/ql/lockmgr/DummyTxnManager.java:
##
@@ -380,7 +389,7 @@ private HiveLockMode getWriteEntityLockMode (WriteEntity
we) {
}
}
- private List getLockObjects(QueryPlan plan, Database db,
+ private List getLockObjects(QueryPlan plan, Catalog catalog,
Database db,
Review Comment:
could we make method generic:
private List getLockObjects(QueryPlan plan, T target,
HiveLockMode mode)
throws LockException {
List locks = new LinkedList();
switch (target) {
case Catalog catalog:
locks.add(new HiveLockObj(new HiveLockObject(catalog.getName(),
lockData), mode));
return locks;
case Database db:
String catName = Objects.requireNonNullElse(db.getCatalogName(),
HiveUtils.getCurrentCatalogOrDefault(conf));
locks.add(new HiveLockObj(new HiveLockObject(catName, lockData), mode));
db.setCatalogName(catName);
locks.add(new HiveLockObj(new HiveLockObject(db, lockData), mode));
return locks;
...
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
deniskuzZ commented on code in PR #6088:
URL: https://github.com/apache/hive/pull/6088#discussion_r2606744513
##
ql/src/java/org/apache/hadoop/hive/ql/lockmgr/DummyTxnManager.java:
##
@@ -380,7 +389,7 @@ private HiveLockMode getWriteEntityLockMode (WriteEntity
we) {
}
}
- private List getLockObjects(QueryPlan plan, Database db,
+ private List getLockObjects(QueryPlan plan, Catalog catalog,
Database db,
Review Comment:
could we make method generic:
private List getLockObjects(QueryPlan plan, T target,
HiveLockMode mode)
throws LockException {
List locks = new LinkedList();
switch (target) {
case Catalog catalog:
locks.add(new HiveLockObj(new HiveLockObject(catalog.getName(),
lockData), mode));
return locks;
case Database db:
String catName = Objects.requireNonNullElse(db.getCatalogName(),
HiveUtils.getCurrentCatalogOrDefault(conf));
locks.add(new HiveLockObj(new HiveLockObject(catName, lockData), mode));
db.setCatalogName(catName);
locks.add(new HiveLockObj(new HiveLockObject(db, lockData), mode));
...
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
deniskuzZ commented on code in PR #6088:
URL: https://github.com/apache/hive/pull/6088#discussion_r2606744513
##
ql/src/java/org/apache/hadoop/hive/ql/lockmgr/DummyTxnManager.java:
##
@@ -380,7 +389,7 @@ private HiveLockMode getWriteEntityLockMode (WriteEntity
we) {
}
}
- private List getLockObjects(QueryPlan plan, Database db,
+ private List getLockObjects(QueryPlan plan, Catalog catalog,
Database db,
Review Comment:
could we make method generic:
private List getLockObjects(QueryPlan plan, T target,
HiveLockMode mode) throws LockException {
List locks = new LinkedList();
switch (target) {
case Catalog catalog:
locks.add(new HiveLockObj(new HiveLockObject(catalog.getName(),
lockData), mode));
case Database db:
String catName = Objects.requireNonNullElse(db.getCatalogName(),
HiveUtils.getCurrentCatalogOrDefault(conf));
locks.add(new HiveLockObj(new HiveLockObject(catName, lockData), mode));
db.setCatalogName(catName);
locks.add(new HiveLockObj(new HiveLockObject(db, lockData), mode));
...
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
deniskuzZ commented on code in PR #6088:
URL: https://github.com/apache/hive/pull/6088#discussion_r2606744513
##
ql/src/java/org/apache/hadoop/hive/ql/lockmgr/DummyTxnManager.java:
##
@@ -380,7 +389,7 @@ private HiveLockMode getWriteEntityLockMode (WriteEntity
we) {
}
}
- private List getLockObjects(QueryPlan plan, Database db,
+ private List getLockObjects(QueryPlan plan, Catalog catalog,
Database db,
Review Comment:
could we make method generic:
private List getLockObjects(QueryPlan plan, T target,
HiveLockMode mode)
throws LockException {
List locks = new LinkedList();
switch (target) {
case Catalog catalog:
locks.add(new HiveLockObj(new HiveLockObject(catalog.getName(),
lockData), mode));
case Database db:
String catName = Objects.requireNonNullElse(db.getCatalogName(),
HiveUtils.getCurrentCatalogOrDefault(conf));
locks.add(new HiveLockObj(new HiveLockObject(catName, lockData), mode));
db.setCatalogName(catName);
locks.add(new HiveLockObj(new HiveLockObject(db, lockData), mode));
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
deniskuzZ commented on code in PR #6088: URL: https://github.com/apache/hive/pull/6088#discussion_r2605977637 ## parser/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g: ## @@ -379,10 +379,13 @@ TOK_CATALOGLOCATION; TOK_CATALOGCOMMENT; TOK_ALTERCATALOG_LOCATION; TOK_ALTERCATALOG_PROPERTIES; +TOK_SWITCHCATALOG; TOK_DESCDATABASE; TOK_DATABASELOCATION; TOK_DATABASE_MANAGEDLOCATION; TOK_PROPLIST; +TOK_DBPROPLIST; Review Comment: where is that used? -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
deniskuzZ commented on code in PR #6088: URL: https://github.com/apache/hive/pull/6088#discussion_r2605949320 ## parser/src/java/org/apache/hadoop/hive/ql/parse/AlterClauseParser.g: ## @@ -50,7 +50,7 @@ alterStatement : KW_ALTER KW_TABLE tableName alterTableStatementSuffix -> ^(TOK_ALTERTABLE tableName alterTableStatementSuffix) | KW_ALTER KW_VIEW tableName KW_AS? alterViewStatementSuffix -> ^(TOK_ALTERVIEW tableName alterViewStatementSuffix) | KW_ALTER KW_MATERIALIZED KW_VIEW tableNameTree=tableName alterMaterializedViewStatementSuffix[$tableNameTree.tree] -> alterMaterializedViewStatementSuffix -| KW_ALTER (KW_DATABASE|KW_SCHEMA) alterDatabaseStatementSuffix -> alterDatabaseStatementSuffix +| KW_ALTER (KW_DATABASE|KW_SCHEMA) databaseName alterDatabaseStatementSuffix -> ^(TOK_ALTERDATABASE databaseName alterDatabaseStatementSuffix) Review Comment: can we use `name=identifier` instead of databaseName? -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
deniskuzZ commented on code in PR #6088: URL: https://github.com/apache/hive/pull/6088#discussion_r2605949320 ## parser/src/java/org/apache/hadoop/hive/ql/parse/AlterClauseParser.g: ## @@ -50,7 +50,7 @@ alterStatement : KW_ALTER KW_TABLE tableName alterTableStatementSuffix -> ^(TOK_ALTERTABLE tableName alterTableStatementSuffix) | KW_ALTER KW_VIEW tableName KW_AS? alterViewStatementSuffix -> ^(TOK_ALTERVIEW tableName alterViewStatementSuffix) | KW_ALTER KW_MATERIALIZED KW_VIEW tableNameTree=tableName alterMaterializedViewStatementSuffix[$tableNameTree.tree] -> alterMaterializedViewStatementSuffix -| KW_ALTER (KW_DATABASE|KW_SCHEMA) alterDatabaseStatementSuffix -> alterDatabaseStatementSuffix +| KW_ALTER (KW_DATABASE|KW_SCHEMA) databaseName alterDatabaseStatementSuffix -> ^(TOK_ALTERDATABASE databaseName alterDatabaseStatementSuffix) Review Comment: can we use `name=identifier` instead of databaseName? -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
deniskuzZ commented on code in PR #6088: URL: https://github.com/apache/hive/pull/6088#discussion_r2605912482 ## ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java: ## @@ -1297,7 +1303,12 @@ public void renamePartition(Table tbl, Map oldPartSpec, Partitio } } - // TODO: this whole path won't work with catalogs + /** + * When you call this method, you need to ensure that the catalog has been set in the db object. Review Comment: could we squash MetaException & TException handling -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
deniskuzZ commented on code in PR #6088: URL: https://github.com/apache/hive/pull/6088#discussion_r2605895503 ## ql/src/java/org/apache/hadoop/hive/ql/lockmgr/HiveTxnManagerImpl.java: ## @@ -38,8 +39,10 @@ import org.apache.hadoop.hive.ql.lockmgr.HiveLockObject.HiveLockObjectData; import org.apache.hadoop.hive.ql.metadata.Hive; import org.apache.hadoop.hive.ql.metadata.HiveException; +import org.apache.hadoop.hive.ql.metadata.HiveUtils; import org.apache.hadoop.hive.ql.metadata.Partition; import org.apache.hadoop.hive.ql.metadata.Table; +import org.stringtemplate.v4.ST; Review Comment: redundant import -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
zhangbutao commented on code in PR #6088: URL: https://github.com/apache/hive/pull/6088#discussion_r2596775030 ## ql/src/java/org/apache/hadoop/hive/ql/lockmgr/HiveTxnManagerImpl.java: ## @@ -162,7 +167,8 @@ public int lockDatabase(Hive hiveDB, LockDatabaseDesc lockDb) throws HiveExcepti String.valueOf(System.currentTimeMillis()), "EXPLICIT", lockDb.getQueryStr(), conf); -HiveLock lck = lockMgr.lock(new HiveLockObject(dbObj.getName(), lockData), mode, true); +// Using the catalogName@databaseName format to uniquely identify a database. +HiveLock lck = lockMgr.lock(new HiveLockObject(catName + "@" +dbObj.getName(), lockData), mode, true); Review Comment: Fixed in https://github.com/apache/hive/pull/6088/commits/f066b1ea51f109982d0a7e960b531d44a912ee69 -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
sonarqubecloud[bot] commented on PR #6088: URL: https://github.com/apache/hive/pull/6088#issuecomment-3622472801 ## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=6088) **Quality Gate passed** Issues  [117 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=6088&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=6088&issueStatuses=ACCEPTED) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=6088&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=6088&metric=new_coverage&view=list)  [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=6088&metric=new_duplicated_lines_density&view=list) [See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=6088) -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
sonarqubecloud[bot] commented on PR #6088: URL: https://github.com/apache/hive/pull/6088#issuecomment-3621923079 ## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=6088) **Quality Gate passed** Issues  [117 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=6088&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=6088&issueStatuses=ACCEPTED) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=6088&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=6088&metric=new_coverage&view=list)  [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=6088&metric=new_duplicated_lines_density&view=list) [See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=6088) -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
sonarqubecloud[bot] commented on PR #6088: URL: https://github.com/apache/hive/pull/6088#issuecomment-3621669878 ## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=6088) **Quality Gate passed** Issues  [120 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=6088&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=6088&issueStatuses=ACCEPTED) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=6088&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=6088&metric=new_coverage&view=list)  [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=6088&metric=new_duplicated_lines_density&view=list) [See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=6088) -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
zhangbutao commented on code in PR #6088: URL: https://github.com/apache/hive/pull/6088#discussion_r2581022053 ## ql/src/java/org/apache/hadoop/hive/ql/lockmgr/HiveTxnManagerImpl.java: ## @@ -162,7 +167,8 @@ public int lockDatabase(Hive hiveDB, LockDatabaseDesc lockDb) throws HiveExcepti String.valueOf(System.currentTimeMillis()), "EXPLICIT", lockDb.getQueryStr(), conf); -HiveLock lck = lockMgr.lock(new HiveLockObject(dbObj.getName(), lockData), mode, true); +// Using the catalogName@databaseName format to uniquely identify a database. +HiveLock lck = lockMgr.lock(new HiveLockObject(catName + "@" +dbObj.getName(), lockData), mode, true); Review Comment: There are some issues with the current database locks; I've just submitted another PR to fix these problems. Please see https://github.com/apache/hive/pull/6222 -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
deniskuzZ commented on PR #6088: URL: https://github.com/apache/hive/pull/6088#issuecomment-3590022648 > @deniskuzZ Thank you very much for your review. I've been quite busy with various matters recently, which has caused some delay in addressing these comments. My apologies. nw at all. thank you @zhangbutao for driving this feature. I think it would be a game-changer -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
zhangbutao commented on code in PR #6088: URL: https://github.com/apache/hive/pull/6088#discussion_r2567056771 ## ql/src/java/org/apache/hadoop/hive/ql/lockmgr/HiveTxnManagerImpl.java: ## @@ -162,7 +167,8 @@ public int lockDatabase(Hive hiveDB, LockDatabaseDesc lockDb) throws HiveExcepti String.valueOf(System.currentTimeMillis()), "EXPLICIT", lockDb.getQueryStr(), conf); -HiveLock lck = lockMgr.lock(new HiveLockObject(dbObj.getName(), lockData), mode, true); +// Using the catalogName@databaseName format to uniquely identify a database. +HiveLock lck = lockMgr.lock(new HiveLockObject(catName + "@" +dbObj.getName(), lockData), mode, true); Review Comment: Upon further inspection here, it was found that HIVE-2093 introduced the semantics for SHOW LOCKS DATABASE, specifically designed to retrieve the lock objects for a particular database. Therefore, I still need to spend some time confirming this. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
sonarqubecloud[bot] commented on PR #6088: URL: https://github.com/apache/hive/pull/6088#issuecomment-3583612233 ## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=6088) **Quality Gate passed** Issues  [109 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=6088&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=6088&issueStatuses=ACCEPTED) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=6088&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=6088&metric=new_coverage&view=list)  [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=6088&metric=new_duplicated_lines_density&view=list) [See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=6088) -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
zhangbutao commented on PR #6088: URL: https://github.com/apache/hive/pull/6088#issuecomment-3580917440 @deniskuzZ Thank you very much for your review. I've been quite busy with various matters recently, which has caused some delay in addressing these comments. My apologies. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
sonarqubecloud[bot] commented on PR #6088: URL: https://github.com/apache/hive/pull/6088#issuecomment-3580911857 ## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=6088) **Quality Gate passed** Issues  [107 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=6088&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=6088&issueStatuses=ACCEPTED) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=6088&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=6088&metric=new_coverage&view=list)  [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=6088&metric=new_duplicated_lines_density&view=list) [See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=6088) -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
zhangbutao commented on PR #6088: URL: https://github.com/apache/hive/pull/6088#issuecomment-3580907907 @dengzhhu653 Thanks for the suggesstion! > It seems to me this is a significant/big change, and the user should follow a new way of catalog.db.table to request the table as the Trino/Presto does today. That doesn't seem quite right. My goal is that even if users don't use this feature, they can still use the classic Hive approach, such as `select * from table` or `select * from db.table`. Users don't need to worry about catalogs because their current session has a default catalog. Of course, users can also specify a catalog name as a prefix to query a table, for example, `select * from hive.db.tbl`. Additionally, if users want to perform federated queries across multiple data sources in Hive, similar to Trino, they would need to create multiple catalogs and execute similar queries. This federated query capability is a new feature and does not pose compatibility issues for users. ``` SELECT u.user_name, a.action_time FROM mysql.test.users u JOIN hive.default.user_actions a ON u.user_id = a.user_id; ``` > I think we can have a more simple idea, let's push the catalog awareness down into the Metastore client. For example, > SET CATALOG testcat; -> change to a metaconf setting: set metaconf:metastore.catalog.default = testcat, default hive, this configuration will be propagated to HMS, every HMS API request will be appended with the testcat, both client and server can handle it. Although this allows switching catalogs, the current client can only access the databases and tables of one catalog at a time. > For cross-catalog queries, we can also do it in a similar way, we can introduce a new catalog awareness lawyer between session client and the thrift client, as SessionHiveMetaStoreClient -> CatalogAwarenessMetastoreClient -> ThriftHiveMetaStoreClient. The CatalogAwarenessMetastoreClient can acknowledge of the catalog through configuration or properties stored in HMS([HIVE-27186](https://issues.apache.org/jira/browse/HIVE-27186)). Something as hive.matastore.catalog.mapping.pattern=iceberg_catalog:ice_db1*,hive_db1,hive_db2.ice_tab*;hive:default,acid_db*, the CatalogAwarenessMetastoreClient even can point to a third-party catalog partner. This method allows accessing databases and tables across multiple catalogs, but it is quite inelegant. Moreover, as you mentioned, it cannot support having the same database names across different catalogs. This solution is somewhat similar to https://github.com/ExpediaGroup/waggle-dance. Additionally, it requires users to constantly modify parameters to add or remove database names in `hive.matastore.catalog.mapping.pattern`, which seems very cumbersome... Finally, I want to reference the multi/federated catalog capabilities of Trino and StarRocks/Doris. I believe their design approaches are quite reasonable. I'm considering implementing this capability with the principle of minimizing code modifications. Considerable preliminary work has already been done on the HMS side with HIVE-18685. The subsequent work mainly involves connecting all catalog capabilities from the HS2 side to the HMS side. I've been thinking about writing down the related design considerations, but haven't had enough time yet... I will provide more thoughts on this later. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
zhangbutao commented on code in PR #6088: URL: https://github.com/apache/hive/pull/6088#discussion_r2564555443 ## ql/src/test/queries/clientnegative/lockneg_try_lock_cat_db_in_use.q: ## @@ -0,0 +1,9 @@ +set hive.lock.numretries=0; +set hive.support.concurrency=true; Review Comment: lock/unlock need this. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
zhangbutao commented on code in PR #6088: URL: https://github.com/apache/hive/pull/6088#discussion_r2564554316 ## ql/src/test/queries/clientpositive/catalog.q: ## @@ -45,3 +45,6 @@ SHOW CATALOGS LIKE 'test__'; -- ALTER LOCATION ALTER CATALOG test_cat SET LOCATION '/tmp/test_cat_new'; DESC CATALOG EXTENDED test_cat; + +-- DROP catalog at the end +DROP CATALOG test_cat; Review Comment: Fixed in https://github.com/apache/hive/pull/6088/commits/00a75f9fe1d13cc9e49a00a3cd2b598dadef1f0b -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
zhangbutao commented on code in PR #6088:
URL: https://github.com/apache/hive/pull/6088#discussion_r2564552762
##
ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java:
##
@@ -6463,6 +6564,18 @@ public List getFunctionsInDb(String dbName,
String pattern) throws Hiv
}
}
+ public List getFunctionsInDb(String catName, String dbName, String
pattern) throws HiveException {
Review Comment:
Fixed in
https://github.com/apache/hive/pull/6088/commits/cca5bdcfad23fa3653c3c51129c48dc21cce459d
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
zhangbutao commented on code in PR #6088:
URL: https://github.com/apache/hive/pull/6088#discussion_r2564540795
##
ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java:
##
@@ -6463,6 +6564,18 @@ public List getFunctionsInDb(String dbName,
String pattern) throws Hiv
}
}
+ public List getFunctionsInDb(String catName, String dbName, String
pattern) throws HiveException {
Review Comment:
See https://github.com/apache/hive/pull/6088#discussion_r2564531315
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
zhangbutao commented on code in PR #6088:
URL: https://github.com/apache/hive/pull/6088#discussion_r2564540795
##
ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java:
##
@@ -6463,6 +6564,18 @@ public List getFunctionsInDb(String dbName,
String pattern) throws Hiv
}
}
+ public List getFunctionsInDb(String catName, String dbName, String
pattern) throws HiveException {
Review Comment:
See https://github.com/apache/hive/pull/6088#discussion_r2564531315
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
zhangbutao commented on code in PR #6088:
URL: https://github.com/apache/hive/pull/6088#discussion_r2564538946
##
ql/src/java/org/apache/hadoop/hive/ql/metadata/Table.java:
##
@@ -1198,6 +1198,10 @@ public String getCatalogName() {
return this.tTable.getCatName();
}
+ public void setCatalogName(String catalogName) {
Review Comment:
Yes. We need. We may create a table like this create table
catName.dbName.tblName(id int);
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
zhangbutao commented on code in PR #6088:
URL: https://github.com/apache/hive/pull/6088#discussion_r2564534844
##
ql/src/java/org/apache/hadoop/hive/ql/metadata/HiveUtils.java:
##
@@ -543,4 +546,10 @@ public static String getLowerCaseTableName(String refName)
{
}
return refName.toLowerCase();
}
+
+ public static String getCurrentCatalogOrDefault(Configuration conf) {
+return SessionState.get() != null ?
Review Comment:
Fixed in
https://github.com/apache/hive/pull/6088/commits/00a75f9fe1d13cc9e49a00a3cd2b598dadef1f0b
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
zhangbutao commented on code in PR #6088: URL: https://github.com/apache/hive/pull/6088#discussion_r2564533177 ## ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java: ## @@ -1997,6 +2035,52 @@ public List getTablesByType(String dbName, String pattern, TableType typ } } + /** + * Returns all existing tables of a type (VIRTUAL_VIEW|EXTERNAL_TABLE|MANAGED_TABLE) from the specified + * database which match the given pattern. The matching occurs as per Java regular expressions. + * @param catName catalog name to find the tables in. if null, uses the current catalog in this session. + * @param dbName Database name to find the tables in. if null, uses the current database in this session. + * @param pattern A pattern to match for the table names.If null, returns all names from this DB. + * @param type The type of tables to return. VIRTUAL_VIEWS for views. If null, returns all tables and views. + * @return list of table names that match the pattern. + * @throws HiveException + */ + public List getTablesByType(String catName, String dbName, String pattern, TableType type) Review Comment: Fixed in https://github.com/apache/hive/pull/6088/commits/00a75f9fe1d13cc9e49a00a3cd2b598dadef1f0b -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
zhangbutao commented on code in PR #6088:
URL: https://github.com/apache/hive/pull/6088#discussion_r2564531315
##
ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java:
##
@@ -1914,6 +1942,16 @@ public Table
apply(org.apache.hadoop.hive.metastore.api.Table table) {
}
}
+ public List getTableObjects(String catName, String dbName, String
pattern, TableType tableType) throws HiveException {
Review Comment:
I just follow other method pattern. There are still many methods with
numerous parameters like this. Perhaps we can incrementally optimize them
later. However, these methods might be used by other components like Spark, so
we may need to proceed with caution.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
zhangbutao commented on code in PR #6088: URL: https://github.com/apache/hive/pull/6088#discussion_r2564520075 ## ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java: ## @@ -1297,7 +1303,12 @@ public void renamePartition(Table tbl, Map oldPartSpec, Partitio } } - // TODO: this whole path won't work with catalogs + /** + * When you call this method, you need to ensure that the catalog has been set in the db object. Review Comment: https://github.com/apache/hive/pull/6088/files#diff-0b11c3fb6f813b496b291de76cbdf98dff3e7f47d1ba4d8bb68c4dedb858da42R516 In this method, the call to` BaseMetaStoreClient::alterDatabase(String name, Database db)` will retrieve the specific catalog name based on the current session or the db object. However, we need to ensure that the db object preferably contains the catalog name, because references in the format "cat.db" might be unrelated to the current session's catalog. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
zhangbutao commented on code in PR #6088: URL: https://github.com/apache/hive/pull/6088#discussion_r2564479861 ## ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java: ## @@ -721,9 +726,10 @@ public void dropDatabase(String name, boolean deleteData, boolean ignoreUnknownD * @throws HiveException * @throws NoSuchObjectException */ + @Deprecated Review Comment: Fixed in https://github.com/apache/hive/pull/6088/commits/00a75f9fe1d13cc9e49a00a3cd2b598dadef1f0b -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
zhangbutao commented on code in PR #6088:
URL: https://github.com/apache/hive/pull/6088#discussion_r2564479034
##
ql/src/java/org/apache/hadoop/hive/ql/lockmgr/HiveTxnManagerImpl.java:
##
@@ -173,13 +179,15 @@ public int lockDatabase(Hive hiveDB, LockDatabaseDesc
lockDb) throws HiveExcepti
public int unlockDatabase(Hive hiveDB, UnlockDatabaseDesc unlockDb) throws
HiveException {
HiveLockManager lockMgr = getAndCheckLockManager();
+String catName = Objects.requireNonNullElse(unlockDb.getCatalogName(),
+HiveUtils.getCurrentCatalogOrDefault(conf));
String dbName = unlockDb.getDatabaseName();
-Database dbObj = hiveDB.getDatabase(dbName);
+Database dbObj = hiveDB.getDatabase(catName, dbName);
if (dbObj == null) {
throw new HiveException("Database " + dbName + " does not exist ");
}
-HiveLockObject obj = new HiveLockObject(dbObj.getName(), null);
+HiveLockObject obj = new HiveLockObject(catName + "@" +dbObj.getName(),
null);
Review Comment:
Fixed in
https://github.com/apache/hive/pull/6088/commits/88a24cd836431409358a643dfa570de2141aba5a
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
zhangbutao commented on code in PR #6088: URL: https://github.com/apache/hive/pull/6088#discussion_r2564478222 ## ql/src/java/org/apache/hadoop/hive/ql/lockmgr/HiveTxnManagerImpl.java: ## @@ -162,7 +167,8 @@ public int lockDatabase(Hive hiveDB, LockDatabaseDesc lockDb) throws HiveExcepti String.valueOf(System.currentTimeMillis()), "EXPLICIT", lockDb.getQueryStr(), conf); -HiveLock lck = lockMgr.lock(new HiveLockObject(dbObj.getName(), lockData), mode, true); +// Using the catalogName@databaseName format to uniquely identify a database. +HiveLock lck = lockMgr.lock(new HiveLockObject(catName + "@" +dbObj.getName(), lockData), mode, true); Review Comment: Fixed in https://github.com/apache/hive/pull/6088/commits/88a24cd836431409358a643dfa570de2141aba5a I suddenly realized that this hack isn't a good approach. I referenced the method for obtaining lock objects for tables and created a similar method for databases, which might be better. Additionally, I noticed that the SHOW LOCKS statement could only display table-related locks but not database-related locks. Here's the test query, referenced from lockneg_try_lock_db_in_use.q: ``` set hive.lock.numretries=0; set hive.support.concurrency=true; create database lockneg2; lock database lockneg2 shared; show locks; ``` After this code optimization https://github.com/apache/hive/pull/6088/commits/88a24cd836431409358a643dfa570de2141aba5a, the lock information for lockneg2 can now be seen: ``` 0: jdbc:hive2://127.0.0.1:1/default> show locks; INFO : Compiling command(queryId=hive_20251126170301_32696de3-7d1e-490f-a3b4-d956be706897): show locks ++-+ |tab_name| mode | ++-+ | hive@lockneg2 | SHARED | ++-+ ``` -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
zhangbutao commented on code in PR #6088:
URL: https://github.com/apache/hive/pull/6088#discussion_r2564452815
##
ql/src/java/org/apache/hadoop/hive/ql/ddl/database/drop/DropDatabaseAnalyzer.java:
##
@@ -51,12 +53,17 @@ public DropDatabaseAnalyzer(QueryState queryState) throws
SemanticException {
@Override
public void analyzeInternal(ASTNode root) throws SemanticException {
-String databaseName = unescapeIdentifier(root.getChild(0).getText());
+Pair catDbNamePair = getCatDbNamePair((ASTNode)
root.getChild(0));
boolean ifExists = root.getFirstChildWithType(HiveParser.TOK_IFEXISTS) !=
null;
boolean cascade = root.getFirstChildWithType(HiveParser.TOK_CASCADE) !=
null;
boolean isSoftDelete = HiveConf.getBoolVar(conf,
HiveConf.ConfVars.HIVE_ACID_LOCKLESS_READS_ENABLED);
-Database database = getDatabase(databaseName, !ifExists);
+String catalogName = catDbNamePair.getLeft();
+if (catalogName != null && getCatalog(catalogName) == null) {
+ throw new SemanticException(ErrorMsg.CATALOG_NOT_EXISTS, catalogName);
+}
+String databaseName = catDbNamePair.getRight();
+Database database = getDatabase(catDbNamePair.getLeft(),
catDbNamePair.getRight(), !ifExists);
Review Comment:
Fixed in
https://github.com/apache/hive/pull/6088/commits/00a75f9fe1d13cc9e49a00a3cd2b598dadef1f0b
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
zhangbutao commented on code in PR #6088:
URL: https://github.com/apache/hive/pull/6088#discussion_r2564450081
##
ql/src/java/org/apache/hadoop/hive/ql/ddl/database/create/CreateDatabaseDesc.java:
##
@@ -45,13 +46,14 @@ public class CreateDatabaseDesc implements DDLDesc,
Serializable {
private final String remoteDbName;
private final Map dbProperties;
- public CreateDatabaseDesc(String databaseName, String comment, String
locationUri, String managedLocationUri,
+ public CreateDatabaseDesc(String catalogName, String databaseName, String
comment, String locationUri, String managedLocationUri,
boolean ifNotExists, Map dbProperties) {
-this(databaseName, comment, locationUri, managedLocationUri, ifNotExists,
dbProperties, "NATIVE", null, null);
+this(catalogName, databaseName, comment, locationUri, managedLocationUri,
ifNotExists, dbProperties, "NATIVE", null, null);
Review Comment:
> should we introduce a flag like native/non-native similar to tables?
This can be done when we implement specific catalogs (JDBC catalog & Iceberg
catalog, etc.).
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
zhangbutao commented on code in PR #6088:
URL: https://github.com/apache/hive/pull/6088#discussion_r256588
##
ql/src/java/org/apache/hadoop/hive/ql/ddl/catalog/use/SwitchCatalogDesc.java:
##
@@ -0,0 +1,43 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hive.ql.ddl.catalog.use;
+
+import org.apache.hadoop.hive.ql.ddl.DDLDesc;
+import org.apache.hadoop.hive.ql.plan.Explain;
+
+import java.io.Serializable;
+
+/**
+ * DDL task description for SET CATALOG commands.
+ */
+@Explain(displayName = "Switch Catalog", explainLevels = { Explain.Level.USER,
Explain.Level.DEFAULT, Explain.Level.EXTENDED })
+public class SwitchCatalogDesc implements DDLDesc, Serializable {
+private static final long serialVersionUID = 1L;
+
+private final String catalogName;
+
+public SwitchCatalogDesc(String databaseName) {
Review Comment:
Fixed in
https://github.com/apache/hive/pull/6088/commits/00a75f9fe1d13cc9e49a00a3cd2b598dadef1f0b
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
zhangbutao commented on code in PR #6088:
URL: https://github.com/apache/hive/pull/6088#discussion_r2564442838
##
ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java:
##
@@ -1926,6 +1960,19 @@ protected Database getDatabase(String dbName, boolean
throwException) throws Sem
return database;
}
+ protected Database getDatabase(String catalogName, String dbName, boolean
throwException) throws SemanticException {
Review Comment:
Fixed in
https://github.com/apache/hive/pull/6088/commits/00a75f9fe1d13cc9e49a00a3cd2b598dadef1f0b
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
zhangbutao commented on code in PR #6088:
URL: https://github.com/apache/hive/pull/6088#discussion_r2564443676
##
common/src/java/org/apache/hadoop/hive/ql/ErrorMsg.java:
##
@@ -468,7 +468,7 @@ public enum ErrorMsg {
RESOURCE_PLAN_ALREADY_EXISTS(10417, "Resource plan {0} already exists",
true),
RESOURCE_PLAN_NOT_EXISTS(10418, "Resource plan {0} does not exist", true),
INCOMPATIBLE_STRUCT(10419, "Incompatible structs.", true),
- OBJECTNAME_CONTAINS_DOT(10420, "Table or database name may not contain
dot(.) character", true),
+ OBJECTNAME_CONTAINS_DOT(10420, "Catalog or table or database name may not
contain dot(.) character", true),
Review Comment:
Fixed in
https://github.com/apache/hive/pull/6088/commits/00a75f9fe1d13cc9e49a00a3cd2b598dadef1f0b
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
deniskuzZ commented on PR #6088: URL: https://github.com/apache/hive/pull/6088#issuecomment-3580620476 > The biggest flaw is we cannot have two same db name/table name among catalogs I don't follow this: FederatedMetastoreClient should be able to resolve proper Catalog to connect to in order to fetch/update db/table metadata. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
dengzhhu653 commented on PR #6088: URL: https://github.com/apache/hive/pull/6088#issuecomment-3489230252 The biggest flaw is we cannot have two same db name/table name among catalogs, this might be a question in the future. For cross-catalog queries e.g, db1.name1 join db1.name1 in this case, CatalogAwarenessMetastoreClient mightn't be able to figure out whether the all db1.name1 tables belong to the same catalog or not. I don't have an elegant solution yet -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
dengzhhu653 commented on PR #6088: URL: https://github.com/apache/hive/pull/6088#issuecomment-3489161909 Sorry for the late reply. Took a quick overview of the changes, looks we introduce a new syntax for the catalog, as well as the future items: >The code related to cat.db.tbl (part of the work in [HIVE-29177](https://issues.apache.org/jira/browse/HIVE-29177)) will also involve many table-related interfaces, which presents certain refactoring challenges. > let's further explore this issue when we work on cross-catalog queries. An already created ticket, [HIVE-29242](https://issues.apache.org/jira/browse/HIVE-29242), can be used to track this matter. It seems to me this is a significant/big change, and the user should follow a new way of catalog.db.table to request the table as the Trino/Presto does today. I think we can have a more simple idea, let's push the catalog awareness down into the Metastore client. For example, SET CATALOG testcat; -> change to a metaconf change, set metaconf:hive.metastore.current.catalog = testcat, default hive, this configuration will be propagated to HMS, every HMS API request will be appended with the `testcat`, both client and server can handle it. For cross-catalog queries, we can also do it in a similar way, we can introduce a new catalog awareness lawyer between session client and the thrift client, as SessionHiveMetaStoreClient -> CatalogAwarenessMetastoreClient -> ThriftHiveMetaStoreClient. The CatalogAwarenessMetastoreClient can acknowledge of the catalog through configuration or properties stored in HMS(HIVE-27186). Something as `hive.matastore.catalog.mapping.pattern`=`iceberg_catalog`:`ice_db1*,hive_db1,hive_db2.ice_tab*`;`hive`:`default,acid_db*`, the CatalogAwarenessMetastoreClient even can point to a third-party catalog partner. By this way, we don't need to introduce the change on the Hive SQL syntax and test it throughly, and other engines(such as Impala) can also benefit from this way to add the support on cross-catalog queries. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
zhangbutao commented on code in PR #6088: URL: https://github.com/apache/hive/pull/6088#discussion_r2465781286 ## ql/src/java/org/apache/hadoop/hive/ql/exec/repl/incremental/IncrementalLoadTasksBuilder.java: ## @@ -264,8 +265,9 @@ private Task dbUpdateReplStateTask(String dbName, String replState, Task p HashMap mapProp = new HashMap<>(); mapProp.put(ReplicationSpec.KEY.CURR_STATE_ID_SOURCE.toString(), replState); -AlterDatabaseSetPropertiesDesc alterDbDesc = new AlterDatabaseSetPropertiesDesc(dbName, mapProp, -new ReplicationSpec(replState, replState)); +// TODO catalog. Need to double check the actual catalog here. Depend on HIVE-29278 +AlterDatabaseSetPropertiesDesc alterDbDesc = new AlterDatabaseSetPropertiesDesc(HiveUtils.getCurrentCatalogOrDefault(conf), Review Comment: HIVE-29278 is used to track this matter. I think we can make the rep work with multiple native hms catalog. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
zhangbutao commented on code in PR #6088:
URL: https://github.com/apache/hive/pull/6088#discussion_r2465770546
##
ql/src/java/org/apache/hadoop/hive/ql/exec/repl/ReplLoadTask.java:
##
@@ -448,8 +449,9 @@ private void setDbReadOnly() {
Map props = new HashMap<>();
props.put(READONLY, Boolean.TRUE.toString());
+// TODO catalog. Need to double check the actual catalog here. Depend on
HIVE-29278
Review Comment:
You are right. Some task can only do in NATIVE DBs. Like this ReploadTask.
This TODO is added to verify if the correct native catalog name is obtained.
BTW, besides the default `hive ` native catalog, we still need to support
other multiple native catalogs.
As for external catalogs, we need to skip those tasks that are exclusive to
native catalogs. However, the specific types of external catalogs (e.g., JDBC,
REST) have not yet been defined, and this part of the work can be refined and
completed subsequently. HIVE-29243 can be used to track this matter.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
deniskuzZ commented on code in PR #6088:
URL: https://github.com/apache/hive/pull/6088#discussion_r2465751862
##
ql/src/java/org/apache/hadoop/hive/ql/ddl/database/create/CreateDatabaseDesc.java:
##
@@ -45,13 +46,14 @@ public class CreateDatabaseDesc implements DDLDesc,
Serializable {
private final String remoteDbName;
private final Map dbProperties;
- public CreateDatabaseDesc(String databaseName, String comment, String
locationUri, String managedLocationUri,
+ public CreateDatabaseDesc(String catalogName, String databaseName, String
comment, String locationUri, String managedLocationUri,
boolean ifNotExists, Map dbProperties) {
-this(databaseName, comment, locationUri, managedLocationUri, ifNotExists,
dbProperties, "NATIVE", null, null);
+this(catalogName, databaseName, comment, locationUri, managedLocationUri,
ifNotExists, dbProperties, "NATIVE", null, null);
Review Comment:
should we introduce a flag like native/non-native similar to tables?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
zhangbutao commented on code in PR #6088:
URL: https://github.com/apache/hive/pull/6088#discussion_r2465721095
##
ql/src/java/org/apache/hadoop/hive/ql/ddl/table/lock/show/ShowLocksOperation.java:
##
@@ -160,6 +160,8 @@ private ShowLocksResponse
getLocksForNewFormat(HiveLockManager lockMgr) throws H
throw new HiveException("New lock format only supported with db lock
manager.");
}
+// TODO catalog. Need to add catalog into ShowLocksRequest. But
ShowLocksRequest doesn't have catalog field.
Review Comment:
Replied in https://github.com/apache/hive/pull/6088#discussion_r2465716095
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
zhangbutao commented on code in PR #6088:
URL: https://github.com/apache/hive/pull/6088#discussion_r2465716095
##
ql/src/java/org/apache/hadoop/hive/ql/ddl/database/unlock/UnlockDatabaseAnalyzer.java:
##
@@ -41,14 +43,19 @@ public UnlockDatabaseAnalyzer(QueryState queryState) throws
SemanticException {
@Override
public void analyzeInternal(ASTNode root) throws SemanticException {
-String databaseName = unescapeIdentifier(root.getChild(0).getText());
+Pair catDbNamePair = getCatDbNamePair((ASTNode)
root.getChild(0));
+String catalogName = catDbNamePair.getLeft();
+if (catalogName != null && getCatalog(catalogName) == null) {
+ throw new SemanticException(ErrorMsg.CATALOG_NOT_EXISTS, catalogName);
+}
+String databaseName = catDbNamePair.getRight();
-inputs.add(new ReadEntity(getDatabase(databaseName)));
+inputs.add(new ReadEntity(getDatabase(catalogName, databaseName, true)));
// Unlock database operation is to release the lock explicitly, the
operation itself don't need to be locked.
// Set the WriteEntity as WriteType: DDL_NO_LOCK here, otherwise it will
conflict with Hive's transaction.
-outputs.add(new WriteEntity(getDatabase(databaseName),
WriteType.DDL_NO_LOCK));
+outputs.add(new WriteEntity(getDatabase(catalogName, databaseName, true),
WriteType.DDL_NO_LOCK));
Review Comment:
> HIVE_LOCKS table doesn't have catalog column.
Currently, this only retrieves a specific database under a particular
catalog and does not involve any lock/unlock operations on the catalog.
However, I believe future development should consider implementing
lock/unlock operations for catalogs, though I haven't yet identified the
specific use cases. It might be necessary during cross-catalog federated
queries.
> Maybe some operation like Unlock/Lock DB, ShowDbLocks should only be
supported by NATIVE DBs?
To be honest, I haven't fully thought this through. Locking/unlocking
databases in external catalogs (such as a MySQL JDBC catalog) doesn't seem very
meaningful, as these lock operations won't take effect on the MySQL source
side. However, I believe allowing locks on databases in external catalogs in
Hive might have some significance, particularly in scenarios involving
cross-catalog and cross-database operations, to ensure task transactional
consistency. Therefore, let's further explore this issue when we work on
cross-catalog queries. An already created ticket, HIVE-29242, can be used to
track this matter.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
zhangbutao commented on code in PR #6088:
URL: https://github.com/apache/hive/pull/6088#discussion_r2465632832
##
ql/src/java/org/apache/hadoop/hive/ql/ddl/database/create/CreateDatabaseDesc.java:
##
@@ -45,13 +46,14 @@ public class CreateDatabaseDesc implements DDLDesc,
Serializable {
private final String remoteDbName;
private final Map dbProperties;
- public CreateDatabaseDesc(String databaseName, String comment, String
locationUri, String managedLocationUri,
+ public CreateDatabaseDesc(String catalogName, String databaseName, String
comment, String locationUri, String managedLocationUri,
boolean ifNotExists, Map dbProperties) {
-this(databaseName, comment, locationUri, managedLocationUri, ifNotExists,
dbProperties, "NATIVE", null, null);
+this(catalogName, databaseName, comment, locationUri, managedLocationUri,
ifNotExists, dbProperties, "NATIVE", null, null);
Review Comment:
> for the external catalogs dbtype should be "REMOTE", right?
I don't think so. `REMOTE ` dbtyp was introduced by HIVE-24396 data
connector. If we implement all tasks about federated catalog, **we can
deprecate data connector**. Because the data connector can only map external
databases to internal databases by manually creating remote databases one by
one to query external data sources, like this:
`CREATE REMOTE DATABASE db_map_mysql USING mysqlconnector with
DBPROPERTIES("connector.remoteDbName"="testmysqldb");`
whereas the federated catalog can retrieve all external databases at once
without the need to manually create remote databases, once we create a catalog.
> It seems that we always hardcode "NATIVE" and below condition never
returns true
If you create a remote database with a specified dataconnector, this
condition will return true.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
zhangbutao commented on code in PR #6088:
URL: https://github.com/apache/hive/pull/6088#discussion_r2465632832
##
ql/src/java/org/apache/hadoop/hive/ql/ddl/database/create/CreateDatabaseDesc.java:
##
@@ -45,13 +46,14 @@ public class CreateDatabaseDesc implements DDLDesc,
Serializable {
private final String remoteDbName;
private final Map dbProperties;
- public CreateDatabaseDesc(String databaseName, String comment, String
locationUri, String managedLocationUri,
+ public CreateDatabaseDesc(String catalogName, String databaseName, String
comment, String locationUri, String managedLocationUri,
boolean ifNotExists, Map dbProperties) {
-this(databaseName, comment, locationUri, managedLocationUri, ifNotExists,
dbProperties, "NATIVE", null, null);
+this(catalogName, databaseName, comment, locationUri, managedLocationUri,
ifNotExists, dbProperties, "NATIVE", null, null);
Review Comment:
> for the external catalogs dbtype should be "REMOTE", right?
I don't think so. `REMOTE ` dbtyp was introduced by HIVE-24396 data
connector. If we implement all tasks about federated catalog, **we can
deprecate data connector**. Because the data connector can only map external
databases to internal databases by manually creating remote databases one by
one to query external data sources, like this:
`CREATE REMOTE DATABASE db_map_mysql USING mysqlconnector with
DBPROPERTIES("connector.remoteDbName"="testmysqldb");`
whereas the federated catalog can retrieve all external databases at once
without the need to manually create remote databases, once we create a catalog.
`It seems that we always hardcode "NATIVE" and below condition never returns
true`
If you create a remote database with a specified dataconnector, this
condition will return true.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
harshal-16 commented on code in PR #6088: URL: https://github.com/apache/hive/pull/6088#discussion_r2459669634 ## ql/src/java/org/apache/hadoop/hive/ql/exec/repl/incremental/IncrementalLoadTasksBuilder.java: ## @@ -264,8 +265,9 @@ private Task dbUpdateReplStateTask(String dbName, String replState, Task p HashMap mapProp = new HashMap<>(); mapProp.put(ReplicationSpec.KEY.CURR_STATE_ID_SOURCE.toString(), replState); -AlterDatabaseSetPropertiesDesc alterDbDesc = new AlterDatabaseSetPropertiesDesc(dbName, mapProp, -new ReplicationSpec(replState, replState)); +// TODO catalog. Need to double check the actual catalog here. Depend on HIVE-29278 +AlterDatabaseSetPropertiesDesc alterDbDesc = new AlterDatabaseSetPropertiesDesc(HiveUtils.getCurrentCatalogOrDefault(conf), Review Comment: Oh, then I think we will need to update the incremental dump filter also to make it consistent with the load. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
deniskuzZ commented on code in PR #6088: URL: https://github.com/apache/hive/pull/6088#discussion_r2459539538 ## ql/src/java/org/apache/hadoop/hive/ql/exec/repl/incremental/IncrementalLoadTasksBuilder.java: ## @@ -264,8 +265,9 @@ private Task dbUpdateReplStateTask(String dbName, String replState, Task p HashMap mapProp = new HashMap<>(); mapProp.put(ReplicationSpec.KEY.CURR_STATE_ID_SOURCE.toString(), replState); -AlterDatabaseSetPropertiesDesc alterDbDesc = new AlterDatabaseSetPropertiesDesc(dbName, mapProp, -new ReplicationSpec(replState, replState)); +// TODO catalog. Need to double check the actual catalog here. Depend on HIVE-29278 +AlterDatabaseSetPropertiesDesc alterDbDesc = new AlterDatabaseSetPropertiesDesc(HiveUtils.getCurrentCatalogOrDefault(conf), Review Comment: thanks for checking, @harshal-16 ! what if we register multiple external HMS catalogs? -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
harshal-16 commented on code in PR #6088: URL: https://github.com/apache/hive/pull/6088#discussion_r2458946185 ## ql/src/java/org/apache/hadoop/hive/ql/exec/repl/incremental/IncrementalLoadTasksBuilder.java: ## @@ -264,8 +265,9 @@ private Task dbUpdateReplStateTask(String dbName, String replState, Task p HashMap mapProp = new HashMap<>(); mapProp.put(ReplicationSpec.KEY.CURR_STATE_ID_SOURCE.toString(), replState); -AlterDatabaseSetPropertiesDesc alterDbDesc = new AlterDatabaseSetPropertiesDesc(dbName, mapProp, -new ReplicationSpec(replState, replState)); +// TODO catalog. Need to double check the actual catalog here. Depend on HIVE-29278 +AlterDatabaseSetPropertiesDesc alterDbDesc = new AlterDatabaseSetPropertiesDesc(HiveUtils.getCurrentCatalogOrDefault(conf), Review Comment: Just checked the details in the main JIRA: https://issues.apache.org/jira/browse/HIVE-28879 As ACID tables can only reside in Hive (default), other catalogs don't make sense in case of replication here Also, during dump operation itself, it filters only default catalog here: https://github.com/apache/hive/blob/d2bd35394d82ef5dc778e3c2c193288e29e63187/ql/src/java/org/apache/hadoop/hive/ql/exec/repl/ReplDumpTask.java#L923 -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
deniskuzZ commented on code in PR #6088: URL: https://github.com/apache/hive/pull/6088#discussion_r2454629225 ## ql/src/java/org/apache/hadoop/hive/ql/lockmgr/HiveTxnManagerImpl.java: ## @@ -162,7 +167,8 @@ public int lockDatabase(Hive hiveDB, LockDatabaseDesc lockDb) throws HiveExcepti String.valueOf(System.currentTimeMillis()), "EXPLICIT", lockDb.getQueryStr(), conf); -HiveLock lck = lockMgr.lock(new HiveLockObject(dbObj.getName(), lockData), mode, true); +// Using the catalogName@databaseName format to uniquely identify a database. +HiveLock lck = lockMgr.lock(new HiveLockObject(catName + "@" +dbObj.getName(), lockData), mode, true); Review Comment: oh, nice hack! however, maybe we should use "." ? I think it might actually work. i am not sure if `show locks` would handle this change, might need some tweaks: show locks db1 show locks cat1.db1 note, might be problematic if CU has multiple HMS versions on a cluster nit: please add space `+ dbObj.getName()` -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
deniskuzZ commented on code in PR #6088: URL: https://github.com/apache/hive/pull/6088#discussion_r2454629225 ## ql/src/java/org/apache/hadoop/hive/ql/lockmgr/HiveTxnManagerImpl.java: ## @@ -162,7 +167,8 @@ public int lockDatabase(Hive hiveDB, LockDatabaseDesc lockDb) throws HiveExcepti String.valueOf(System.currentTimeMillis()), "EXPLICIT", lockDb.getQueryStr(), conf); -HiveLock lck = lockMgr.lock(new HiveLockObject(dbObj.getName(), lockData), mode, true); +// Using the catalogName@databaseName format to uniquely identify a database. +HiveLock lck = lockMgr.lock(new HiveLockObject(catName + "@" +dbObj.getName(), lockData), mode, true); Review Comment: oh, nice hack! however, maybe we should use "." ? note, might be problematic if CU has multiple HMS versions on a cluster I think it might actually work. i am not sure if `show locks` would handle this change, might need some tweaks: show locks db1 show locks cat1.db1 nit: please add space `+ dbObj.getName()` -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
deniskuzZ commented on code in PR #6088: URL: https://github.com/apache/hive/pull/6088#discussion_r2454716065 ## ql/src/test/queries/clientpositive/catalog.q: ## @@ -45,3 +45,6 @@ SHOW CATALOGS LIKE 'test__'; -- ALTER LOCATION ALTER CATALOG test_cat SET LOCATION '/tmp/test_cat_new'; DESC CATALOG EXTENDED test_cat; + +-- DROP catalog at the end +DROP CATALOG test_cat; Review Comment: redundant space -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
deniskuzZ commented on code in PR #6088: URL: https://github.com/apache/hive/pull/6088#discussion_r2454714458 ## ql/src/test/queries/clientnegative/lockneg_try_lock_cat_db_in_use.q: ## @@ -0,0 +1,9 @@ +set hive.lock.numretries=0; +set hive.support.concurrency=true; Review Comment: that is only needed for ACID tables when DBTxnManager is used -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
deniskuzZ commented on code in PR #6088:
URL: https://github.com/apache/hive/pull/6088#discussion_r2454710971
##
ql/src/java/org/apache/hadoop/hive/ql/processors/HiveCommand.java:
##
@@ -86,6 +86,8 @@ public static HiveCommand find(String[] command, boolean
findOnlyForTesting) {
} else if (command.length > 1 && "show".equalsIgnoreCase(command[0]) &&
"processlist".equalsIgnoreCase(command[1])) {
return PROCESSLIST;
+ } else if(command.length > 1 && "set".equalsIgnoreCase(command[0]) &&
"catalog".equalsIgnoreCase(command[1])) {
Review Comment:
space
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
deniskuzZ commented on PR #6088: URL: https://github.com/apache/hive/pull/6088#issuecomment-3436309016 thanks @zhangbutao! in general LGTM, added some comments -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
deniskuzZ commented on code in PR #6088: URL: https://github.com/apache/hive/pull/6088#discussion_r2454708493 ## ql/src/java/org/apache/hadoop/hive/ql/parse/repl/load/message/CreateDatabaseHandler.java: ## @@ -56,22 +56,23 @@ public List> handle(Context context) Database db = metaData.getDatabase(); String destinationDBName = context.dbName == null ? db.getName() : context.dbName; +String destinationCatalogName = db.getCatalogName(); // TODO catalog. Need to double check the catalog here. Depend on HIVE-29278 Review Comment: cc @harshal-16 -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
deniskuzZ commented on code in PR #6088:
URL: https://github.com/apache/hive/pull/6088#discussion_r2454701996
##
ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java:
##
@@ -1926,6 +1960,19 @@ protected Database getDatabase(String dbName, boolean
throwException) throws Sem
return database;
}
+ protected Database getDatabase(String catalogName, String dbName, boolean
throwException) throws SemanticException {
Review Comment:
try reuse code. you could call new method within the old one without
catalogName
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
deniskuzZ commented on code in PR #6088:
URL: https://github.com/apache/hive/pull/6088#discussion_r2454698626
##
ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java:
##
@@ -421,6 +421,33 @@ public static String charSetString(String charSetName,
String charSetString)
}
}
+ /**
+ *
+ * @param dbNameNode A root node that contains database fields
+ * @return Return a Pair object which includes catalogName and dbName
+ * @throws SemanticException
+ */
+ public static Pair getCatDbNamePair(ASTNode dbNameNode)
throws SemanticException {
+String catName = null;
+String dbName;
+
+if (dbNameNode.getChildCount() == 2) {
+ catName = unescapeIdentifier(dbNameNode.getChild(0).getText());
Review Comment:
why not apply unescapeIdentifier after the init block?
##
ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java:
##
@@ -421,6 +421,33 @@ public static String charSetString(String charSetName,
String charSetString)
}
}
+ /**
+ *
+ * @param dbNameNode A root node that contains database fields
+ * @return Return a Pair object which includes catalogName and dbName
+ * @throws SemanticException
+ */
+ public static Pair getCatDbNamePair(ASTNode dbNameNode)
throws SemanticException {
+String catName = null;
+String dbName;
+
+if (dbNameNode.getChildCount() == 2) {
+ catName = unescapeIdentifier(dbNameNode.getChild(0).getText());
Review Comment:
could we apply unescapeIdentifier after the init block?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
deniskuzZ commented on code in PR #6088:
URL: https://github.com/apache/hive/pull/6088#discussion_r2454698626
##
ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java:
##
@@ -421,6 +421,33 @@ public static String charSetString(String charSetName,
String charSetString)
}
}
+ /**
+ *
+ * @param dbNameNode A root node that contains database fields
+ * @return Return a Pair object which includes catalogName and dbName
+ * @throws SemanticException
+ */
+ public static Pair getCatDbNamePair(ASTNode dbNameNode)
throws SemanticException {
+String catName = null;
+String dbName;
+
+if (dbNameNode.getChildCount() == 2) {
+ catName = unescapeIdentifier(dbNameNode.getChild(0).getText());
Review Comment:
could we apply unescapeIdentifier after the init block?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
deniskuzZ commented on code in PR #6088:
URL: https://github.com/apache/hive/pull/6088#discussion_r2454693050
##
ql/src/java/org/apache/hadoop/hive/ql/metadata/Table.java:
##
@@ -1198,6 +1198,10 @@ public String getCatalogName() {
return this.tTable.getCatName();
}
+ public void setCatalogName(String catalogName) {
Review Comment:
do we need this setter on a Table object?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
deniskuzZ commented on code in PR #6088:
URL: https://github.com/apache/hive/pull/6088#discussion_r2454690316
##
ql/src/java/org/apache/hadoop/hive/ql/metadata/HiveUtils.java:
##
@@ -543,4 +546,10 @@ public static String getLowerCaseTableName(String refName)
{
}
return refName.toLowerCase();
}
+
+ public static String getCurrentCatalogOrDefault(Configuration conf) {
+return SessionState.get() != null ?
Review Comment:
you could try Optional.ofNullable
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
deniskuzZ commented on code in PR #6088:
URL: https://github.com/apache/hive/pull/6088#discussion_r2454687483
##
ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java:
##
@@ -6463,6 +6564,18 @@ public List getFunctionsInDb(String dbName,
String pattern) throws Hiv
}
}
+ public List getFunctionsInDb(String catName, String dbName, String
pattern) throws HiveException {
Review Comment:
same as https://github.com/apache/hive/pull/6088/files#r2454681994
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
deniskuzZ commented on code in PR #6088: URL: https://github.com/apache/hive/pull/6088#discussion_r2454681994 ## ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java: ## @@ -1997,6 +2035,52 @@ public List getTablesByType(String dbName, String pattern, TableType typ } } + /** + * Returns all existing tables of a type (VIRTUAL_VIEW|EXTERNAL_TABLE|MANAGED_TABLE) from the specified + * database which match the given pattern. The matching occurs as per Java regular expressions. + * @param catName catalog name to find the tables in. if null, uses the current catalog in this session. + * @param dbName Database name to find the tables in. if null, uses the current database in this session. + * @param pattern A pattern to match for the table names.If null, returns all names from this DB. + * @param type The type of tables to return. VIRTUAL_VIEWS for views. If null, returns all tables and views. + * @return list of table names that match the pattern. + * @throws HiveException + */ + public List getTablesByType(String catName, String dbName, String pattern, TableType type) Review Comment: same as https://github.com/apache/hive/pull/6088/files#r2454680968 can we reuse it in getTablesByType(String dbName, String pattern, TableType type) ? -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
deniskuzZ commented on code in PR #6088: URL: https://github.com/apache/hive/pull/6088#discussion_r2454681994 ## ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java: ## @@ -1997,6 +2035,52 @@ public List getTablesByType(String dbName, String pattern, TableType typ } } + /** + * Returns all existing tables of a type (VIRTUAL_VIEW|EXTERNAL_TABLE|MANAGED_TABLE) from the specified + * database which match the given pattern. The matching occurs as per Java regular expressions. + * @param catName catalog name to find the tables in. if null, uses the current catalog in this session. + * @param dbName Database name to find the tables in. if null, uses the current database in this session. + * @param pattern A pattern to match for the table names.If null, returns all names from this DB. + * @param type The type of tables to return. VIRTUAL_VIEWS for views. If null, returns all tables and views. + * @return list of table names that match the pattern. + * @throws HiveException + */ + public List getTablesByType(String catName, String dbName, String pattern, TableType type) Review Comment: same as https://github.com/apache/hive/pull/6088/files#r2454680968 -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
deniskuzZ commented on code in PR #6088:
URL: https://github.com/apache/hive/pull/6088#discussion_r2454680968
##
ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java:
##
@@ -1914,6 +1942,16 @@ public Table
apply(org.apache.hadoop.hive.metastore.api.Table table) {
}
}
+ public List getTableObjects(String catName, String dbName, String
pattern, TableType tableType) throws HiveException {
Review Comment:
@dengzhhu653 is it ok or we should use some request object like Database?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
deniskuzZ commented on code in PR #6088: URL: https://github.com/apache/hive/pull/6088#discussion_r2454667910 ## ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java: ## @@ -1297,7 +1303,12 @@ public void renamePartition(Table tbl, Map oldPartSpec, Partitio } } - // TODO: this whole path won't work with catalogs + /** + * When you call this method, you need to ensure that the catalog has been set in the db object. Review Comment: why not have fallback similar to https://github.com/apache/hive/pull/6088/files#diff-9d6bc2f1a2972d844b3107f29bc1731af7adf6f4c922e08904345c5ae88cd727R1443 -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
deniskuzZ commented on code in PR #6088: URL: https://github.com/apache/hive/pull/6088#discussion_r2454657125 ## ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java: ## @@ -721,9 +726,10 @@ public void dropDatabase(String name, boolean deleteData, boolean ignoreUnknownD * @throws HiveException * @throws NoSuchObjectException */ + @Deprecated Review Comment: please add to java doc "@deprecated since 4.1.0, will be removed in 5.0.0" -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
deniskuzZ commented on code in PR #6088: URL: https://github.com/apache/hive/pull/6088#discussion_r2454657125 ## ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java: ## @@ -721,9 +726,10 @@ public void dropDatabase(String name, boolean deleteData, boolean ignoreUnknownD * @throws HiveException * @throws NoSuchObjectException */ + @Deprecated Review Comment: @deprecated since 4.2.0, will be removed in 5.0.0, ## ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java: ## @@ -721,9 +726,10 @@ public void dropDatabase(String name, boolean deleteData, boolean ignoreUnknownD * @throws HiveException * @throws NoSuchObjectException */ + @Deprecated Review Comment: @deprecated since 4.1.0, will be removed in 5.0.0, -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] HIVE-29177: Implement default Catalog selection [hive]
deniskuzZ commented on code in PR #6088: URL: https://github.com/apache/hive/pull/6088#discussion_r2454657125 ## ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java: ## @@ -721,9 +726,10 @@ public void dropDatabase(String name, boolean deleteData, boolean ignoreUnknownD * @throws HiveException * @throws NoSuchObjectException */ + @Deprecated Review Comment: @deprecated since 4.0.1, will be removed in 5.0.0, -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
