avamingli commented on PR #1762: URL: https://github.com/apache/cloudberry/pull/1762#issuecomment-4523937319
> Thanks! This is without a doubt a landmark work! Thanks. > I agree with the approach. Yes, GPORCA contains a huge number of logical transformations, but not all of them lead to such changes that the query can be calculated much faster. Let's find out which of the transformations were particularly effective and we will implement them in the PostgreSQL optimizer. Then, the PostgreSQL optimizer will be no worse than GPORCA. Not always, but it can be improved iteratively. Also my experience shows that bottom-up optimizers are faster. Agreed. ORCA's optimizer is routinely 100x+ slower than PG's. My take after this work is that ORCA's real edge was never the architecture, it was the features — and once the equivalent features land in PG (CTE handling being the clearest example), PG comes out ahead. Long term, gradually retiring ORCA and consolidating on PG is the right direction. > The work is so extensive that it takes time for a thorough review. At the same time, for the most part we are talking about logical transformations of queries, where the price of a mistake is not performance, but a wrong answer. So (take for example the first transformation - CTE Predicate Pushdown via OR Collection and CNF Conversion) I would like to read the original ACMSIGMOD article, its criticism and citations, look at the implementation and tests. It takes time. Agree correctness matters more than speed here.A few things that I hope make an earlier merge less risky:Every transformation here is grounded in techniques ORCA has used in Greenplum production for years. The theory isn't new. What's new is bringing it into the PG planner.Results were verified by diffing against ORCA across all 99 queries, on top of the regression suite. > > Is it OK for you if we take **2 months** to review these changes? For my part, I can promise that we (various guys from our team) will provide details as soon as the review of individual changes is performed, rather than accumulating a list of comments. One timing constraint I should flag: @chenjinbao1989 is preparing the PostgreSQL 14 → 16 kernel upgrade https://github.com/apache/cloudberry/pull/1760 (5700+ commits touching the planner). We have discussed about that rebasing this PR across that would be impractical, so landing it first is really the only realistic path. Not sure for the timing, but review doesn't have to end at merge. I'd genuinely welcome the team continuing to dig in afterwards, bugs found later are still bugs found. -- 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]
