-
-
Notifications
You must be signed in to change notification settings - Fork 88
feat: add aggregate pushdown support via GetForeignUpperPaths #549
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
Open
JohnCari
wants to merge
11
commits into
supabase:main
Choose a base branch
from
JohnCari:feature/aggregate-pushdown
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.
+823
−9
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
Add support for pushing aggregate operations (COUNT, SUM, AVG, MIN, MAX) to foreign data sources via the GetForeignUpperPaths callback. New types: - AggregateKind enum: Represents supported aggregate functions - Aggregate struct: Contains aggregate operation details New ForeignDataWrapper trait methods: - supported_aggregates(): Declare which aggregates the FDW can push down - supports_group_by(): Declare GROUP BY pushdown support - get_aggregate_rel_size(): Cost estimation for aggregate queries - begin_aggregate_scan(): Initialize aggregate query execution New module: - upper.rs: Implements GetForeignUpperPaths callback for aggregate pushdown All new methods have default implementations for backward compatibility.
- Add debug2! logging throughout upper.rs for better observability: - HAVING clause rejection - Unsupported aggregate function names - DISTINCT modifier processing - Extracted aggregates list - GROUP BY columns - Cost estimation values - Update docs/contributing/native.md with aggregate trait methods - Add comprehensive Aggregate Pushdown section to docs/guides/query-pushdown.md
Member
|
Thanks for the PR! Could you format the code so CI can pass? |
Contributor
Author
|
Yes ofcourse, I will do it at night when I get home and thank you! @burmecia |
Contributor
Author
|
@burmecia working on this now |
Apply rustfmt formatting to satisfy CI checks: - Break long lines in debug2! macro calls - Adjust line formatting in interface.rs
…er_path - Replace direct List field access with pgrx::PgList::from_pg() for proper iteration - Fix create_foreign_upper_path signature for different PostgreSQL versions: - PG13-16: 9 parameters (no disabled_nodes, no fdw_restrictinfo) - PG17: 10 parameters (added fdw_restrictinfo) - PG18: 11 parameters (added disabled_nodes) - Use conditional compilation (#[cfg(feature = "...")]) for version-specific parameters
Replace pgrx::PgList usage with direct pg_sys::List element access. This avoids type availability issues during cargo test since PgList may not be exported at the pgrx crate level in all configurations. The new list_iter helper function iterates over List elements using raw pointer access to the elements array, which is always available as part of the pg_sys FFI bindings.
- Change type_oid: 0 to type_oid: pgrx::pg_sys::Oid::INVALID in Aggregate::deparse doctest - Change trait method doc examples to rust,ignore since they show implementation patterns with &self parameters that don't compile as standalone functions
Contributor
Author
|
@burmecia I finished formatting the code, it was harder and took me longer than i expected but finally finished, now all seven checks pass, let me know if you need anything else for this PR. |
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
This PR adds support for pushing aggregate operations (COUNT, SUM, AVG, MIN, MAX) to foreign data sources via the PostgreSQL
GetForeignUpperPathscallback. This enables FDWs to execute aggregate queries remotely, significantly reducing data transfer for analytics queries.Key Changes
New types in
interface.rs:AggregateKindenum: Represents supported aggregate functions (Count, CountColumn, Sum, Avg, Min, Max)Aggregatestruct: Contains aggregate operation details including kind, column, distinct flag, and aliassql_name(),deparse(),deparse_with_alias()for SQL generationNew ForeignDataWrapper trait methods:
supported_aggregates(): Declare which aggregates the FDW can push down (default: empty = no pushdown)supports_group_by(): Declare GROUP BY pushdown support (default: false)get_aggregate_rel_size(): Cost estimation for aggregate queriesbegin_aggregate_scan(): Initialize aggregate query executionNew module
upper.rs:GetForeignUpperPathscallback for aggregate pushdownGroupPathExtraDataBackward Compatibility
All new trait methods have default implementations, ensuring existing FDWs continue to work without modification. FDWs opt-in to aggregate pushdown by overriding
supported_aggregates()to return a non-empty vector.Example Usage
Related Issue
Closes #22
Test Plan
SELECT MIN(x), MAX(x), COUNT(*) FROM t)