Skip to content

Support FILTER in over clause - #1007

Merged
alamb merged 5 commits into
apache:mainfrom
lovasoa:filter-in-over-clause
Oct 23, 2023
Merged

Support FILTER in over clause#1007
alamb merged 5 commits into
apache:mainfrom
lovasoa:filter-in-over-clause

Conversation

@lovasoa

@lovasoalovasoa commented Oct 18, 2023•
edited
Loading

Copy link
Copy Markdown
Contributor

support FILTER clause in window functions

fixes#1006

@lovasoa
lovasoaforce-pushed the filter-in-over-clause branch from 4ada270 to c8cf584CompareOctober 18, 2023 16:05
@coveralls

coveralls commented Oct 18, 2023•
edited
Loading

Copy link
Copy Markdown

Pull Request Test Coverage Report for Build 6564024257

  • 66 of 67(98.51%) changed or added relevant lines in 13 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.04%) to 87.392%
Changes Missing CoverageCovered LinesChanged/Added Lines%
src/dialect/sqlite.rs1250.0%
TotalsCoverage Status
Change from base Build 6435373786:0.04%
Covered Lines:16844
Relevant Lines:19274
💛 - Coveralls

@lovasoa
lovasoaforce-pushed the filter-in-over-clause branch from c8cf584 to 64c232eCompareOctober 18, 2023 16:49
@lovasoa

lovasoa commented Oct 20, 2023

Copy link
Copy Markdown
ContributorAuthor

@alamb , I'd love to get your feedback on this 🙂

As before, I need this for SQLPage

@alambalamb changed the title filter in over clauseSupport FILTER in over clauseOct 20, 2023
alamb
alamb approved these changes Oct 20, 2023
View reviewed changes

@alambalamb left a comment

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.

Thanks @lovasoa -- I have a few suggestions and comments, but I also think we can merge this PR as is

Comment threadsrc/ast/mod.rs Outdated
Comment threadsrc/ast/mod.rs Outdated
Comment threadsrc/ast/mod.rs Outdated
Comment threadtests/sqlparser_sqlite.rs Outdated
Comment threadsrc/ast/mod.rs
Comment threadsrc/dialect/sqlite.rs
lovasoaand others added 3 commits October 20, 2023 22:15
@lovasoa@alamb
docstring
667ba0a
Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org>
@lovasoa@alamb
docstrings
9b7f67d
Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org>
@lovasoa
test with a non-aggregate window function name
dd51436
addresses https://github.com/sqlparser-rs/sqlparser-rs/pull/1007/files#r1367383050
@lovasoa

lovasoa commented Oct 20, 2023•
edited
Loading

Copy link
Copy Markdown
ContributorAuthor

@alamb : thanks for the review, I addressed your comments. We can see about removing AggregateExpressionWithFilter and supports_filter_during_aggregation later, but I'd be careful because it will prevent parsing some statements that parsed before.

lovasoa
lovasoa commented Oct 21, 2023
View reviewed changes
Comment threadsrc/ast/mod.rs Outdated
alamb
alamb approved these changes Oct 23, 2023
View reviewed changes

@alambalamb left a comment

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.

Thanks @lovasoa -- I still thing something is confused here as I don't think SQL supports things like select foo FILTER(where x > 5) FROM t which I think this PR does allow.

However I think this is a reasonable improvement

It would be great to sort out the naming of Dialect::supports_filter_during_aggregation as that now also controls filter during any function

@alamb
alamb merged commit ce62fe6 into apache:mainOct 23, 2023
@lovasoa
lovasoa deleted the filter-in-over-clause branch October 23, 2023 23:28
@alambalamb mentioned this pull request Oct 30, 2023
Merged
serprex pushed a commit to serprex/sqlparser-rs that referenced this pull request Nov 6, 2023
@lovasoa@alamb@serprex
SupportFILTERin over clause (apache#1007)
7db5d9f
Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org>
@pepijnvepepijnve mentioned this pull request Jan 9, 2025
Closed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alambalambalamb approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

support FILTER clause in window functions with an OVER clause

3 participants

@lovasoa@coveralls@alamb