search

aws / pg_tle

Framework for building trusted language extensions for PostgreSQL
Apache License 2.0
337 stars 30 forks source link

Relax extension_update_paths argument to text #277

Closed Rajanpandey closed 6 months ago

Rajanpandey commented 7 months ago

Issue #, if available: #126

Description of changes: All of the other extension functions use text as name, so this appears to be an arbitrary restriction. Hence, relaxing the argument from name to text.

The name data (currently in extension_update_paths()) can hold 63 characters only (ref #define NAMEDATALEN 64). Whereas text data type (updated in this PR) has no limit at all.

Note that install_extension() already uses text instead of name data type for the extension-name's argument. So a user can create an extension with names that are >63 characters, but the operation will fail when trying to find its update paths using extension_update_paths() function because it's using a data type that limits the extension name to 63 chars!

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

adamguo0 commented 7 months ago

To update the extension version you'll need to bump EXTVERSION in the Makefile as well. Not sure if that's causing these regression test failures though

Rajanpandey commented 6 months ago

Thanks @adamguo0, fixed it! 😄

adamguo0 commented 6 months ago

Sorry I haven't gotten to this in a while -- the change looks good to me. I did a manual sanity check to make sure that after updating pg_tle from 1.4.0 to 1.4.1, the function works as expected

anth0nyleung commented 6 months ago

Hi, do you mind elaborating what are the restrictions that you're facing before this change?

Rajanpandey commented 6 months ago

Hi @anth0nyleung 😃, The name data type can hold 63 characters string only (ref #define NAMEDATALEN 64). Whereas text has no limit at all!

Note that install_extension already uses text instead of name data type for the extension-name's argument. So a user can create an extension with names that are >63 characters, but the operation will fail when trying to find its update paths using extension_update_paths() function because it's using a data type that limits the extension name to 63 chars!

This PR addresses a long time pending issue, #126 raised by @jkatz. 😄

adamguo0 commented 6 months ago

As a side-note there's another issue preventing users from creating extensions with almost-63 chars. Since function names are capped at 63 chars, I don't think our current implementation can handle this case. We'll need to address that separately

https://github.com/aws/pg_tle/issues/100