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:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
