Pranav Lodha has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/22865 )

Change subject: IMPALA-13869: Support for 'hive.sql.query' property for Hive 
JDBC tables
......................................................................


Patch Set 3:

(23 comments)

> Uploaded patch set 3.

http://gerrit.cloudera.org:8080/#/c/22865/1/fe/src/main/java/org/apache/impala/catalog/TableLoader.java
File fe/src/main/java/org/apache/impala/catalog/TableLoader.java:

http://gerrit.cloudera.org:8080/#/c/22865/1/fe/src/main/java/org/apache/impala/catalog/TableLoader.java@234
PS1, Line 234:
> we also need to check either table or query property is set for NON hive jd
Done


http://gerrit.cloudera.org:8080/#/c/22865/2/fe/src/main/java/org/apache/impala/catalog/TableLoader.java
File fe/src/main/java/org/apache/impala/catalog/TableLoader.java:

http://gerrit.cloudera.org:8080/#/c/22865/2/fe/src/main/java/org/apache/impala/catalog/TableLoader.java@234
PS2, Line 234:
> call Strings.isNullOrEmpty() for table and query
Done


http://gerrit.cloudera.org:8080/#/c/22865/2/fe/src/main/java/org/apache/impala/catalog/TableLoader.java@235
PS2, Line 235:     if (Strings.isNullOrEmpty(table) && 
Strings.isNullOrEmpty(query)) {
> line too long (98 > 90)
Done


http://gerrit.cloudera.org:8080/#/c/22865/1/fe/src/main/java/org/apache/impala/extdatasource/jdbc/JdbcDataSource.java
File fe/src/main/java/org/apache/impala/extdatasource/jdbc/JdbcDataSource.java:

http://gerrit.cloudera.org:8080/#/c/22865/1/fe/src/main/java/org/apache/impala/extdatasource/jdbc/JdbcDataSource.java@307
PS1, Line 307:     if (!Strings.isNullOrEmpty(tableName)) { // Ensure 'table' 
prop
> fix the indent after this line
Done


http://gerrit.cloudera.org:8080/#/c/22865/1/fe/src/main/java/org/apache/impala/extdatasource/jdbc/JdbcDataSource.java@336
PS1, Line 336:       // Use 'query' property if 'table' is null
> add an assertion to make sure query is not empty
Done


http://gerrit.cloudera.org:8080/#/c/22865/2/fe/src/main/java/org/apache/impala/extdatasource/jdbc/JdbcDataSource.java
File fe/src/main/java/org/apache/impala/extdatasource/jdbc/JdbcDataSource.java:

http://gerrit.cloudera.org:8080/#/c/22865/2/fe/src/main/java/org/apache/impala/extdatasource/jdbc/JdbcDataSource.java@307
PS2, Line 307: !Strings.isNullOr
> call !Strings.isNullOrEmpty(tableName)
Done


http://gerrit.cloudera.org:8080/#/c/22865/2/fe/src/main/java/org/apache/impala/extdatasource/jdbc/JdbcDataSource.java@319
PS2, Line 319:
> add indent spaces
Done


http://gerrit.cloudera.org:8080/#/c/22865/2/fe/src/main/java/org/apache/impala/extdatasource/jdbc/JdbcDataSource.java@338
PS2, Line 338: Strings.isNullOrEmpty(query)) {
> call Strings.isNullOrEmpty(query)
Done


http://gerrit.cloudera.org:8080/#/c/22865/2/fe/src/main/java/org/apache/impala/extdatasource/jdbc/conf/JdbcStorageConfigManager.java
File 
fe/src/main/java/org/apache/impala/extdatasource/jdbc/conf/JdbcStorageConfigManager.java:

http://gerrit.cloudera.org:8080/#/c/22865/2/fe/src/main/java/org/apache/impala/extdatasource/jdbc/conf/JdbcStorageConfigManager.java@108
PS2, Line 108: ng query = props.get(JdbcStorageConfig.QUERY.getPropertyName());
> import com.google.common.base.Strings, them call Strings.isNullOrEmpty(tabl
Done


http://gerrit.cloudera.org:8080/#/c/22865/2/fe/src/main/java/org/apache/impala/extdatasource/jdbc/conf/JdbcStorageConfigManager.java@110
PS2, Line 110:           throw new IllegalArgumentException(
> line too long (95 > 90)
Done


http://gerrit.cloudera.org:8080/#/c/22865/2/fe/src/main/java/org/apache/impala/extdatasource/jdbc/conf/JdbcStorageConfigManager.java@110
PS2, Line 110: egalArgu
> Don't need to mention non-Hive
Done


http://gerrit.cloudera.org:8080/#/c/22865/1/tests/custom_cluster/test_ext_data_sources.py
File tests/custom_cluster/test_ext_data_sources.py:

http://gerrit.cloudera.org:8080/#/c/22865/1/tests/custom_cluster/test_ext_data_sources.py@247
PS1, Line 247:
> remove spaces here
Done


http://gerrit.cloudera.org:8080/#/c/22865/1/tests/custom_cluster/test_ext_data_sources.py@273
PS1, Line 273:
> remove spaces
Done


http://gerrit.cloudera.org:8080/#/c/22865/2/tests/custom_cluster/test_ext_data_sources.py
File tests/custom_cluster/test_ext_data_sources.py:

http://gerrit.cloudera.org:8080/#/c/22865/2/tests/custom_cluster/test_ext_data_sources.py@84
PS2, Line 84:
> flake8: E501 line too long (96 > 90 characters)
Done


http://gerrit.cloudera.org:8080/#/c/22865/2/tests/custom_cluster/test_ext_data_sources.py@85
PS2, Line 85:
> flake8: E501 line too long (105 > 90 characters)
Done


http://gerrit.cloudera.org:8080/#/c/22865/2/tests/custom_cluster/test_ext_data_sources.py@100
PS2, Line 100:
> flake8: E501 line too long (97 > 90 characters)
Done


http://gerrit.cloudera.org:8080/#/c/22865/2/tests/custom_cluster/test_ext_data_sources.py@124
PS2, Line 124:
> flake8: E501 line too long (96 > 90 characters)
Done


http://gerrit.cloudera.org:8080/#/c/22865/2/tests/custom_cluster/test_ext_data_sources.py@125
PS2, Line 125:
> flake8: E501 line too long (109 > 90 characters)
Done


http://gerrit.cloudera.org:8080/#/c/22865/2/tests/custom_cluster/test_ext_data_sources.py@271
PS2, Line 271:
> flake8: E501 line too long (164 > 90 characters)
Done


http://gerrit.cloudera.org:8080/#/c/22865/2/tests/custom_cluster/test_ext_data_sources.py@299
PS2, Line 299:
> flake8: E501 line too long (164 > 90 characters)
Done


http://gerrit.cloudera.org:8080/#/c/22865/2/tests/custom_cluster/test_ext_data_sources.py@436
PS2, Line 436:     );
> flake8: W293 blank line contains whitespace
Done


http://gerrit.cloudera.org:8080/#/c/22865/2/tests/custom_cluster/test_ext_data_sources.py@436
PS2, Line 436:
> remove spaces
Done


http://gerrit.cloudera.org:8080/#/c/22865/2/tests/custom_cluster/test_ext_data_sources.py@507
PS2, Line 507:
> flake8: E501 line too long (92 > 90 characters)
Done



--
To view, visit http://gerrit.cloudera.org:8080/22865
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I039fcc1e008233a3eeed8d09554195fdb8c8706b
Gerrit-Change-Number: 22865
Gerrit-PatchSet: 3
Gerrit-Owner: Pranav Lodha <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Pranav Lodha <[email protected]>
Gerrit-Reviewer: Wenzhe Zhou <[email protected]>
Gerrit-Comment-Date: Fri, 13 Jun 2025 21:19:34 +0000
Gerrit-HasComments: Yes

Reply via email to