Skip to content

feat: support null_treatment, distinct, and filter for window functions in proto #18024

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Jefffrey merged 14 commits into from
Oct 17, 2025
Merged

feat: support null_treatment, distinct, and filter for window functions in proto #18024

Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
08151f4
chore: typo and comments
dqkqd Oct 12, 2025
12b4b51
test: add `null_treatment`, `distinct`, `filter` test
dqkqd Oct 12, 2025
fa49554
feat: add `null_treatment`, `distinct`, `filter` fields
dqkqd Oct 12, 2025
edbdae8
feat: support `null_treatment`
dqkqd Oct 12, 2025
ecb9a67
refactor: reuse common code
dqkqd Oct 12, 2025
5e9174e
feat: support `distinct`, `filter`
dqkqd Oct 12, 2025
f09c7ff
refactor: use `UNSPECIFIED`
dqkqd Oct 12, 2025
47285bd
chore: cleanup
dqkqd Oct 12, 2025
3168e14
test: add `null_treatment` for aggregate udf
dqkqd Oct 12, 2025
7d91dd9
feat: support `null_treatment` for aggegrate udf
dqkqd Oct 12, 2025
9dedd7f
refactor: use optional `null_treatment`
dqkqd Oct 13, 2025
8286f22
refactor: remove `UNSPECIFIED` in `null_treatment`
dqkqd Oct 14, 2025
6da6986
Merge remote-tracking branch 'public/main' into support-null-treatmen…
dqkqd Oct 14, 2025
7023c9f
Merge branch 'main' into support-null-treatment-distinct-and-filter-i…
Jefffrey Oct 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion datafusion/proto/proto/datafusion.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -518,6 +518,7 @@ message AggregateUDFExprNode {
LogicalExprNode filter = 3;
repeated SortExprNode order_by = 4;
optional bytes fun_definition = 6;
optional NullTreatment null_treatment = 7;
}

message ScalarUDFExprNode {
Expand All @@ -538,6 +539,9 @@ message WindowExprNode {
// repeated LogicalExprNode filter = 7;
WindowFrame window_frame = 8;
optional bytes fun_definition = 10;
optional NullTreatment null_treatment = 11;
bool distinct = 12;
LogicalExprNode filter = 13;
}

message BetweenNode {
Expand Down Expand Up @@ -622,6 +626,12 @@ message WindowFrameBound {
datafusion_common.ScalarValue bound_value = 2;
}

enum NullTreatment {
NULL_TREATMENT_UNSPECIFIED = 0;

@dqkqd dqkqd Oct 13, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As described in the issue, there should be a UNSPECIFIED field.
This is also noted here:
https://protobuf.dev/best-practices/dos-donts/#unspecified-enum

But I don't see we are doing it for other enums.

@dqkqd dqkqd Oct 13, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

e.g:

datafusion/datafusion/proto/proto/datafusion.proto

Lines 1314 to 1317 in b27fedc

enum GenerateSeriesName {
GS_GENERATE_SERIES = 0;
GS_RANGE = 1;
}

@Jefffrey Jefffrey Oct 14, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess we never did it before because we don't try to maintain version compatibility:

datafusion/datafusion/proto/src/lib.rs

Lines 46 to 50 in b27fedc

//! # Version Compatibility
//!
//! The serialized form are not guaranteed to be compatible across
//! DataFusion versions. A plan serialized with one version of DataFusion
//! may not be able to deserialized with a different version.

RESPECT_NULLS = 1;
IGNORE_NULLS = 2;
}

///////////////////////////////////////////////////////////////////////////////////////////////////
// Arrow Data Types
///////////////////////////////////////////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -1365,4 +1375,4 @@ message SortMergeJoinExecNode {
JoinFilter filter = 5;
repeated SortExprNode sort_options = 6;
datafusion_common.NullEquality null_equality = 7;
}
}
148 changes: 148 additions & 0 deletions datafusion/proto/src/generated/pbjson.rs
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

39 changes: 38 additions & 1 deletion datafusion/proto/src/generated/prost.rs
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading