gianm commented on PR #14123: URL: https://github.com/apache/druid/pull/14123#issuecomment-1515688058
> What's the reason for the various changes in the tests? They all seem to actually be better plans anyway (well, there's the addition of a filter on an effectively constant virtual column, which I hope primarily ends up as an always-true bitmap and effectively becomes a noop), so I don't think they are problematic. Just wondering if we know why they changed or if it's more of a "well, they changed, but not necessarily in a bad way, so consider it good"? The changes are all due to reductions in the search space for subqueries. Analysis: - `CalciteQueryTest.testGroupByTimeFloorAndDimOnGroupByTimeFloorAndDim`: We're no longer considering plans that involve a `filter` in an outer query when `havingSpec` in the inner query would do. So, in this one, an outer `filter` got pushed into an inner `havingSpec`. - `CalciteJoinQueryTest.testLeftJoinOnTwoInlineDataSourcesWithTimeFilter`, `CalciteJoinQueryTest.testLeftJoinOnTwoInlineDataSourcesWithOuterWhere`, `CalciteJoinQueryTest.testInnerJoinOnTwoInlineDataSourcesWithOuterWhere`: These don't use DruidOuterQueryRel (the rel corresponding to the rule that was changed). Likely, the new plan and old plan were considered equally good by the planner, and there's some path-dependency to which one gets picked. So, that's a "well, they changed, but not necessarily in a bad way, so consider it good". -- 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]
