Zoltan Borok-Nagy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/23530 )

Change subject: IMPALA-14092 Part1: Prohibit Unsupported Operation for paimon 
table
......................................................................


Patch Set 3: Code-Review+1

(8 comments)

Found a few nits, otherwise LGTM.

http://gerrit.cloudera.org:8080/#/c/23530/3/fe/src/main/java/org/apache/impala/analysis/AlterTableOrViewRenameStmt.java
File 
fe/src/main/java/org/apache/impala/analysis/AlterTableOrViewRenameStmt.java:

http://gerrit.cloudera.org:8080/#/c/23530/3/fe/src/main/java/org/apache/impala/analysis/AlterTableOrViewRenameStmt.java@87
PS3, Line 87: REANAME
typo: should be RENAME


http://gerrit.cloudera.org:8080/#/c/23530/3/fe/src/main/java/org/apache/impala/analysis/AlterTableOrViewRenameStmt.java@87
PS3, Line 87:
nit: continuation line requires +4 indent


http://gerrit.cloudera.org:8080/#/c/23530/3/fe/src/main/java/org/apache/impala/analysis/AlterTableStmt.java
File fe/src/main/java/org/apache/impala/analysis/AlterTableStmt.java:

http://gerrit.cloudera.org:8080/#/c/23530/3/fe/src/main/java/org/apache/impala/analysis/AlterTableStmt.java@125
PS3, Line 125: paimon
nit: in AlterTableOrViewRenameStmt.java it was all capital, here it's all lower 
case. It'd be better if those were consistent, e.g. all capital.


http://gerrit.cloudera.org:8080/#/c/23530/3/fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java
File fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java:

http://gerrit.cloudera.org:8080/#/c/23530/3/fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java@412
PS3, Line 412: paimon
nit: upper/lower case is inconsistent with other files.


http://gerrit.cloudera.org:8080/#/c/23530/3/fe/src/main/java/org/apache/impala/analysis/DropStatsStmt.java
File fe/src/main/java/org/apache/impala/analysis/DropStatsStmt.java:

http://gerrit.cloudera.org:8080/#/c/23530/3/fe/src/main/java/org/apache/impala/analysis/DropStatsStmt.java@113
PS3, Line 113: paimon
nit: inconsistent lower/upper case with other files


http://gerrit.cloudera.org:8080/#/c/23530/3/fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
File fe/src/main/java/org/apache/impala/analysis/InsertStmt.java:

http://gerrit.cloudera.org:8080/#/c/23530/3/fe/src/main/java/org/apache/impala/analysis/InsertStmt.java@624
PS3, Line 624: paimon
nit: lower case


http://gerrit.cloudera.org:8080/#/c/23530/3/fe/src/main/java/org/apache/impala/catalog/paimon/PaimonHiveTypeUtils.java
File fe/src/main/java/org/apache/impala/catalog/paimon/PaimonHiveTypeUtils.java:

http://gerrit.cloudera.org:8080/#/c/23530/3/fe/src/main/java/org/apache/impala/catalog/paimon/PaimonHiveTypeUtils.java@64
PS3, Line 64: refractor
nit: refactor


http://gerrit.cloudera.org:8080/#/c/23530/3/tests/query_test/test_paimon.py
File tests/query_test/test_paimon.py:

http://gerrit.cloudera.org:8080/#/c/23530/3/tests/query_test/test_paimon.py@54
PS3, Line 54:
nit: indentation is off



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie39fa4836cb1be1b1a53aa62d5c02d7ec8fdc9d7
Gerrit-Change-Number: 23530
Gerrit-PatchSet: 3
Gerrit-Owner: ji chen <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Riza Suminto <[email protected]>
Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]>
Gerrit-Comment-Date: Fri, 17 Oct 2025 14:37:10 +0000
Gerrit-HasComments: Yes

Reply via email to