Fang-Yu Rao has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/23905 )

Change subject: IMPALA-12844: Support setting DBPROPERTIES
......................................................................


Patch Set 12: Code-Review+1

(2 comments)

Thanks for addressing my minor comments on patch set 9!

I left 2 very minor comments. I don't have other comments at the moment.

I do not know enough about test_events_custom_configs.py so could not provide 
concrete feedback on this python file.

http://gerrit.cloudera.org:8080/#/c/23905/12/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/23905/12/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@8497
PS12, Line 8497: params.getSet_owner_params()
nit: I just noticed that we already called params.getSet_owner_params() at 
https://gerrit.cloudera.org/c/23905/10..12/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java#8465
 too.

I was wondering if we could move the declaration of setOwnerParams and 
setDbPropertiesParams before the first switch statement 
(https://gerrit.cloudera.org/c/23905/10..12/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java#8463).

    if (db == null) {
      throw new CatalogException("Database: " + dbName + " does not exist.");
    }

    TAlterDbSetOwnerParams setOwnerParams = null;
    TAlterDbSetDbPropertiesParams setDbPropertiesParams = null;
    switch (params.getAlter_type()) {
      ...
    }

In this case, we don't have to call params.getSet_owner_params() again.


http://gerrit.cloudera.org:8080/#/c/23905/12/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@8503
PS12, Line 8503:           TAlterDbSetDbPropertiesParams setDbPropertiesParams =
               :             params.getSet_dbproperties_params();
We could move the declaration of setDbPropertiesParams before the first switch 
statement so that we could save this line as suggested above.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I628c7bed4f0c39aed7f5f4ffea52421caf501933
Gerrit-Change-Number: 23905
Gerrit-PatchSet: 12
Gerrit-Owner: Balazs Hevele <[email protected]>
Gerrit-Reviewer: Anonymous Coward <[email protected]>
Gerrit-Reviewer: Balazs Hevele <[email protected]>
Gerrit-Reviewer: Csaba Ringhofer <[email protected]>
Gerrit-Reviewer: Fang-Yu Rao <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Sai Hemanth Gantasala <[email protected]>
Gerrit-Comment-Date: Thu, 05 Feb 2026 19:22:48 +0000
Gerrit-HasComments: Yes

Reply via email to