Changing the queryFrameWork in Calcite*Tests may have sideeffects

[kgyrtkirk]

• changes how its configured a bit to use an annotation instead of methods
• framework is cached - not thrown away; this makes it run a bit faster 16s vs 20s for CQT
• annotated cases which were using more merge buffers due to the fact that the framework was not reset to the default for every testcase (Changing the queryFrameWork in Calcite*Tests may have sideeffects #15427)

Fixes #15427.

This PR has:

• been self-reviewed.
• is a test only change

[LakshSingla]

I have a few general comments:

• The way things are configured before this patch, does changing minTopNThreshold do anything at all? For example, look at the test case testExactTopNOnInnerJoinWithLimit. We manually set the value, however, we don't create a new framework.

• I was also thinking that cannotVectorize() and msqCompatible() could be very good candidates for adding to the list, however, it seems that we are creating a map of values to the query frameworks that support those values, which we don't wanna add here as is.

• Can you add a Javadoc to this method, as to how it works, and that it caches the framework based on the properties supplied?

[kgyrtkirk]

• regarding minTopNThreshold: I was doing an equiv refactor - so that it retains the old behaviour;
  • if I change the limit to 2 it still runs just fine; however at execution time this conditional decides to run the other branch: so although it does not affect the plan itself; it does affect the execution path choosen
  • I also seen that minTopNThreshold can be overriden in the queryContext ; so there is not much reason to configure it at the framework level; but I was thinking there were whatever reasons....and I only wanted to fix this issue
  • I have to say the usage minTopNThreshold raises my elbows quite a bit - especially because in case mergeBufferCount is not 0 it have no effect...
  • I would rather put that change into a separate PR - it seems like it will be trivial; but in case things got out of hand... it will be better to have it separately
• I think cannotVectorize and msqCompatible are a bit different kind of settings; as they define the expected behaviour of the test; or kinda like obstacles it should be able to jump over :) do you have in mind something like: @TestConfigX(msqCompatible = false) ? we could probably do something like that at some point - but I'm not sure of the direct benefits of changing that right away
  • fyi: cannotVectorize also has an unexpected feature: interpretation of cannotVectorize in tests may stop further processing of the testcase #15423 :)

Can you add a Javadoc to this method

not sure which method :) but I've added some apidoc to SqlTestFrameworkConfig

[LakshSingla]

Thanks for the fix!!
I am also iffy on how the TopNThreshold is wired in the tests, though that is a separate discussion.