Yifan Zhang has posted comments on this change. ( http://gerrit.cloudera.org:8080/20941 )
Change subject: IMPALA-12381: Set JDBC related properties in JDBC data source ...................................................................... Patch Set 2: Code-Review+1 (1 comment) http://gerrit.cloudera.org:8080/#/c/20941/2/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/20941/2/fe/src/main/java/org/apache/impala/catalog/DataSourceTable.java@324 PS2, Line 324: if (Strings.isNullOrEmpty(tblInitString)) { : if (dsPropertyMap != null) { : // Change keys to lower case. : for (Map.Entry<String, String> entry : dsPropertyMap.entrySet()) { : combinedPropertyMap.put(entry.getKey().toLowerCase(), entry.getValue()); : } : } : } else { : // Append additional properties of DataSource to initString. : try { : Map<String, String> tblPropertyMap = : JsonUtil.convertJSonToPropertyMap(tblInitString); : if (tblPropertyMap != null) { : // Change keys to lower case. : for (Map.Entry<String, String> entry : tblPropertyMap.entrySet()) { : combinedPropertyMap.put(entry.getKey().toLowerCase(), entry.getValue()); : } : } : } catch (ImpalaRuntimeException e) { : // Return initString which is set in the table creation statement if it's : // invalid JSON string. This could happen for non JDBC data source. : return tblInitString; : } : if (dsPropertyMap != null) { : for (Map.Entry<String, String> entry : dsPropertyMap.entrySet()) { : if (!combinedPropertyMap.containsKey(entry.getKey().toLowerCase())) { : combinedPropertyMap.put(entry.getKey().toLowerCase(), entry.getValue()); : } : } : } : } Can we reorganize this to: if (dsPropertyMap != null) { // Change keys to lower case. for (Map.Entry<String, String> entry : dsPropertyMap.entrySet()) { combinedPropertyMap.put(entry.getKey().toLowerCase(), entry.getValue()); } } if (Strings.isNullOrEmpty(tblInitString)) { Map<String, String> tblPropertyMap = JsonUtil.convertJSonToPropertyMap(tblInitString); ... } That would be more readable. -- To view, visit http://gerrit.cloudera.org:8080/20941 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0a0b5d9d7b06825842828c3722c2bcdb4eacccc8 Gerrit-Change-Number: 20941 Gerrit-PatchSet: 2 Gerrit-Owner: Wenzhe Zhou <[email protected]> Gerrit-Reviewer: Abhishek Rawat <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Yifan Zhang <[email protected]> Gerrit-Reviewer: gaurav singh <[email protected]> Gerrit-Comment-Date: Fri, 02 Feb 2024 11:53:39 +0000 Gerrit-HasComments: Yes
