Moved the kstream window application to the physical plan phase.

[hjafarpour]

Description

This is a minor refactor to move the code for the application of windows in the physical plan from parser to engine. This code is physical plan code and should be in engine.

Testing done

Unit tests.

Reviewer checklist

• [ ] Ensure docs are updated if necessary. (eg. if a user visible feature is being added or changed).
• [ ] Ensure relevant issues are linked (description should include text like "Fixes #")

[hjafarpour]

@dguy the change is more from the point of separating the parsing layer from the physical plan layer. Parser layer should be agnostic to the physical execution layer. The AST nodes should not know which physical plan layer they will be used in and should not have any dependency to the KStreams. Similarly, if we have new window types the physical layer that uses the window is responsible to apply it based on the physical layer specifications.

[big-andy-coates]

+1 to what @dguy is saying, though I can understand wanting to decouple. Not sure how else to handle this though.

Maybe what is needed is to convert/wrap the WindowExpression currently passed to SchemaKGroupedStream.aggregate into a suitable engine-specific polymorphic type, which handles the aggregate call, e.g. maybe it should be the job of Analyzer.analyzeWindowExpression to convert the parser-specific WindowExpression into a suitable engine-specific polymorphic window type.

i.e. there should be one bit of code that takes the output from the parser and converts it into the polymorphic type the engine needs.

BTW, there are a few other places in parser that use kstreams stuff directly. These should also be addressed and then remove kstreams as a dependency from ksql-parser module so that using the physical stuff won't compile.

[hjafarpour]

@big-andy-coates I don't think we need any engine specific wrapper for this. I know you prefer not to use instance of check so I changed it to switch using WindowType representing the type of the window for each class. Engine needs to know the window type to take the proper action and the window class should not know how different engines will use it. I agree with removing the dependency to KStreams from the parser. This may be a bit more complex since currently, parser depends on common which include kafka dependency that has KStreams in it. But we can definitely start looking into it and enforce exclusion of physical layer dependency in parser.

[cla-assistant[bot]]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}