Support DictionaryArray in OVER clause

[adriangb]

Fixes #13151

[adriangb]

I tested this manually with datafusion-cli. Happy to add a test if needed but maybe it's okay as is.

[adriangb]

cc @alamb

[findepi]

It looks like a new & cool capability. Is it possible to add a test?

[adriangb]

More like existing functionality that wasn't enabled.

Any suggestions as to where I should add said test?

[alamb]

I tried to produce an error / write some examples in dictionary.slt and I couldn't figure out how to trigger an issue. Here is what I tried

Basically my feeble mind can't figure out if the newly added code can ever be executed.

(venv) andrewlamb@Andrews-MacBook-Pro-2:~/Software/datafusion$ git diff
diff --git a/datafusion/sqllogictest/test_files/dictionary.slt b/datafusion/sqllogictest/test_files/dictionary.slt
index 176331f57..e802ddfe6 100644
--- a/datafusion/sqllogictest/test_files/dictionary.slt
+++ b/datafusion/sqllogictest/test_files/dictionary.slt
@@ -320,6 +320,41 @@ ORDER BY
 2023-12-20T01:30:00 1000 f1 32.0
 2023-12-20T01:30:00 1000 f2 foo

+# Window Functions
+query TTTT
+SELECT "tag_id", "type",
+  lead("type") OVER (PARTITION BY "tag_id" ORDER BY "time") as "next_type",
+  lag("type") OVER (PARTITION BY "tag_id" ORDER BY "time") as "last_type"
+FROM m2;
+----
+1000 active active NULL
+1000 active active active
+1000 active active active
+1000 active active active
+1000 active active active
+1000 active passive active
+1000 passive passive active
+1000 passive passive passive
+1000 passive passive passive
+1000 passive NULL passive
+
+query TTTT
+SELECT "tag_id", "type",
+  lead("type") OVER (PARTITION BY "tag_id" ORDER BY "time"  RANGE BETWEEN 2 PRECEDING AND CURRENT ROW) as "next_type",
+  lag("type") OVER (PARTITION BY "tag_id" ORDER BY "time") as "last_type"
+FROM m2;
+----
+1000 active active NULL
+1000 active active active
+1000 active active active
+1000 active active active
+1000 active active active
+1000 active passive active
+1000 passive passive active
+1000 passive passive passive
+1000 passive passive passive
+1000 passive NULL passive
+

[alamb]

I am concerned that the lack of testing here would mean we could accidentally break the behavior without knowing it in some future refactor

[alamb]

Oh, now I see there is an example in #13151 (comment)

Maybe we can just add that example to dictionary.slt and we'll be good?

[adriangb]

I have no experience with these slt files but I'll give it a shot!

[adriangb]

@alamb is 359721f right?

[adriangb]

Guess we'll find out soon 😆

[adriangb]

@alamb looks like that worked 🚀

[alamb]

@alamb is 359721f right?

BTW I normally run this like cargo test --test sqllogictests -- window --complete and it will fill in the correct results

[alamb]

Looks good to me -- thank you @adriangb

[alamb]

🚀