Skip to content

Implement check_function_bodies only for Postgres#135

Open
cmer wants to merge 1 commit into
teoljungberg:masterfrom
cmer:check_function_bodies-pg
Open

Implement check_function_bodies only for Postgres#135
cmer wants to merge 1 commit into
teoljungberg:masterfrom
cmer:check_function_bodies-pg

Conversation

@cmer

@cmer cmer commented Aug 29, 2023

Copy link
Copy Markdown

No description provided.

@cmer

cmer commented Aug 29, 2023

Copy link
Copy Markdown
Author

Here's a different implementation of #134 . Specs don't pass (not sure why yet), but this is simpler and only for PG.

@cmer

cmer commented Aug 29, 2023

Copy link
Copy Markdown
Author

Is this implementation more aligned with what you had in mind?

@cmer

cmer commented Sep 30, 2024

Copy link
Copy Markdown
Author

Any chance this could be merged in? Thanks!

@teoljungberg

Copy link
Copy Markdown
Owner

I'm unsure I want to support this, because I have no context of the use-case or how it is supported in other databases (and therefore the other adapters).

@cmer

cmer commented Nov 8, 2024

Copy link
Copy Markdown
Author

From the previous PR:

Added a new configuration parameter check_function_bodies to allow controlling the database behavior when loading functions.

This is necessary if the function created relies on the existence of a table that is not yet created, for example.

Disabling check_function_bodies is pg_dump's default behaviour and, in my opinion, should be Fx's default behavior as well.

As mentioned, this is the default behavior of pg_dump and essential to load a schema that includes functions. Without this, creating a functions that depend on tables that are yet to be created fails (ie: will be created in a subsequent query from the schema).

@teoljungberg

Copy link
Copy Markdown
Owner

Thanks @cmer - I need to do some reading on this and testing. Can't promise when or if that happens, but I will try.

@cmer

cmer commented Nov 10, 2024

Copy link
Copy Markdown
Author

Thanks! This would be game-changer TBH. Hope you see the value in it.

Comment thread lib/fx/configuration.rb
# https://www.postgresql.org/docs/9.5/runtime-config-client.html#GUC-CHECK-FUNCTION-BODIES
#
# Set to `nil` to use the database default configuration.
# The default and recommended value is `false` to mimic pg_dump's behavior.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I am not sure about this. The docs say:

This parameter is normally on.

So maybe the default should be true here, with the possibility to set it to false for specific use cases?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

You could do that. It's off by default for pg_dump from what I remember, and this gem is the closest thing to pg_dump so it seemed reasonable to follow the pg_dump conventions. Either way works. Just having the option would be a huge win!

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@cmer

cmer commented Sep 12, 2025

Copy link
Copy Markdown
Author

Still hoping for this to get merged in! 🙏

@teoljungberg

Copy link
Copy Markdown
Owner

Keeping this open as I'd like to support this, if it proves of value!

I have just released a new version after lagging behind a lot, with some small refactorings and versioning updates (ruby x rails).

What I'd like to tackle next is #180, because it dawned on me that I'm supporting EOL postgres versions and that feels...redundant.

Once that is done, I'll come back to this. I need to review some more what this functional use-case is, benefits, drawbacks. But expect that after the holiday break.

@cmer

cmer commented Dec 12, 2025

Copy link
Copy Markdown
Author

I'm still here for it.

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.

3 participants