Quanlong Huang 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 11: (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. http://gerrit.cloudera.org:8080/#/c/23536/10/fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java@658 PS10, Line 658: } : if (numRows >= 0) { : allTblProps.put("numRows", Long.toString(numRows)); : } : : // Add STATS_GENERATED_VIA_STATS_TASK if present in HMS parameters : if (msTbl != null && msTbl.getParameters() != null) { : String statsGenerated = msTbl.getParameters().get("STATS_GENERATED_VIA_STATS_TASK"); : if (statsGenerated != null) { : allTblProps.put("STATS_GENERATED_VIA_STATS_TASK", statsGenerated); : } : } : : / > FeTable.NUM_ROWS = "numRows" and StatsSetupConst.ROW_COUNT are two different > property keys. StatsSetupConst.ROW_COUNT is also "numRows": https://github.com/apache/hive/blob/6b5febcc559e9f20ebea6fb04679477cf125144f/standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/common/StatsSetupConst.java#L103 So I think they are the same key. Or did I miss anything? On the other side, I think the in-memory row count in Table instances are actually extracted from HMS properties: https://github.com/apache/impala/blob/826c8cf9b083ed951fbfe649dc9551436efceb7e/fe/src/main/java/org/apache/impala/catalog/Table.java#L458 Is there a case that 'table.getNumRows()' is negative but there is a property of "numRows" set in the HMS properties (msTbl.getParameters())? 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 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 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 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 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 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 in Python. So don't need to remove this. http://gerrit.cloudera.org:8080/#/c/23536/11/tests/metadata/test_show_create_table.py@341 PS11, Line 341: # For partition statements, we need special handling : if 'PARTITION' in s and 'SET TBLPROPERTIES' in s: : # Remove: SET TBLPROPERTIES (...) from partition statements : # Match from SET TBLPROPERTIES ( to the last ) : s = re.sub(r"SET\s+TBLPROPERTIES\s*\(.*\)", "", s, flags=re.DOTALL) : else: To be simple, I think we don't need changes in __remove_properties_maps(). Having an additional "SET" at the end is OK if both expected and actual sides have it. -- 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: 11 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 11:16:21 +0000 Gerrit-HasComments: Yes
