joooger commented on PR #5344: URL: https://github.com/apache/calcite/pull/5344#issuecomment-6275013738
> Are there no changes in any of the existing plans? @mihaibudiu No existing plan changes with the default configuration. I compared the actual plans and outputs of the whole `core` suite with and without this PR: the DiffRepository `_actual` files, the Quidem `.iq` outputs and the test failures. They are identical. Most tests run with the top-down optimizer disabled, so I also ran the whole `core` suite with `calcite.planner.topdown.opt=true`. In that mode many tests already fail on main, because their reference plans differ under top-down. Compared with main, there are two differences: * In `new-decorr.iq`, one query failed on main with an `AssertionError` in `TopDownRuleDriver.DeriveTrait`. It now returns its result, and the file passes. That assertion fires when a rel is derived after its set was merged into another one. The new derivation path made this more likely, so I added a check that skips such rels. * In `sub-query.iq`, one plan changes, and it now matches the reference plan in the file. The Windows failures came from line endings in the new test, and that is now fixed. I have also added these details to the PR description. -- 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]
