Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/20806 )
Change subject: IMPALA-12380 WIP ...................................................................... Patch Set 1: (18 comments) http://gerrit.cloudera.org:8080/#/c/20806/1/java/ext-data-source/jdbc/src/main/java/org/apache/impala/extdatasource/jdbc/conf/JdbcStorageConfigManager.java File java/ext-data-source/jdbc/src/main/java/org/apache/impala/extdatasource/jdbc/conf/JdbcStorageConfigManager.java: http://gerrit.cloudera.org:8080/#/c/20806/1/java/ext-data-source/jdbc/src/main/java/org/apache/impala/extdatasource/jdbc/conf/JdbcStorageConfigManager.java@32 PS1, Line 32: org.apache.hadoop.hive.conf.Constants; do we need this? http://gerrit.cloudera.org:8080/#/c/20806/1/java/ext-data-source/jdbc/src/main/java/org/apache/impala/extdatasource/jdbc/conf/JdbcStorageConfigManager.java@33 PS1, Line 33: // import org.apache.hadoop.hive.ql.secrets.URISecretSource; : // import org.apache.impala.extdatasource.jdbc.util.URISecretSource; remove http://gerrit.cloudera.org:8080/#/c/20806/1/java/ext-data-source/jdbc/src/main/java/org/apache/impala/extdatasource/jdbc/conf/JdbcStorageConfigManager.java@45 PS1, Line 45: define jdbc/conf/JdbcStorageConfig.java, don't need Contants from org.apache.hadoop.hive.conf.Constants, define as constant string directly. http://gerrit.cloudera.org:8080/#/c/20806/1/java/ext-data-source/jdbc/src/main/java/org/apache/impala/extdatasource/jdbc/conf/JdbcStorageConfigManager.java@79 PS1, Line 79: String key = properties.getProperty(keyTransform.apply(CONFIG_PWD_KEY)); add log to verify the key value http://gerrit.cloudera.org:8080/#/c/20806/1/java/ext-data-source/jdbc/src/main/java/org/apache/impala/extdatasource/jdbc/conf/JdbcStorageConfigManager.java@81 PS1, Line 81: passwd = getPasswdFromKeystore(keystore, key); add log to verify the passwrd http://gerrit.cloudera.org:8080/#/c/20806/1/java/ext-data-source/jdbc/src/main/java/org/apache/impala/extdatasource/jdbc/conf/JdbcStorageConfigManager.java@114 PS1, Line 114: char[] pwdCharArray = conf.getPassword(key); add log to check the password from keystore file http://gerrit.cloudera.org:8080/#/c/20806/1/java/ext-data-source/jdbc/src/main/java/org/apache/impala/extdatasource/jdbc/secrets/SecretSource.java File java/ext-data-source/jdbc/src/main/java/org/apache/impala/extdatasource/jdbc/secrets/SecretSource.java: http://gerrit.cloudera.org:8080/#/c/20806/1/java/ext-data-source/jdbc/src/main/java/org/apache/impala/extdatasource/jdbc/secrets/SecretSource.java@31 PS1, Line 31: SecretSource where is this interface used? http://gerrit.cloudera.org:8080/#/c/20806/1/java/ext-data-source/jdbc/src/main/java/org/apache/impala/extdatasource/jdbc/secrets/URISecretSource.java File java/ext-data-source/jdbc/src/main/java/org/apache/impala/extdatasource/jdbc/secrets/URISecretSource.java: http://gerrit.cloudera.org:8080/#/c/20806/1/java/ext-data-source/jdbc/src/main/java/org/apache/impala/extdatasource/jdbc/secrets/URISecretSource.java@33 PS1, Line 33: URISecretSource implement SecretSource interface? http://gerrit.cloudera.org:8080/#/c/20806/1/java/ext-data-source/jdbc/src/main/java/org/apache/impala/extdatasource/jdbc/secrets/URISecretSourceTest.java File java/ext-data-source/jdbc/src/main/java/org/apache/impala/extdatasource/jdbc/secrets/URISecretSourceTest.java: http://gerrit.cloudera.org:8080/#/c/20806/1/java/ext-data-source/jdbc/src/main/java/org/apache/impala/extdatasource/jdbc/secrets/URISecretSourceTest.java@a1 PS1, Line 1: empty file http://gerrit.cloudera.org:8080/#/c/20806/1/testdata/bin/copy-ext-data-sources.sh File testdata/bin/copy-ext-data-sources.sh: http://gerrit.cloudera.org:8080/#/c/20806/1/testdata/bin/copy-ext-data-sources.sh@59 PS1, Line 59: passwd1 password http://gerrit.cloudera.org:8080/#/c/20806/1/testdata/bin/copy-ext-data-sources.sh@59 PS1, Line 59: host1.password hiveuser http://gerrit.cloudera.org:8080/#/c/20806/1/testdata/workloads/functional-query/queries/QueryTest/jdbc-data-source-dbcp-password.test File testdata/workloads/functional-query/queries/QueryTest/jdbc-data-source-dbcp-password.test: http://gerrit.cloudera.org:8080/#/c/20806/1/testdata/workloads/functional-query/queries/QueryTest/jdbc-data-source-dbcp-password.test@43 PS1, Line 43: "dbcp.password":"password", remove http://gerrit.cloudera.org:8080/#/c/20806/1/testdata/workloads/functional-query/queries/QueryTest/jdbc-data-source-dbcp-password.test@44 PS1, Line 44: user/foo/ update path http://gerrit.cloudera.org:8080/#/c/20806/1/testdata/workloads/functional-query/queries/QueryTest/jdbc-data-source-dbcp-password.test@45 PS1, Line 45: = : http://gerrit.cloudera.org:8080/#/c/20806/1/testdata/workloads/functional-query/queries/QueryTest/jdbc-data-source-dbcp-password.test@50 PS1, Line 50: ---- QUERY : # Create external JDBC DataSource table : DROP TABLE IF EXISTS alltypes_jdbc_datasource_2; : CREATE TABLE alltypes_jdbc_datasource_2 ( : id INT, : bool_col BOOLEAN, : tinyint_col TINYINT, : smallint_col SMALLINT, : int_col INT, : bigint_col BIGINT, : float_col FLOAT, : double_col DOUBLE, : date_string_col STRING, : string_col STRING, : timestamp_col TIMESTAMP) : PRODUCED BY DATA SOURCE TestJdbcDataSource( : '{"database.type":"POSTGRES", : "jdbc.url":"jdbc:postgresql://$INTERNAL_LISTEN_HOST:5432/functional", : "jdbc.driver":"org.postgresql.Driver", : "driver.url":"$FILESYSTEM_PREFIX/test-warehouse/data-sources/jdbc-drivers/postgresql-jdbc.jar", : "dbcp.username":"hiveuser", : "dbcp.password":"password", : "table":"AllTypesWithQuote", : "column.mapping":"id=id, bool_col=Bool_col, tinyint_col=Tinyint_col, smallint_col=Smallint_col, int_col=Int_col, bigint_col=Bigint_col, float_col=Float_col, double_col=Double_col, date_string_col=Date_string_col, string_col=String_col, timestamp=Timestamp"}'); : ---- RESULTS : 'Table has been created.' : ==== remove http://gerrit.cloudera.org:8080/#/c/20806/1/testdata/workloads/functional-query/queries/QueryTest/jdbc-data-source-dbcp-password.test@94 PS1, Line 94: ==== : ---- QUERY : # Gets all types including a row with a NULL value. The predicate pushed to : # the DataSource. : select * : from alltypes_jdbc_datasource : where id > 10 and int_col< 5 limit 5 : ---- RESULTS : 11,false,1,1,1,10,1.100000023841858,10.1,'01/02/09','1',2009-01-02 00:11:00.450000000 : 12,true,2,2,2,20,2.200000047683716,20.2,'01/02/09','2',2009-01-02 00:12:00.460000000 : 13,false,3,3,3,30,3.299999952316284,30.3,'01/02/09','3',2009-01-02 00:13:00.480000000 : 14,true,4,4,4,40,4.400000095367432,40.4,'01/02/09','4',2009-01-02 00:14:00.510000000 : 20,true,0,0,0,0,0,0,'01/03/09','0',2009-01-03 00:20:00.900000000 : ---- TYPES : INT, BOOLEAN, TINYINT, SMALLINT, INT, BIGINT, FLOAT, DOUBLE, STRING, STRING, TIMESTAMP : ==== : ---- QUERY : # Gets specified columns. : select id, bool_col, smallint_col, float_col, double_col, date_string_col : from alltypes_jdbc_datasource : where id > 10 and int_col< 5 limit 5 : ---- RESULTS : 11,false,1,1.100000023841858,10.1,'01/02/09' : 12,true,2,2.200000047683716,20.2,'01/02/09' : 13,false,3,3.299999952316284,30.3,'01/02/09' : 14,true,4,4.400000095367432,40.4,'01/02/09' : 20,true,0,0,0,'01/03/09' : ---- TYPES : INT, BOOLEAN, SMALLINT, FLOAT, DOUBLE, STRING : ==== : ---- QUERY : # Gets specified columns from external jdbc table with case sensitive column names : # and table name. : select id, bool_col, smallint_col, float_col, double_col, date_string_col : from alltypes_jdbc_datasource_2 : where id > 10 and int_col< 5 limit 5 : ---- RESULTS : 11,false,1,1.100000023841858,10.1,'01/02/09' : 12,true,2,2.200000047683716,20.2,'01/02/09' : 13,false,3,3.299999952316284,30.3,'01/02/09' : 14,true,4,4.400000095367432,40.4,'01/02/09' : 20,true,0,0,0,'01/03/09' : ---- TYPES : INT, BOOLEAN, SMALLINT, FLOAT, DOUBLE, STRING : ==== : ---- QUERY : # Inner join with a non jdbc table : select a.id, b.int_col : from alltypes_jdbc_datasource a inner join functional.alltypes b on (a.id = b.id) : where a.id = 1 : ---- RESULTS : 1,1 : ---- TYPES : INT, INT : ==== : ---- QUERY : # Inner join with another jdbc table : select a.id, b.int_col : from alltypes_jdbc_datasource a inner join alltypes_jdbc_datasource_2 b on (a.id = b.id) : where a.id < 3 group by a.id, b.int_col : ---- RESULTS : 0,0 : 1,1 : 2,2 : ---- TYPES : INT, INT : ==== : ---- QUERY : # Cross join : select a.id, b.id : from alltypes_jdbc_datasource a cross join alltypes_jdbc_datasource b : where (a.id < 3 and b.id < 3) : order by a.id, b.id limit 10 : ---- RESULTS : 0,0 : 0,1 : 0,2 : 1,0 : 1,1 : 1,2 : 2,0 : 2,1 : 2,2 : ---- TYPES : INT, INT remove http://gerrit.cloudera.org:8080/#/c/20806/1/tests/custom_cluster/test_ext_data_sources.py File tests/custom_cluster/test_ext_data_sources.py: http://gerrit.cloudera.org:8080/#/c/20806/1/tests/custom_cluster/test_ext_data_sources.py@119 PS1, Line 119: @pytest.mark.execute_serially : @CustomClusterTestSuite.with_args( : impalad_args="--use_local_catalog=true", : catalogd_args="--catalog_topic_mode=minimal") : def test_data_source_tables(self, vector, unique_database): : """Start Impala cluster in LocalCatalog Mode""" : self.run_test_case('QueryTest/jdbc-data-source-dbcp-password', vector, use_db=unique_database) remove http://gerrit.cloudera.org:8080/#/c/20806/1/tests/custom_cluster/test_ext_data_sources.py@135 PS1, Line 135: @pytest.mark.execute_serially : @CustomClusterTestSuite.with_args( : impalad_args='--data_source_batch_size=2048') : def test_data_source_big_batch_size(self, vector, unique_database): : """Run test with batch size greater than default size 1024""" : self.run_test_case('QueryTest/jdbc-data-source-dbcp-password', vector, use_db=unique_database) : : @pytest.mark.execute_serially : @CustomClusterTestSuite.with_args( : impalad_args='--data_source_batch_size=512') : def test_data_source_small_batch_size(self, vector, unique_database): : """Run test with batch size less than default size 1024""" : self.run_test_case('QueryTest/jdbc-data-source-dbcp-password', vector, use_db=unique_database) remove -- To view, visit http://gerrit.cloudera.org:8080/20806 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2a9a47e12abcf46bc1a4e12ce95e7528b25c0e2b Gerrit-Change-Number: 20806 Gerrit-PatchSet: 1 Gerrit-Owner: Anonymous Coward <gsi...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com> Gerrit-Comment-Date: Sat, 16 Dec 2023 00:19:30 +0000 Gerrit-HasComments: Yes
