xiedeyantu commented on PR #4704: URL: https://github.com/apache/calcite/pull/4704#issuecomment-3693894121
> The code looks great, I just have some questions about the quidem test: > > 1. We want to use the new decorrelator to cover the test cases in the current iq files. After implementing left mark join in enumerable convention, we can verify the execution results. However, many iq files (such as sub-query.iq) use `!plan` to verify the plan (the plan is based on the old decorrelator ). How should we handle these? > 2. After we verify that the new decorrelator correctly covers the existing test cases, `CoreQuidemTest2` might be removed. Perhaps at that time, `CoreQuidemTest` will support a configuration similar to `set new_decorrelator`, allowing users to switch between them in the iq file. Is that the intended approach? For the first issue, since quidem does not support the !if-!else syntax, I have added enable_new_decorrelater to support the demonstration of the new decorrelator’s execution. For the second issue, I believe we may maintain this state for a long time until we consider the old decorrelator no longer needs maintenance. At that point, we might clean up some of the logic, but it is also possible that both algorithms will continue to coexist. What we need to do now is to gradually include the excluded files in CoreQuidemTest2 one by one and ensure the new decorrelator can execute correctly. -- 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]
