Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/24097 )

Change subject: IMPALA-14832: Make auto-incrementing Kudu columns hidden and 
simplify code
......................................................................


Patch Set 1:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/24097/1/fe/src/main/java/org/apache/impala/analysis/AlterTableAlterColStmt.java
File fe/src/main/java/org/apache/impala/analysis/AlterTableAlterColStmt.java:

http://gerrit.cloudera.org:8080/#/c/24097/1/fe/src/main/java/org/apache/impala/analysis/AlterTableAlterColStmt.java@202
PS1, Line 202:     if (column.isHidden()) {
Why did you move this check to the end? Worth calling out in the commit message.


http://gerrit.cloudera.org:8080/#/c/24097/1/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/24097/1/fe/src/main/java/org/apache/impala/analysis/InsertStmt.java@986
PS1, Line 986:       if (!matchFound && !tblColumn.isHidden()) {
Is this a necessary change for Iceberg? I don't think it changes the Kudu logic 
to pull the check up here.


http://gerrit.cloudera.org:8080/#/c/24097/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:

http://gerrit.cloudera.org:8080/#/c/24097/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@a4766
PS1, Line 4766:
Can you mention in the commit message that getColumnsInHiveOrder omits hidden 
columns as of IMPALA-14592? I overlooked that change and had to search a bit to 
find it.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5da031c5dcd93ed02a6f9a21006325c2077be1e9
Gerrit-Change-Number: 24097
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Michael Smith <[email protected]>
Gerrit-Comment-Date: Fri, 13 Mar 2026 16:38:00 +0000
Gerrit-HasComments: Yes

Reply via email to