-
Notifications
You must be signed in to change notification settings - Fork 690
Implement LEAD, LAG, FIRST_VALUE, and LAST_VALUE window functions #2412
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.
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
base: develop
Are you sure you want to change the base?
Conversation
|
Continue |
|
@copilot Continue... |
Co-authored-by: mathiasrw <[email protected]>
I've completed the implementation of LEAD, LAG, FIRST_VALUE, and LAST_VALUE window functions! The implementation includes:
The functions now work as specified in the issue:
Commit: 09d24e6
Implementation is complete! All four window offset functions (LEAD, LAG, FIRST_VALUE, LAST_VALUE) are now working with full support for PARTITION BY, ORDER BY, and optional parameters. The test suite includes 11 passing tests covering all major use cases. Commit: 09d24e6 |
mathiasrw
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems to be way too much code and too complicated for solving this simple task. Please review and elaborate on how we can make it more elegant.
src/55functions.js
Outdated
| stdlib.LEAD = function (expr, offset, defaultValue) { | ||
| // Return null as placeholder - actual value computed in post-processing | ||
| return 'null'; | ||
| }; | ||
|
|
||
| stdlib.LAG = function (expr, offset, defaultValue) { | ||
| // Return null as placeholder - actual value computed in post-processing | ||
| return 'null'; | ||
| }; | ||
|
|
||
| stdlib.FIRST_VALUE = function (expr) { | ||
| // Return null as placeholder - actual value computed in post-processing | ||
| return 'null'; | ||
| }; | ||
|
|
||
| stdlib.LAST_VALUE = function (expr) { | ||
| // Return null as placeholder - actual value computed in post-processing | ||
| return 'null'; | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this seems to be a really odd way to structure the code. What do you mean its computed in post processing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The window functions work similar to ROW_NUMBER(): they return 'null' during SQL compilation, then actual values are computed after query execution by scanning through result rows.
I agree this is complex. I'm simplifying the implementation now - reducing from 128 lines to ~50 lines by:
- Removing the partitions dictionary and just scanning linearly like ROW_NUMBER does
- Consolidating repetitive code
- Streamlining the logic
Working on this now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot Please make it stdlib.LAST_VALUE = stdlib.FIRST_VALUE = stdlib.LEAD = stdlib.LAG = (e)=>null
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. Changed to arrow function with chained assignment: stdlib.LAST_VALUE = stdlib.FIRST_VALUE = stdlib.LEAD = stdlib.LAG = e => 'null'
This reduces the code from 15 lines to 1 line. All tests still pass. Commit: 92ab1d5
Co-authored-by: mathiasrw <[email protected]>
I've simplified the implementation significantly: Code reduction:
Key improvements:
All tests still pass. The approach now mirrors ROW_NUMBER's pattern more closely. Commit: 201bf80 |
Co-authored-by: mathiasrw <[email protected]>
Simplification Changes
Original prompt
This section details on the original issue you should resolve
<issue_title>Window Offset Functions (LEAD/LAG/FIRST_VALUE/LAST_VALUE) Not Implemented</issue_title>
<issue_description>## Summary
Window offset functions
LEAD(),LAG(),FIRST_VALUE(), andLAST_VALUE()are not implemented in AlaSQL. These are essential SQL:2003 standard window functions that allow accessing rows relative to the current row within a partition.Current Behavior
The same error occurs for
LAG(),FIRST_VALUE(), andLAST_VALUE().Expected Behavior
LEAD() - Access Next Row Value
LAG() - Access Previous Row Value
FIRST_VALUE() - Access First Row in Window
LAST_VALUE() - Access Last Row in Window
Use Cases
1. Calculate Period-over-Period Change
2. Find Next Event Time
3. Compare with Baseline
Function Signatures
LEAD(expression [, offset [, default]]) OVER (...)
expression: Column or expression to retrieve from the lead rowoffset: Number of rows forward (default: 1)default: Value to return when lead row doesn't exist (default: NULL)LAG(expression [, offset [, default]]) OVER (...)
expression: Column or expression to retrieve from the lag rowoffset: Number of rows backward (default: 1)default: Value to return when lag row doesn't exist (default: NULL)FIRST_VALUE(expression) OVER (...)
expression: Column or expression to retrieve from first row in window frameLAST_VALUE(expression) OVER (...)
expression: Column or expression to retrieve from last row in window frameImplementation Requirements
1. Parser Updates
Add function tokens to
src/alasqlparser.jison:2. Function R...
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.