DRILL-4270: Create a separate WindowFramer that supports the FRAME cl… #322
Conversation
…ause separate DefaultFrameTemplate into 2 implementations: one that supports custom frames (aggregations, first_value, last_value) and one that doesn't
|
The purpose of this PR is to separate the functions that support the FRAME clause in CustomFrameTemplate from the ones that don't in DefaultFrameTemplate. Work still needs to be done to make the refactoring complete (like making sure code is not duplicated between the templates), but I decided to leave it after I make the necessary changes to support the FRAME clause. @amansinha100 can you please review ? |
|
Overall refactoring looks ok. One thing that needs some clarity: Is the definition of default frame 'between unbounded preceding and current row' ? If the user explicitly specifies this frame in the query, will it be treated as default or custom frame (as opposed to not specifying anything frame) ? |
|
According to the SQL specification you can have a FRAME clause with FIRST_VALUE, LAST_VALUE or aggregate functions. Custom frame template will be used for those functions regardless of the presence/absence of the FRAME clause. |
|
maybe I should rename the templates to FrameTemplate and NoFrameTemplate instead of CustomFrameTemplate and DefaultFrameTemplate |
|
Renaming sounds fine to avoid misinterpretation. +1. |
2 participants
…ause
separate DefaultFrameTemplate into 2 implementations: one that supports custom frames (aggregations, first_value, last_value) and one that doesn't