Steve Carlin has posted comments on this change. ( http://gerrit.cloudera.org:8080/22728 )
Change subject: IMPALA-13738 (Part2) WIP: Clean up code in Catalog's table and partition interfaces ...................................................................... Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/22728/2/fe/src/main/java/org/apache/impala/catalog/Table.java File fe/src/main/java/org/apache/impala/catalog/Table.java: http://gerrit.cloudera.org:8080/#/c/22728/2/fe/src/main/java/org/apache/impala/catalog/Table.java@135 PS2, Line 135: protected SqlConstraints sqlConstraints_ = new SqlConstraints(new ArrayList<>(), > I'm thinking about this field. The situation before this cleanup was defini I think it's a really good idea! Not sure of your issue with having the static SqlConstraints class. The SqlConstraints seems like an immutable object, so we wouldn't have to worry about it being modified. I suppose theoretically someone can make SqlConstraints and make it mutable? We can solve this one of two ways: 1) Put in a comment in SqlConstraints telling people NOT to do this :) 2) Create a derived ImmutableSqlConstraints subclass or EmptySqlConstraints which becomes more implied that this code should never be changed. Personally, I think #1 is good enough, #2 seems like over-engineering. But I definitely think we should do this. -- To view, visit http://gerrit.cloudera.org:8080/22728 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8d9c7ba876e39fa4f4d067761f25dc4ecfd55702 Gerrit-Change-Number: 22728 Gerrit-PatchSet: 2 Gerrit-Owner: Noemi Pap-Takacs <[email protected]> Gerrit-Reviewer: Daniel Becker <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Steve Carlin <[email protected]> Gerrit-Comment-Date: Fri, 04 Apr 2025 17:53:38 +0000 Gerrit-HasComments: Yes
