Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/23627 )
Change subject: IMPALA-14553: Run schema eval concurrently ...................................................................... Patch Set 14: Code-Review+1 (1 comment) One small nit, but otherwise I'm glad to see this change http://gerrit.cloudera.org:8080/#/c/23627/14/testdata/bin/generate-schema-statements.py File testdata/bin/generate-schema-statements.py: http://gerrit.cloudera.org:8080/#/c/23627/14/testdata/bin/generate-schema-statements.py@843 PS14, Line 843: def schema_only(table_name, table_format): : constraint = schema_only_constraints.get(table_format) : return constraint is not None and table_name not in constraint : : def schema_include(table_name, table_format): : constraint = schema_include_constraints.get(table_name) : return constraint is not None and table_format not in constraint : : def schema_exclude(table_name, table_format): : constraint = schema_exclude_constraints.get(table_name) : return constraint is not None and table_format in constraint Nit: For some of these, the direction of the name vs what it's testing can be unclear (e.g. schema_only is returning True if the table_format is NOT meeting the only constraint). Do you mind if we make this a bit more verbose? Something like: skip_due_to_only_constraint skip_due_to_include_constraint skip_due_to_exclude_constraint Maybe there is a more concise version: fails_only_constraint fails_include_constraint fails_exclude_constraint Or we can flip the conditions and use positive names: satisfies_only_constraint satisfies_include_constraint satisfies_exclude_constraint -- To view, visit http://gerrit.cloudera.org:8080/23627 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2a78d05fd6a0005c83561978713237da2dde6af2 Gerrit-Change-Number: 23627 Gerrit-PatchSet: 14 Gerrit-Owner: Michael Smith <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Jason Fehr <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]> Gerrit-Reviewer: Riza Suminto <[email protected]> Gerrit-Comment-Date: Sat, 15 Nov 2025 00:22:31 +0000 Gerrit-HasComments: Yes
