[ 
https://issues.apache.org/jira/browse/HIVE-24970?focusedWorklogId=611103&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-611103
 ]

ASF GitHub Bot logged work on HIVE-24970:
-----------------------------------------

                Author: ASF GitHub Bot
            Created on: 15/Jun/21 02:18
            Start Date: 15/Jun/21 02:18
    Worklog Time Spent: 10m 
      Work Description: nrg4878 commented on a change in pull request #2389:
URL: https://github.com/apache/hive/pull/2389#discussion_r651208929



##########
File path: 
ql/src/java/org/apache/hadoop/hive/ql/ddl/database/create/CreateDatabaseAnalyzer.java
##########
@@ -78,10 +78,17 @@ public void analyzeInternal(ASTNode root) throws 
SemanticException {
         ASTNode nextNode = (ASTNode) root.getChild(i);
         connectorName = ((ASTNode)nextNode).getChild(0).getText();
         outputs.add(toWriteEntity(connectorName));
-        if (managedLocationUri != null) {
-          outputs.remove(toWriteEntity(managedLocationUri));
-          managedLocationUri = null;
+
+        // HIVE-2436: Reject location and managed locations in DDL for REMOTE 
databases.

Review comment:
       Is this jira reference correct? seems like old jira.

##########
File path: ql/src/test/queries/clientnegative/remoteDB_locationUri_reject.q
##########
@@ -0,0 +1,12 @@
+-- CREATE IF NOT EXISTS already
+CREATE CONNECTOR IF NOT EXISTS mysql_test
+TYPE 'mysql'
+URL 'jdbc:mysql://nightly1.apache.org:3306/hive1'
+COMMENT 'test connector'
+WITH DCPROPERTIES (
+"hive.sql.dbcp.username"="hive1",
+"hive.sql.dbcp.password"="hive1");
+SHOW CONNECTORS;
+
+-- reject location and managedlocation config in remote database
+create REMOTE database mysql_rej location '/tmp/rej1.db' managedlocation 
'/tmp/rej2.db' using mysql_test with 
DBPROPERTIES("connector.remoteDbName"="hive1");

Review comment:
       can we split this into 2 (or even 3) scenarios? we want to make sure if 
fails in all scenarios.
   create REMOTE db with location
   create REMOTE db with managedlocation
   create REMOTE db with location and managedlocation 
   

##########
File path: 
ql/src/java/org/apache/hadoop/hive/ql/ddl/database/create/CreateDatabaseAnalyzer.java
##########
@@ -78,10 +78,17 @@ public void analyzeInternal(ASTNode root) throws 
SemanticException {
         ASTNode nextNode = (ASTNode) root.getChild(i);
         connectorName = ((ASTNode)nextNode).getChild(0).getText();
         outputs.add(toWriteEntity(connectorName));
-        if (managedLocationUri != null) {
-          outputs.remove(toWriteEntity(managedLocationUri));
-          managedLocationUri = null;
+
+        // HIVE-2436: Reject location and managed locations in DDL for REMOTE 
databases.
+        if (locationUri != null || managedLocationUri != null ) {
+          if (locationUri == null) {
+            outputs.remove(toWriteEntity(locationUri));

Review comment:
       so in HMS schema, "locationUri" is a required column. So I think 
HiveServer2 may have to send a location for a database, regardless of its type. 
The HiveMetastore might also use a default one. So a couple of things here
   1) Seems like line 85 and 87 might result in a NullPointerException. 
Shouldn't we remove it when the value is NOT null?
   2) given we are throwing an exception from this condition, do we even have 
to do outputs.remove() ? We are failing the operation anyway.

##########
File path: parser/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g
##########
@@ -1156,7 +1156,7 @@ createDatabaseStatement
         dbManagedLocation?
         dbConnectorName?
         (KW_WITH KW_DBPROPERTIES dbprops=dbProperties)?
-    -> {$remote != null}? ^(TOK_CREATEDATABASE $name ifNotExists? 
databaseComment? $dbprops? dbConnectorName?)
+    -> {$remote != null}? ^(TOK_CREATEDATABASE $name ifNotExists? dbLocation? 
dbManagedLocation? databaseComment? $dbprops? dbConnectorName?)

Review comment:
       Seems like the grammer shouldn't support the accepting a location and 
managedlocation when the dbtype is REMOTE. I am bit surprised that the antlr 
parser wasn't throwing an exception when location or managedlocation was 
specified.
   Could you please see why we were not seeing an exception with the old 
grammer?




-- 
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.

For queries about this service, please contact Infrastructure at:
[email protected]


Issue Time Tracking
-------------------

            Worklog Id:     (was: 611103)
    Remaining Estimate: 0h
            Time Spent: 10m

> Reject location and managed locations in DDL for REMOTE databases.
> ------------------------------------------------------------------
>
>                 Key: HIVE-24970
>                 URL: https://issues.apache.org/jira/browse/HIVE-24970
>             Project: Hive
>          Issue Type: Sub-task
>          Components: HiveServer2
>    Affects Versions: 4.0.0
>            Reporter: Naveen Gangam
>            Assignee: Dantong Dong
>            Priority: Major
>              Labels: pull-request-available
>          Time Spent: 10m
>  Remaining Estimate: 0h
>
> This was part of the review feedback from Yongzhi. Creating a followup jira 
> to track this discussion.
> So, using DB connector for DB, will not create managed tables?
>  
> @nrg4878 nrg4878 1 hour ago Author Member
> we don't support create/drop/alter in REMOTE databases at this point. the 
> concepts of managed vs external is not in the picture at this point. When we 
> do support it, it will be application to the hive connectors only (or other 
> hive based connectors like AWS Glue)
>  
> @nrg4878 nrg4878 2 minutes ago Author Member
> will file a separate jira for this. Basically, instead of ignoring the 
> location and managedlocation that may be specified for remote database, the 
> grammer needs to not accept any locations in the DDL at all.
> The argument is fair, why accept something we do not honor or entirely 
> irrelevant for such databases. However, this requires some thought when we 
> have additional connectors for remote hive instances. It might have some 
> relevance in terms of security with Ranger etc.
> So will create new jira for followup discussion.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Reply via email to