Skip to content
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

PG-1361 Fix relocatablility #54

Merged
merged 3 commits into from
Feb 11, 2025
Merged

Conversation

jeltz
Copy link
Collaborator

@jeltz jeltz commented Feb 10, 2025

Creating the extension in another schema was broken.

$ CREATE EXTENSION pg_tde SCHEMA foo;
CREATE EXTENSION
# SELECT foo.pg_tde_add_key_provider_file('a', '""');
ERROR:  function pg_tde_add_key_provider(unknown, character varying, json) does not exist
LINE 4:     SELECT pg_tde_add_key_provider('file', provider_name,
                   ^
HINT:  No function matches the given name and argument types. You might need to add explicit type casts.
QUERY:  
    -- JSON keys in the options must be matched to the keys in
    -- load_file_keyring_provider_options function.
    SELECT pg_tde_add_key_provider('file', provider_name,
                json_object('type' VALUE 'file', 'path' VALUE COALESCE(file_path, '')));

This PR fixes the issue by using BEGIN ATOMIC for SQL functions and setting the search_path for plpgsql functions. But since the plpgsql functions are not reloctable we set relocatable = false in the extension control file. If we want to make the extension relocatable I propose we do it as a separate PR.

I also did some style fixes which you are free to object to. They are as a separate commit so that can be dropped or changed if you do not like it.

@jeltz jeltz requested review from dutow and dAdAbird as code owners February 10, 2025 13:06
Copy link
Collaborator

@dutow dutow left a comment

Choose a reason for hiding this comment

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

Can you also add a testcase for it, or modify on of the existing SQL tests to create the extension in a different schema?

@jeltz
Copy link
Collaborator Author

jeltz commented Feb 11, 2025

Can you also add a testcase for it, or modify on of the existing SQL tests to create the extension in a different schema?

Sure, will add one!

This makes it easier to read the definitions, especially when bodies
are long.
This makes the SQL functions not depend on the search_path so they
are by definition relocatable. As a bonus we can more easily find bugs
since they are parsed and evaluated on creation so any type in a
function call will be known when creating the extension.
The grant functions did not support being created in a schema not in
the search_path and even after fixing this they do not support being
relocated after creation so we set "relocatable = false".
@jeltz
Copy link
Collaborator Author

jeltz commented Feb 11, 2025

@dutow A test has been added.

@jeltz jeltz merged commit e3dfba0 into percona:TDE_REL_17_STABLE Feb 11, 2025
13 checks passed
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