You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@hive.apache.org by GitBox <gi...@apache.org> on 2021/07/29 11:26:30 UTC

[GitHub] [hive] vnhive opened a new pull request #2546: [WIP] HIVE-25405 : Implement Connector Provider for Amazon Redshift

vnhive opened a new pull request #2546:
URL: https://github.com/apache/hive/pull/2546


   This PR proposes the addition of a data connector implementation to support amazon redshift.
   
   The data connector enables connecting to and seamlessly working with a redshift database.
   
   0: jdbc:hive2://> CREATE CONNECTOR IF NOT EXISTS redshift_test_7
   . . . . . . . . > TYPE 'redshift'
   . . . . . . . . > URL '<redshift-jdbc-url>'
   . . . . . . . . > COMMENT 'test redshift connector'
   . . . . . . . . > WITH DCPROPERTIES (
   . . . . . . . . > "hive.sql.dbcp.username"="******",
   . . . . . . . . > "hive.sql.dbcp.password"="******");
   No rows affected (0.015 seconds)
   
   0: jdbc:hive2://> CREATE REMOTE DATABASE db_sample_7 USING redshift_test_7 with DBPROPERTIES("connector.remoteDbName"="dbname");
   21/07/29 16:40:06 [HiveServer2-Background-Pool: Thread-217]: WARN exec.DDLTask: metastore.warehouse.external.dir is not set, falling back to metastore.warehouse.dir. This could cause external tables to use to managed tablespace.
   No rows affected (0.02 seconds)
   
   0: jdbc:hive2://> use db_sample_7;
   No rows affected (0.014 seconds)
   
   0: jdbc:hive2://> show tables;
   +-----------------+
   |    tab_name     |
   +-----------------+
   | accommodations  |
   | category        |
   | date            |
   | event           |
   | listing         |
   | sales           |
   | sample          |
   | test_time       |
   | test_time_2     |
   | test_timestamp  |
   | users           |
   | venue           |
   | zipcode         |
   +-----------------+
   13 rows selected (8.578 seconds)
   
   
   
   
   
   


-- 
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: gitbox-unsubscribe@hive.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] github-actions[bot] closed pull request #2546: HIVE-25405 : Implement Connector Provider for Amazon Redshift

Posted by GitBox <gi...@apache.org>.
github-actions[bot] closed pull request #2546:
URL: https://github.com/apache/hive/pull/2546


   


-- 
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: gitbox-unsubscribe@hive.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] github-actions[bot] commented on pull request #2546: HIVE-25405 : Implement Connector Provider for Amazon Redshift

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #2546:
URL: https://github.com/apache/hive/pull/2546#issuecomment-1013986428


   This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
   Feel free to reach out on the dev@hive.apache.org list if the patch is in need of reviews.


-- 
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: gitbox-unsubscribe@hive.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] github-actions[bot] closed pull request #2546: HIVE-25405 : Implement Connector Provider for Amazon Redshift

Posted by GitBox <gi...@apache.org>.
github-actions[bot] closed pull request #2546:
URL: https://github.com/apache/hive/pull/2546


   


-- 
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: gitbox-unsubscribe@hive.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] zabetak commented on a change in pull request #2546: HIVE-25405 : Implement Connector Provider for Amazon Redshift

Posted by GitBox <gi...@apache.org>.
zabetak commented on a change in pull request #2546:
URL: https://github.com/apache/hive/pull/2546#discussion_r751148777



