arvindKandpal-ksolves commented on PR #28218: URL: https://github.com/apache/flink/pull/28218#issuecomment-4646154932
Thanks for the detailed review @weiqingy ! Here is what was updated based on your feedback: - Added the missing null guard in `satisfyTraitsByInput` so it won't NPE when `satisfyCollation` returns null - Rewrote the test using `TableTestBase` + `util.verifyRelPlan(sql)` with a bounded `values` source so it only hits the planner and also pins the plan for future regressions - Renamed the test class from `ExpandConversionRuleFixTest` to `FlinkExpandConversionRuleTest` - Added a short comment above the `return null` in `satisfyCollation` explaining why skipping this path is safe and cannot cause a `CannotPlanException` for any valid query - The earlier CI red was a checkstyle issue (missing blank line before package) caused by our changes, now fixed -- 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]
