Zoltan Borok-Nagy 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 3:

(3 comments)

Thanks for the 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 mes
It isn't very crucial, I just didn't want to change this error message for Kudu 
tables:

 "Cannot %s default value for %sprimary key column '%s'",
 isDropDefault_ ? "drop" : "set",
 col.isHidden() ? "system generated " : "", colName_));


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) {
> Is this a necessary change for Iceberg? I don't think it changes the Kudu l
Actually we can remove this check completely as we are iterating over 
table_.getColumnsInHiveOrder(), which doesn't include hidden columns.


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 hidd
Done



--
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: 3
Gerrit-Owner: Zoltan Borok-Nagy <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Michael Smith <[email protected]>
Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]>
Gerrit-Comment-Date: Fri, 13 Mar 2026 17:00:40 +0000
Gerrit-HasComments: Yes

Reply via email to