[
https://issues.apache.org/jira/browse/HIVE-24396?focusedWorklogId=571788&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-571788
]
ASF GitHub Bot logged work on HIVE-24396:
-----------------------------------------
Author: ASF GitHub Bot
Created on: 25/Mar/21 10:21
Start Date: 25/Mar/21 10:21
Worklog Time Spent: 10m
Work Description: vnhive commented on a change in pull request #2037:
URL: https://github.com/apache/hive/pull/2037#discussion_r601159117
##########
File path:
ql/src/java/org/apache/hadoop/hive/ql/ddl/dataconnector/create/CreateDataConnectorOperation.java
##########
@@ -0,0 +1,71 @@
+/*
Review comment:
I tried creating a connector without the mysql JDBC URL specified
properly and it went through,
please see below,
CREATE CONNECTOR mysql_test_2
TYPE 'mysql'
URL 'jdbc://'
COMMENT 'test connector'
WITH DCPROPERTIES (
"hive.sql.dbcp.username"="hive1",
"hive.sql.dbcp.password"="hive1");
CREATE CONNECTOR mysql_test_3
TYPE 'mysql'
URL 'jdbc:derby://nightly1.apache.org:3306/hive1'
COMMENT 'test connector'
WITH DCPROPERTIES (
"hive.sql.dbcp.username"="hive1",
"hive.sql.dbcp.password"="hive1");
- I am not saying they are wrong, but we should probably call this out in
the documentation. Document that URLs are not verified.
- Another thing I noticed is that the password is displayed in plain
text on the command line. This used be considered a security problem
in a product I worked in a past life. But I notice that an external
table can be created with this semantics. I guess it is acceptable
here.
- It is also stored in plain text in the metastore, please see below,
CREATE TABLE `DATACONNECTOR_PARAMS` (
`NAME` VARCHAR(128) NOT NULL,
`PARAM_KEY` VARCHAR(180) NOT NULL,
`PARAM_VALUE` VARCHAR(4000),
PRIMARY KEY (`NAME`, `PARAM_KEY`),
CONSTRAINT `DATACONNECTOR_NAME_FK1` FOREIGN KEY (`NAME`) REFERENCES
`DATACONNECTORS` (`NAME`) ON DELETE CASCADE
) ENGINE=InnoDB DEFAULT CHARSET=latin1;
Again I am not saying this is a problem, but I thought I can call this out
to you.
##########
File path: ql/src/test/queries/clientpositive/dataconnector.q
##########
@@ -0,0 +1,71 @@
+-- SORT_QUERY_RESULTS
+SHOW CONNECTORS;
+
+-- CREATE with comment
+CREATE CONNECTOR 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;
+
+-- CREATE INE already exists
+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;
+
+-- CREATE INE already exists
Review comment:
Typo INE -> IF
I think it should be IF NOT
##########
File path: ql/src/test/queries/clientpositive/dataconnector.q
##########
@@ -0,0 +1,71 @@
+-- SORT_QUERY_RESULTS
+SHOW CONNECTORS;
+
+-- CREATE with comment
+CREATE CONNECTOR 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;
+
+-- CREATE INE already exists
Review comment:
Typo INE -> IF
I think it should be IF NOT
##########
File path: ql/src/test/queries/clientpositive/dataconnector.q
##########
@@ -0,0 +1,71 @@
+-- SORT_QUERY_RESULTS
+SHOW CONNECTORS;
+
+-- CREATE with comment
+CREATE CONNECTOR 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;
+
+-- CREATE INE already exists
+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;
+
+-- CREATE INE already exists
+CREATE CONNECTOR IF NOT EXISTS derby_test
+TYPE 'derby'
+URL 'jdbc:derby:./target/tmp/junit_metastore_db;create=true'
+COMMENT 'test derby connector'
+WITH DCPROPERTIES (
+"hive.sql.dbcp.username"="APP",
+"hive.sql.dbcp.password"="mine");
+
+-- DROP
+DROP CONNECTOR mysql_test;
+SHOW CONNECTORS;
+
+-- DROP IE exists
+DROP CONNECTOR IF EXISTS mysql_test;
+SHOW CONNECTORS;
+
+-- recreate with same name
+CREATE CONNECTOR 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;
+
+CREATE REMOTE DATABASE db_derby USING derby_test with
DBPROPERTIES("connector.remoteDbName"="APP");
Review comment:
Shouldn't the remoteDbName be junit_metastore_db ? I checked the result
file for show tables; it is just empty. Shouldn't it have thrown up that the
database does not exist? I hope I am not reading it wrong.
This is one place where I believe not verifying that the database exists is
not right.
Food for Thought.
##########
File path: ql/src/test/queries/clientpositive/dataconnector.q
##########
@@ -0,0 +1,71 @@
+-- SORT_QUERY_RESULTS
+SHOW CONNECTORS;
+
+-- CREATE with comment
+CREATE CONNECTOR 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;
+
+-- CREATE INE already exists
+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;
+
+-- CREATE INE already exists
+CREATE CONNECTOR IF NOT EXISTS derby_test
+TYPE 'derby'
+URL 'jdbc:derby:./target/tmp/junit_metastore_db;create=true'
+COMMENT 'test derby connector'
+WITH DCPROPERTIES (
+"hive.sql.dbcp.username"="APP",
+"hive.sql.dbcp.password"="mine");
+
+-- DROP
+DROP CONNECTOR mysql_test;
+SHOW CONNECTORS;
+
+-- DROP IE exists
Review comment:
Typo INE -> IF
--
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:
us...@infra.apache.org
Issue Time Tracking
-------------------
Worklog Id: (was: 571788)
Time Spent: 3h 50m (was: 3h 40m)
> [New Feature] Add data connector support for remote datasources
> ---------------------------------------------------------------
>
> Key: HIVE-24396
> URL: https://issues.apache.org/jira/browse/HIVE-24396
> Project: Hive
> Issue Type: Improvement
> Components: Hive
> Reporter: Naveen Gangam
> Assignee: Naveen Gangam
> Priority: Major
> Labels: pull-request-available
> Time Spent: 3h 50m
> Remaining Estimate: 0h
>
> This feature work is to be able to support in Hive Metastore to be able to
> configure data connectors for remote datasources and map databases. We
> currently have support for remote tables via StorageHandlers like
> JDBCStorageHandler and HBaseStorageHandler.
> Data connectors are a natural extension to this where we can map an entire
> database or catalogs instead of individual tables. The tables within are
> automagically mapped at runtime. The metadata for these tables are not
> persisted in Hive. They are always mapped and built at runtime.
> With this feature, we introduce a concept of type for Databases in Hive.
> NATIVE vs REMOTE. All current databases are NATIVE. To create a REMOTE
> database, the following syntax is to be used
> CREATE REMOTE DATABASE remote_db USING <dataconnector> WITH DCPROPERTIES
> (....);
> Will attach a design doc to this jira.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
