Arnab Karmakar has posted comments on this change. ( http://gerrit.cloudera.org:8080/23536 )
Change subject: IMPALA-13066: Extend SHOW CREATE TABLE to include stats and partitions ...................................................................... Patch Set 12: (9 comments) http://gerrit.cloudera.org:8080/#/c/23536/10/fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java File fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java: http://gerrit.cloudera.org:8080/#/c/23536/10/fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java@238 PS10, Line 238: protected static String getSortingOrder(Map<String, String> properties) { > nit: please keep this unchanged. Done http://gerrit.cloudera.org:8080/#/c/23536/10/fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java@658 PS10, Line 658: out.append("ALTER TABLE ") : .append(getIdentSql(table.getDb().getName())).append('.') : .append(getIdentSql(table.getName())).append(' ') : .append("SET TBLPROPERTIES ") : .append(propertyMapToSql(allTblProps)) : .append(";\n\n"); : } : : // Column stats for non-partition columns : if (appendColumnStatsStatements(table, out)) { : out.append("\n"); : } : : / > > FeTable.NUM_ROWS = "numRows" and StatsSetupConst.ROW_COUNT are two differ Apologies, yes you were right, these references do point out htat both table.getNumRows() and msTbl.getParameters().get(StatsSetupConst.ROW_COUNT) read from the same HMS parameters using the same key ("numRows") Therefore, there's no case where table.getNumRows() is negative but HMS parameters have "numRows" set, because getNumRows() directly reads from those parameters. Removing these redundant lines ?? http://gerrit.cloudera.org:8080/#/c/23536/11/fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java File fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java: http://gerrit.cloudera.org:8080/#/c/23536/11/fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java@469 PS11, Line 469: > nit: we use 4 spaces indention here so need 2 more spaces Done http://gerrit.cloudera.org:8080/#/c/23536/11/fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java@723 PS11, Line 723: > nit: add 2 more spaces here Done http://gerrit.cloudera.org:8080/#/c/23536/11/fe/src/main/java/org/apache/impala/service/JniFrontend.java File fe/src/main/java/org/apache/impala/service/JniFrontend.java: http://gerrit.cloudera.org:8080/#/c/23536/11/fe/src/main/java/org/apache/impala/service/JniFrontend.java@576 PS11, Line 576: > nit: need 2 more spaces here Done http://gerrit.cloudera.org:8080/#/c/23536/11/fe/src/main/java/org/apache/impala/service/JniFrontend.java@578 PS11, Line 578: > nit: need 2 more spaces here Done http://gerrit.cloudera.org:8080/#/c/23536/11/fe/src/main/java/org/apache/impala/service/JniFrontend.java@582 PS11, Line 582: > nit: need 2 more spaces here Done http://gerrit.cloudera.org:8080/#/c/23536/11/tests/metadata/test_show_create_table.py File tests/metadata/test_show_create_table.py: http://gerrit.cloudera.org:8080/#/c/23536/11/tests/metadata/test_show_create_table.py@a218 PS11, Line 218: > nit: I think our code style prefer two blank lines before class definitions Done http://gerrit.cloudera.org:8080/#/c/23536/11/tests/metadata/test_show_create_table.py@341 PS11, Line 341: return re.sub( : properties_map_regex("WITH SERDEPROPERTIES"), "", : re.sub(properties_map_regex("TBLPROPERTIES"), "", s)).strip() : : def __get_properties_map(self, s, properties_map_name): : retur > To be simple, I think we don't need changes in __remove_properties_maps(). Done -- To view, visit http://gerrit.cloudera.org:8080/23536 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I87950ae9d9bb73cb2a435cf5bcad076df1570dc2 Gerrit-Change-Number: 23536 Gerrit-PatchSet: 12 Gerrit-Owner: Arnab Karmakar <[email protected]> Gerrit-Reviewer: Arnab Karmakar <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: Riza Suminto <[email protected]> Gerrit-Comment-Date: Mon, 10 Nov 2025 17:09:04 +0000 Gerrit-HasComments: Yes
