Skip to content

Add a generic function compiler to handle IGNORE NULLS in window functions - #429

Merged
hashhar merged 1 commit into
trinodb:masterfrom
laserkaplan:ignore-nulls
Jan 18, 2024
Merged

hashhar merged 1 commit into
trinodb:masterfrom
laserkaplan:ignore-nulls

Conversation

@laserkaplan

@laserkaplan laserkaplan commented Dec 6, 2023 •

Copy link
Copy Markdown
Member

Description

Adds a simple compiler method for using the IGNORE NULLS functionality of window functions in Trino. I implemented this privately in a package for work but figured I could generalize it and add it to the package as a whole :)

Non-technical explanation

Release notes

( ) This is not user-visible or docs only and no release notes are required.
( ) Release notes are required, please propose a release note for me.
( ) Release notes are required, with the following suggested text:

* Fix some things. ({issue}`issuenumber`)

@cla-bot cla-bot Bot added the cla-signed label Dec 6, 2023
Comment thread tests/unit/sqlalchemy/test_compiler.py
@laserkaplan
laserkaplan requested a review from hovaesco December 21, 2023 15:44
@hovaesco

Copy link
Copy Markdown
Member

Please squash commits and rebase on master.

@laserkaplan
laserkaplan force-pushed the ignore-nulls branch 3 times, most recently from 4312668 to 1ba5e43 Compare December 27, 2023 15:12
@laserkaplan

Copy link
Copy Markdown
Member Author

Please squash commits and rebase on master.

Took a couple of tries but done now :P

Add a generic function compiler to handle IGNORE NULLS in window
functions.
@hashhar

hashhar commented Jan 18, 2024

Copy link
Copy Markdown
Member

just reworded the commit message for release note's sake. Merging.

@hashhar
hashhar merged commit b5225e5 into trinodb:master Jan 18, 2024
evansd added a commit to opensafely-core/ehrql that referenced this pull request Feb 27, 2024
This doesn't benefit us at all (it's designed to avoid the compilation
overhead when executing the same SQL over and over) and it leads to
warnings when we execute clause elements whose caching behaviour has not
be explicitly defined.

We're getting a warning here now because we've just updated to Trino
v0.328.0 which includes this PR:
 * trinodb/trino-python-client#429

This defines a compiler for the `Lag` clause which (because it is not
restricted to a specific dialect) ends up being used by SQLite and
triggering the warning.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

3 participants