-
Notifications
You must be signed in to change notification settings - Fork 1k
Add offset/origin params to time_bucket_gapfill #9123
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
Draft
tjgreen42
wants to merge
10
commits into
main
Choose a base branch
from
feat/gapfill-offset-origin
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
a26a3c4 to
69ffd96
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #9123 +/- ##
==========================================
- Coverage 82.42% 82.14% -0.29%
==========================================
Files 243 243
Lines 47911 46914 -997
Branches 12228 12241 +13
==========================================
- Hits 39492 38536 -956
- Misses 3550 3564 +14
+ Partials 4869 4814 -55 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
54038c3 to
7c07eec
Compare
…amed parameters This implements offset and origin support for time_bucket_gapfill() using named parameters with defaults, following the same pattern as time_bucket(). Changes: - Updated SQL function signatures to include optional origin and offset parameters - Integer variants: 5 args (bucket_width, ts, start, finish, offset) - Timestamp variants: 6 args (bucket_width, ts, start, finish, origin, offset) - Timezone variant: 7 args (bucket_width, ts, timezone, start, finish, origin, offset) - Updated C wrapper functions to pass origin/offset to underlying time_bucket - Updated func_cache.c with new function signatures - Added tests for all new parameter combinations - origin and offset are mutually exclusive - specifying both raises an error Fixes #1304
ec12969 to
fe69d32
Compare
- Apply clang-format to func_cache.c - Update extension.out expected output for new time_bucket_gapfill signatures - Add CHANGELOG entry for issue #1304
The assertion incorrectly expected 4 or 5 arguments, but with the new offset/origin parameters, the function now has 5, 6, or 7 arguments: - 5 args: integer variants - 6 args: timestamp variants (with origin and offset) - 7 args: timezone variant (with origin and offset) Also update the timezone check to look for 7 args instead of 5.
The EXPLAIN output now shows the expanded function call with the new optional origin and offset parameters (as NULLs when not specified).
Adds expected test output for the new offset and origin parameter tests in time_bucket_gapfill.
Keep both old 4/5-arg functions and add new 5/6/7-arg functions for offset/origin support. This ensures backward compatibility with existing code while adding the new feature. - Old 4-arg functions (bucket_width, ts, start, finish) preserved - New functions add offset parameter for integers (5 args) - New functions add origin and offset for timestamps (6 args) - New functions add origin and offset for timezone variant (7 args) - C code handles both old and new argument counts via PG_NARGS() - func_cache.c updated to handle all argument count variants
The get_origin_arg and get_offset_arg functions didn't handle the old 4-argument case, causing a list access out of bounds error. This fix adds proper handling for 4-arg variants by returning NULL for origin and offset. Also fixes have_timezone detection to distinguish between old 5-arg timezone variant and new 5-arg integer-with-offset by checking the type of the third argument.
Add the new time_bucket_gapfill variants with offset and origin parameters to latest-dev.sql so they are created during upgrade. Add DROP statements to reverse-dev.sql so the functions are removed during downgrade to maintain schema compatibility.
6f8d85a to
ee4cb11
Compare
The func_cache needs entries for both old 4-arg and new 5/6/7-arg gapfill variants for the sort transform optimization to work. Without these entries, the planner cannot use indexed scans for queries using the old gapfill function signatures.
ee4cb11 to
f384eaf
Compare
The 5-arg integer functions with offset conflicted with the 5-arg timezone variant during PostgreSQL function resolution when arguments were passed as untyped literals. Integer variants now stay at 4 args (bucket_width, ts, start, finish) without offset support. Timestamp and timezone variants keep their origin/offset parameters (6 and 7 args respectively). This maintains backward compatibility while avoiding the function resolution ambiguity error.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Adds
offsetandoriginparameters totime_bucket_gapfill()using named parameters with defaults.Fixes #1304
Function Signatures
All existing parameters remain in place. New optional parameters are added at the end with NULL defaults.
Integer Variants (5 args)
time_bucket_gapfill( bucket_width SMALLINT, ts SMALLINT, start SMALLINT = NULL, finish SMALLINT = NULL, "offset" SMALLINT = NULL ) RETURNS SMALLINT -- Same pattern for INT and BIGINTTimestamp Variants (6 args)
time_bucket_gapfill( bucket_width INTERVAL, ts TIMESTAMPTZ, start TIMESTAMPTZ = NULL, finish TIMESTAMPTZ = NULL, origin TIMESTAMPTZ = NULL, "offset" INTERVAL = NULL ) RETURNS TIMESTAMPTZ -- Same pattern for DATE and TIMESTAMPTimezone Variant (7 args)
time_bucket_gapfill( bucket_width INTERVAL, ts TIMESTAMPTZ, timezone TEXT, start TIMESTAMPTZ = NULL, finish TIMESTAMPTZ = NULL, origin TIMESTAMPTZ = NULL, "offset" INTERVAL = NULL ) RETURNS TIMESTAMPTZExamples
Offset: Business Day at 6 AM
Origin: Fiscal Months Starting April 1st
Timezone with Offset
Constraints
offsetandoriginare mutually exclusive (same astime_bucket)Testing
tsl/test/shared/sql/gapfill.sql.inlocf()andinterpolate()Disable-check: commit-count