Skip to content

Conversation

@harshit54
Copy link

Adds the postgres version in the schema cache using SQL from the pg-meta library.

Copy link
Collaborator

@psteinroe psteinroe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution! Just one minor note 🙏🏼

) AS active_connections,
current_setting('max_connections') :: int8 AS max_connections;"#
)
.fetch_all(pool)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not just one row?

pub tables: Vec<Table>,
pub functions: Vec<Function>,
pub types: Vec<PostgresType>,
pub versions: Vec<Version>,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

any reason to have a Vec instead of a single struct?

@harshit54
Copy link
Author

Hey @psteinroe , thanks for the reply.

While it does make sense to have a single type instead of a Vec, the SchemaCacheItem trait is defined as

pub trait SchemaCacheItem { type Item; async fn load(pool: &PgPool) -> Vec<Self::Item>; }

is defined as having a Vec of the type. Should we make the version implement a different trait?

@psteinroe
Copy link
Collaborator

Ah yeah I think it makes sense to refactor this a bit. Do you want to do it in your pr or should we merge and do a follow-up? 🙏🏼

@harshit54
Copy link
Author

Let's do a follow up. I'm not sure how the refactoring will look like at the moment.

@psteinroe
Copy link
Collaborator

sounds good! the ci is failing due to an unrelated issue. I am going to fix it soon and then merge 🙏🏼

@psteinroe psteinroe merged commit d76481b into supabase-community:main Aug 29, 2024
@harshit54 harshit54 deleted the schema_cache_versions branch August 29, 2024 07:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants