Skip to content

adds postgres version to the schema cache. #141

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

Merged
merged 1 commit into from
Aug 29, 2024

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?


#[derive(Debug, Clone, Default)]
pub struct SchemaCache {
pub schemas: Vec<Schema>,
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
1 check failed
@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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants