Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/21016 )
Change subject: IMPALA-12793: Create JDBC table without data source ...................................................................... Patch Set 6: (5 comments) http://gerrit.cloudera.org:8080/#/c/21016/5/fe/src/main/java/org/apache/impala/catalog/DataSourceTable.java File fe/src/main/java/org/apache/impala/catalog/DataSourceTable.java: http://gerrit.cloudera.org:8080/#/c/21016/5/fe/src/main/java/org/apache/impala/catalog/DataSourceTable.java@180 PS5, Line 180: // DataSourceTable is still represented as HDFS tables in the metastore but has a : // special table property __IMPALA_DATA_SOURCE_NAME to indicate that Impala should : // use an external data source so we need to add the table property with the name : // > I'm not sure we want to restrict the table properties to only the ones that Changed to check required JDBC parameters. http://gerrit.cloudera.org:8080/#/c/21016/5/fe/src/main/java/org/apache/impala/catalog/DataSourceTable.java@285 PS5, Line 285: ed by JDBC. Missing required metadata > Nit: For the JDBC table case, can we pass in dataSourceName as IMPALA_BUILT Done http://gerrit.cloudera.org:8080/#/c/21016/5/fe/src/main/java/org/apache/impala/catalog/local/LocalDataSourceTable.java File fe/src/main/java/org/apache/impala/catalog/local/LocalDataSourceTable.java: http://gerrit.cloudera.org:8080/#/c/21016/5/fe/src/main/java/org/apache/impala/catalog/local/LocalDataSourceTable.java@123 PS5, Line 123: TableLoadingException(String.format( > See comment from DataSourceTable.java Done http://gerrit.cloudera.org:8080/#/c/21016/5/fe/src/main/java/org/apache/impala/util/JsonUtil.java File fe/src/main/java/org/apache/impala/util/JsonUtil.java: http://gerrit.cloudera.org:8080/#/c/21016/5/fe/src/main/java/org/apache/impala/util/JsonUtil.java@43 PS5, Line 43: JSON > Nit: I would prefer either "Json" (lowercase s) or "JSON" (all caps). Betwe Done http://gerrit.cloudera.org:8080/#/c/21016/5/tests/custom_cluster/test_ext_data_sources.py File tests/custom_cluster/test_ext_data_sources.py: http://gerrit.cloudera.org:8080/#/c/21016/5/tests/custom_cluster/test_ext_data_sources.py@a66 PS5, Line 66: : : : : : : : : : : > Do we need to remove this? Put back this unit-test -- To view, visit http://gerrit.cloudera.org:8080/21016 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I765aa86b430246786ad85ab6857cefaf4332c920 Gerrit-Change-Number: 21016 Gerrit-PatchSet: 6 Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com> Gerrit-Reviewer: Abhishek Rawat <ara...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Joe McDonnell <joemcdonn...@cloudera.com> Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com> Gerrit-Reviewer: gaurav singh <gsi...@cloudera.com> Gerrit-Comment-Date: Sun, 25 Feb 2024 18:17:16 +0000 Gerrit-HasComments: Yes