##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/dataconnector/jdbc/RedshiftConnectorProvider.java
##########
@@ -0,0 +1,86 @@
+package org.apache.hadoop.hive.metastore.dataconnector.jdbc;
+
+import org.apache.hadoop.hive.metastore.ColumnType;
+import org.apache.hadoop.hive.metastore.api.DataConnector;
+import org.apache.hadoop.hive.metastore.api.MetaException;
+import org.apache.hadoop.hive.metastore.api.Table;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.sql.ResultSet;
+import java.sql.SQLException;
+
+public class RedshiftConnectorProvider extends AbstractJDBCConnectorProvider {
+    private static Logger LOG = LoggerFactory.getLogger(RedshiftConnectorProvider.class);
+
+    private static final String DRIVER_CLASS = "com.amazon.redshift.jdbc42.Driver".intern();
+
+    public RedshiftConnectorProvider(String dbName, DataConnector dataConn) {
+        super(dbName, dataConn, DRIVER_CLASS);
+    }
+
+    protected String getDataType(String dbDataType, int size) {
+        String mappedType = super.getDataType(dbDataType, size);
+
+        // The VOID type points to the corresponding datatype not existing in hive. These datatypes are datastore
+        // specific. They need special handling. An example would be the Geometric type that is not supported in Hive.
+        // The other cases where a datatype in redshift is resolved to a VOID type are during the use of aliases like
+        // float8, int8 etc. These can be mapped to existing hive types and are done below.
+        if (!mappedType.equalsIgnoreCase(ColumnType.VOID_TYPE_NAME)) {
+            return mappedType;
+        }
+
+        // map any db specific types here.

Review comment:
       The Intention is clear, comment is redundant.
   ```suggestion
   ```

##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/dataconnector/jdbc/RedshiftConnectorProvider.java
##########
@@ -0,0 +1,86 @@
+package org.apache.hadoop.hive.metastore.dataconnector.jdbc;
+
+import org.apache.hadoop.hive.metastore.ColumnType;
+import org.apache.hadoop.hive.metastore.api.DataConnector;
+import org.apache.hadoop.hive.metastore.api.MetaException;
+import org.apache.hadoop.hive.metastore.api.Table;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.sql.ResultSet;
+import java.sql.SQLException;
+
+public class RedshiftConnectorProvider extends AbstractJDBCConnectorProvider {
+    private static Logger LOG = LoggerFactory.getLogger(RedshiftConnectorProvider.class);
+
+    private static final String DRIVER_CLASS = "com.amazon.redshift.jdbc42.Driver".intern();

Review comment:
       There is no benefit at all calling intern() on static final field. Actually, I don't see a good reason to have field declaration here. We could inline the string to the constructor.

##########
File path: ql/src/test/queries/clientpositive/redshift_data_connector.q
##########
@@ -0,0 +1,65 @@
+-- SORT_QUERY_RESULTS
+SHOW CONNECTORS;
+
+-- CREATE with comment
+CREATE CONNECTOR redshift_test
+TYPE 'redshift'
+URL 'jdbc:redshift://redshift-cluster-1.c1gffkxfot1v.us-east-2.redshift.amazonaws.com:5439/dev'

Review comment:
       Agree with Zoltan, I don't think we can add this. Very likely now the Redhshift instance specified here may not be available and the test will fail.
   
   Since Postgres and Redshift have many similarities I would suggest adding at least CONNECTOR test with Postgres. An easy way to do it would be using `--!qt:database:postgres:` option.

##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/dataconnector/jdbc/AbstractJDBCConnectorProvider.java
##########
@@ -185,9 +185,18 @@ protected Connection getConnection() {
     return null;
   }
 
-  protected abstract ResultSet fetchTableMetadata(String tableName) throws MetaException;
+  protected ResultSet fetchTableMetadata(String tableName) throws MetaException {
+    try {
+      return fetchTablesViaDBMetaData(tableName);
+    }
+    catch (SQLException sqle) {
+      throw new MetaException("Error while trying to access the table names in the database" + sqle);
+    }
+  }
 
-  protected abstract ResultSet fetchTableNames() throws MetaException;
+  protected ResultSet fetchTableNames() throws MetaException {
+    return fetchTableMetadata(null);
+  }

Review comment:
       I don't see any usages of `fetchTableMetadata` or `fetchTableNames` in the project. How about removing these methods completely?

##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/dataconnector/jdbc/RedshiftConnectorProvider.java
##########
@@ -0,0 +1,86 @@
+package org.apache.hadoop.hive.metastore.dataconnector.jdbc;
+
+import org.apache.hadoop.hive.metastore.ColumnType;
+import org.apache.hadoop.hive.metastore.api.DataConnector;
+import org.apache.hadoop.hive.metastore.api.MetaException;
+import org.apache.hadoop.hive.metastore.api.Table;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.sql.ResultSet;
+import java.sql.SQLException;
+
+public class RedshiftConnectorProvider extends AbstractJDBCConnectorProvider {

Review comment:
       Maybe it would be better to extend the Postgres connector instead of the Abstract. Most of the datatypes listed below do exist in Postgres as well so we could possibly add them there and override `getDataType` only for those that do not exist.

##########
File path: ql/src/test/queries/clientpositive/redshift_data_connector.q
##########
@@ -0,0 +1,65 @@
+-- SORT_QUERY_RESULTS
+SHOW CONNECTORS;
+
+-- CREATE with comment
+CREATE CONNECTOR redshift_test
+TYPE 'redshift'
+URL 'jdbc:redshift://redshift-cluster-1.c1gffkxfot1v.us-east-2.redshift.amazonaws.com:5439/dev'

Review comment:
       As mentioned previously, the test should have `SELECT` and `EXPLAIN SELECT` queries to verify the type mapping is done correctly.

##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/dataconnector/jdbc/RedshiftConnectorProvider.java
##########
@@ -0,0 +1,86 @@
+package org.apache.hadoop.hive.metastore.dataconnector.jdbc;
+
+import org.apache.hadoop.hive.metastore.ColumnType;
+import org.apache.hadoop.hive.metastore.api.DataConnector;
+import org.apache.hadoop.hive.metastore.api.MetaException;
+import org.apache.hadoop.hive.metastore.api.Table;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.sql.ResultSet;
+import java.sql.SQLException;
+
+public class RedshiftConnectorProvider extends AbstractJDBCConnectorProvider {
+    private static Logger LOG = LoggerFactory.getLogger(RedshiftConnectorProvider.class);
+
+    private static final String DRIVER_CLASS = "com.amazon.redshift.jdbc42.Driver".intern();
+
+    public RedshiftConnectorProvider(String dbName, DataConnector dataConn) {
+        super(dbName, dataConn, DRIVER_CLASS);
+    }
+
+    protected String getDataType(String dbDataType, int size) {
+        String mappedType = super.getDataType(dbDataType, size);
+
+        // The VOID type points to the corresponding datatype not existing in hive. These datatypes are datastore
+        // specific. They need special handling. An example would be the Geometric type that is not supported in Hive.
+        // The other cases where a datatype in redshift is resolved to a VOID type are during the use of aliases like
+        // float8, int8 etc. These can be mapped to existing hive types and are done below.
+        if (!mappedType.equalsIgnoreCase(ColumnType.VOID_TYPE_NAME)) {
+            return mappedType;
+        }
+
+        // map any db specific types here.
+        //TODO: Geometric and other redshift specific types need to be supported.
+        switch (dbDataType.toLowerCase())
+        {
+            // The below mappings were tested by creating table definitions in redshift with the respective datatype
+            // and querying from the tables.
+            // e.g.
+            // create table test_int_8(i int8);
+            // insert into test_int_8 values(1);
+            // 0: jdbc:hive2://> select * from test_int_8;
+            //+---------------+
+            //| test_int_8.i  |
+            //+---------------+
+            //| 1             |
+            //+---------------+
+            //1 row selected (13.381 seconds)
+            case "bpchar":
+                mappedType = ColumnType.CHAR_TYPE_NAME + wrapSize(size);
+                break;
+            case "int2":
+                mappedType = ColumnType.SMALLINT_TYPE_NAME;
+                break;
+            case "int4":
+                mappedType = ColumnType.INT_TYPE_NAME;
+                break;
+            case "int8":
+                mappedType = ColumnType.BIGINT_TYPE_NAME;
+                break;
+            case "real":
+                mappedType = ColumnType.FLOAT_TYPE_NAME;
+                break;
+            case "float4":
+                mappedType = ColumnType.FLOAT_TYPE_NAME;
+                break;
+            case "float8":
+                mappedType = ColumnType.DOUBLE_TYPE_NAME;
+                break;
+            case "time":
+            case "time without time zone":
+                // A table with a time column gets resolved to "time without time zone" in hive.
+                mappedType = ColumnType.TIMESTAMP_TYPE_NAME;
+                break;
+            case "timez":
+            case "time with time zone":
+                mappedType = ColumnType.TIMESTAMPTZ_TYPE_NAME;
+                break;
+            default:
+                mappedType = ColumnType.VOID_TYPE_NAME;
+                break;

Review comment:
       ```suggestion
   ```

##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/dataconnector/jdbc/RedshiftConnectorProvider.java
##########
@@ -0,0 +1,86 @@
+package org.apache.hadoop.hive.metastore.dataconnector.jdbc;
+
+import org.apache.hadoop.hive.metastore.ColumnType;
+import org.apache.hadoop.hive.metastore.api.DataConnector;
+import org.apache.hadoop.hive.metastore.api.MetaException;
+import org.apache.hadoop.hive.metastore.api.Table;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.sql.ResultSet;
+import java.sql.SQLException;
+
+public class RedshiftConnectorProvider extends AbstractJDBCConnectorProvider {
+    private static Logger LOG = LoggerFactory.getLogger(RedshiftConnectorProvider.class);
+
+    private static final String DRIVER_CLASS = "com.amazon.redshift.jdbc42.Driver".intern();
+
+    public RedshiftConnectorProvider(String dbName, DataConnector dataConn) {
+        super(dbName, dataConn, DRIVER_CLASS);
+    }
+
+    protected String getDataType(String dbDataType, int size) {
+        String mappedType = super.getDataType(dbDataType, size);
+
+        // The VOID type points to the corresponding datatype not existing in hive. These datatypes are datastore
+        // specific. They need special handling. An example would be the Geometric type that is not supported in Hive.
+        // The other cases where a datatype in redshift is resolved to a VOID type are during the use of aliases like
+        // float8, int8 etc. These can be mapped to existing hive types and are done below.
+        if (!mappedType.equalsIgnoreCase(ColumnType.VOID_TYPE_NAME)) {
+            return mappedType;
+        }
+
+        // map any db specific types here.
+        //TODO: Geometric and other redshift specific types need to be supported.

Review comment:
       Please create a JIRA and associate it with the TODO.

##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/dataconnector/jdbc/RedshiftConnectorProvider.java
##########
@@ -0,0 +1,86 @@
+package org.apache.hadoop.hive.metastore.dataconnector.jdbc;
+
+import org.apache.hadoop.hive.metastore.ColumnType;
+import org.apache.hadoop.hive.metastore.api.DataConnector;
+import org.apache.hadoop.hive.metastore.api.MetaException;
+import org.apache.hadoop.hive.metastore.api.Table;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.sql.ResultSet;
+import java.sql.SQLException;
+
+public class RedshiftConnectorProvider extends AbstractJDBCConnectorProvider {
+    private static Logger LOG = LoggerFactory.getLogger(RedshiftConnectorProvider.class);
+
+    private static final String DRIVER_CLASS = "com.amazon.redshift.jdbc42.Driver".intern();
+
+    public RedshiftConnectorProvider(String dbName, DataConnector dataConn) {
+        super(dbName, dataConn, DRIVER_CLASS);
+    }
+
+    protected String getDataType(String dbDataType, int size) {
+        String mappedType = super.getDataType(dbDataType, size);
+
+        // The VOID type points to the corresponding datatype not existing in hive. These datatypes are datastore
+        // specific. They need special handling. An example would be the Geometric type that is not supported in Hive.
+        // The other cases where a datatype in redshift is resolved to a VOID type are during the use of aliases like
+        // float8, int8 etc. These can be mapped to existing hive types and are done below.
+        if (!mappedType.equalsIgnoreCase(ColumnType.VOID_TYPE_NAME)) {
+            return mappedType;
+        }
+
+        // map any db specific types here.
+        //TODO: Geometric and other redshift specific types need to be supported.
+        switch (dbDataType.toLowerCase())
+        {
+            // The below mappings were tested by creating table definitions in redshift with the respective datatype
+            // and querying from the tables.
+            // e.g.
+            // create table test_int_8(i int8);
+            // insert into test_int_8 values(1);
+            // 0: jdbc:hive2://> select * from test_int_8;
+            //+---------------+
+            //| test_int_8.i  |
+            //+---------------+
+            //| 1             |
+            //+---------------+
+            //1 row selected (13.381 seconds)

Review comment:
       ```suggestion
   ```
   Instead of explaining how things were tested concrete tests could be added. Create tables in Redshift/Postgres, populate them with some data and then run `SELECT` and `EXPLAIN SELECT` queries in Hive so that we can verify results are correct and types are mapped correctly.

##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/dataconnector/jdbc/RedshiftConnectorProvider.java
##########
@@ -0,0 +1,86 @@
+package org.apache.hadoop.hive.metastore.dataconnector.jdbc;
+
+import org.apache.hadoop.hive.metastore.ColumnType;
+import org.apache.hadoop.hive.metastore.api.DataConnector;
+import org.apache.hadoop.hive.metastore.api.MetaException;
+import org.apache.hadoop.hive.metastore.api.Table;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.sql.ResultSet;
+import java.sql.SQLException;
+
+public class RedshiftConnectorProvider extends AbstractJDBCConnectorProvider {
+    private static Logger LOG = LoggerFactory.getLogger(RedshiftConnectorProvider.class);
+
+    private static final String DRIVER_CLASS = "com.amazon.redshift.jdbc42.Driver".intern();
+
+    public RedshiftConnectorProvider(String dbName, DataConnector dataConn) {
+        super(dbName, dataConn, DRIVER_CLASS);
+    }
+
+    protected String getDataType(String dbDataType, int size) {
+        String mappedType = super.getDataType(dbDataType, size);
+
+        // The VOID type points to the corresponding datatype not existing in hive. These datatypes are datastore
+        // specific. They need special handling. An example would be the Geometric type that is not supported in Hive.
+        // The other cases where a datatype in redshift is resolved to a VOID type are during the use of aliases like
+        // float8, int8 etc. These can be mapped to existing hive types and are done below.
+        if (!mappedType.equalsIgnoreCase(ColumnType.VOID_TYPE_NAME)) {
+            return mappedType;
+        }

Review comment:
       ```suggestion
   ```
   The `if` is not necessary and possibly wrong. In terms of inheritance it is more natural to continue to the switch case below to attempt to override the `mappedType` if necessary. To do that we have to skip the `default` case below.




-- 
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: gitbox-unsubscribe@hive.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] vnhive commented on a change in pull request #2546: HIVE-25405 : Implement Connector Provider for Amazon Redshift

Posted by GitBox <gi...@apache.org>.
vnhive commented on a change in pull request #2546:
URL: https://github.com/apache/hive/pull/2546#discussion_r685949624



##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/dataconnector/jdbc/RedshiftConnectorProvider.java
##########
@@ -0,0 +1,101 @@
+package org.apache.hadoop.hive.metastore.dataconnector.jdbc;
+
+import org.apache.hadoop.hive.metastore.ColumnType;
+import org.apache.hadoop.hive.metastore.api.DataConnector;
+import org.apache.hadoop.hive.metastore.api.MetaException;
+import org.apache.hadoop.hive.metastore.api.Table;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.sql.ResultSet;
+import java.sql.SQLException;
+
+public class RedshiftConnectorProvider extends AbstractJDBCConnectorProvider {
+    private static Logger LOG = LoggerFactory.getLogger(RedshiftConnectorProvider.class);
+
+    private static final String DRIVER_CLASS = "com.amazon.redshift.jdbc42.Driver".intern();
+
+    public RedshiftConnectorProvider(String dbName, DataConnector dataConn) {
+        super(dbName, dataConn, DRIVER_CLASS);
+    }
+
+    @Override protected ResultSet fetchTableMetadata(String tableName) throws MetaException {

Review comment:
       Done !




-- 
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: gitbox-unsubscribe@hive.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] dantongdong commented on a change in pull request #2546: HIVE-25405 : Implement Connector Provider for Amazon Redshift

Posted by GitBox <gi...@apache.org>.
dantongdong commented on a change in pull request #2546:
URL: https://github.com/apache/hive/pull/2546#discussion_r680161971



##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/dataconnector/jdbc/RedshiftConnectorProvider.java
##########
@@ -0,0 +1,101 @@
+package org.apache.hadoop.hive.metastore.dataconnector.jdbc;
+
+import org.apache.hadoop.hive.metastore.ColumnType;
+import org.apache.hadoop.hive.metastore.api.DataConnector;
+import org.apache.hadoop.hive.metastore.api.MetaException;
+import org.apache.hadoop.hive.metastore.api.Table;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.sql.ResultSet;
+import java.sql.SQLException;
+
+public class RedshiftConnectorProvider extends AbstractJDBCConnectorProvider {
+    private static Logger LOG = LoggerFactory.getLogger(RedshiftConnectorProvider.class);
+
+    private static final String DRIVER_CLASS = "com.amazon.redshift.jdbc42.Driver".intern();
+
+    public RedshiftConnectorProvider(String dbName, DataConnector dataConn) {
+        super(dbName, dataConn, DRIVER_CLASS);
+    }
+
+    @Override protected ResultSet fetchTableMetadata(String tableName) throws MetaException {

Review comment:
       It seems like fetchTableMetadata and fetchTableNames method can be utilized across all the database type we currently support(except for MySQL). Do you think may be better to move the implementation of those two methods to AbstractJDBCConnectorProvider so that the other database types can benefit from it? There is a helper method named fetchTablesViaDBMetaData in AbstractJDBCConnectorProvider that may be helpful.




-- 
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: gitbox-unsubscribe@hive.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] kgyrtkirk commented on a change in pull request #2546: HIVE-25405 : Implement Connector Provider for Amazon Redshift

Posted by GitBox <gi...@apache.org>.
kgyrtkirk commented on a change in pull request #2546:
URL: https://github.com/apache/hive/pull/2546#discussion_r750441378



##########
File path: ql/src/test/queries/clientpositive/redshift_data_connector.q
##########
@@ -0,0 +1,65 @@
+-- SORT_QUERY_RESULTS
+SHOW CONNECTORS;
+
+-- CREATE with comment
+CREATE CONNECTOR redshift_test
+TYPE 'redshift'
+URL 'jdbc:redshift://redshift-cluster-1.c1gffkxfot1v.us-east-2.redshift.amazonaws.com:5439/dev'

Review comment:
       this seems like an external resource - that's not something we should have in a unit test
   

##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/dataconnector/jdbc/PostgreSQLConnectorProvider.java
##########
@@ -12,7 +12,7 @@
 import java.util.List;
 
 public class PostgreSQLConnectorProvider extends AbstractJDBCConnectorProvider {
-  private static Logger LOG = LoggerFactory.getLogger(MySQLConnectorProvider.class);
+  private static Logger LOG = LoggerFactory.getLogger(PostgreSQLConnectorProvider.class);

Review comment:
       lol :D




-- 
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: gitbox-unsubscribe@hive.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org