Vamsi-klu commented on PR #18512: URL: https://github.com/apache/pinot/pull/18512#issuecomment-4912317317
Traced this through SchemaUtils.validate and the fix is correct. The old code passed transformedColumns.retainAll(argumentColumns) as the %s argument; since Java evaluates all of checkState's arguments eagerly, that both rendered the boolean return value into the message (operators saw "Columns: true are a result of transformations...") and mutated transformedColumns as a side effect. The new guard computes the intersection into a fresh set and checks chainedTransformColumns.isEmpty(), which is logically equivalent to the old Collections.disjoint(...) condition, so validation behavior is unchanged and only the message improves. transformedColumns is not read after this block (just the primaryKeyColumns loop), so nothing relied on the old mutation. The regression test's intersection is a single column, so the exact "[x]" assertion is order-independent and it does fail against the old boolean output. Unused Collections import correctly removed. LGTM. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
