Skip to content

Add catalog provider for postgres - #34

Merged
reluctanthero104 merged 14 commits into
mainfrom
add_catalog_provider_for_postgres
Apr 11, 2026
Merged

Add catalog provider for postgres#34
reluctanthero104 merged 14 commits into
mainfrom
add_catalog_provider_for_postgres

Conversation

@reluctanthero104

@reluctanthero104 reluctanthero104 commented Mar 29, 2026

Copy link
Copy Markdown
Contributor

Adds the support for catalog provider on postgres. Note that datafusion doesn't support schema provider registration, so we will only keep the hierarchy into table & catalog for now.

@reluctanthero104 reluctanthero104 changed the title add catalog provider add catalog provider for postgres Mar 29, 2026
@reluctanthero104 reluctanthero104 changed the title add catalog provider for postgres (WIP) add catalog provider for postgres Mar 29, 2026
@reluctanthero104
reluctanthero104 force-pushed the add_catalog_provider_for_postgres branch 2 times, most recently from d944463 to 5960499 Compare March 30, 2026 13:53
@reluctanthero104 reluctanthero104 changed the title (WIP) add catalog provider for postgres Add catalog provider for postgres Apr 8, 2026
@reluctanthero104
reluctanthero104 force-pushed the add_catalog_provider_for_postgres branch 2 times, most recently from 2dc42d0 to fd15c1c Compare April 9, 2026 23:00
@reluctanthero104
reluctanthero104 marked this pull request as ready for review April 10, 2026 18:03
@reluctanthero104
reluctanthero104 requested a review from BtXin April 10, 2026 18:47

@BtXin BtXin 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.

Code Review: Add catalog provider for Postgres

Overall

Well-structured PR. The decomposition of the monolithic register_postgres_tables into focused helpers (init_postgres_pools, build_postgres_table_provider, build_and_register_table, register_table_in_knn_registry) is clean and makes both code paths easy to follow. Config validation is solid, test coverage is thorough, and the README additions are excellent.

See inline comments for specific suggestions. Items #1 (import style) and #3 (credential logging) should be addressed before merge; the rest are improvements that could be follow-ups.

Comment thread crates/skardi/src/sources/providers/sqlx/pg/postgres.rs Outdated
Comment thread crates/skardi/src/sources/providers/sqlx/pg/postgres.rs
Comment thread crates/skardi/src/sources/providers/sqlx/pg/postgres.rs Outdated
Comment thread crates/skardi/src/sources/providers/sqlx/pg/postgres.rs Outdated
Comment thread crates/skardi/src/sources/providers/sqlx/pg/postgres.rs Outdated
Comment thread demo/postgres/README.md
@reluctanthero104
reluctanthero104 force-pushed the add_catalog_provider_for_postgres branch from 4ee1ad1 to c8b79b9 Compare April 11, 2026 19:31
@reluctanthero104
reluctanthero104 merged commit 4db10a6 into main Apr 11, 2026
2 checks passed
@reluctanthero104
reluctanthero104 deleted the add_catalog_provider_for_postgres branch April 11, 2026 20:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants