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

[EARLY WIP] PG_FUNCTION_INFO_V2 #2

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

yrashk
Copy link

@yrashk yrashk commented Jul 18, 2024

PG_FUNCTION_INFO_V1 did quite well for Postgres over the years.

However, it has issues:

The safety of our extensions often relies on being defensive about the types of arguments the functions receive. A Postgres user with sufficient privileges can create a function with any parameters/return signature calling our native function. Unless that function inspects FunctionCallInfo deeply (which comes at a non-zero cost), all bets are off in this case, and a crash or other malfunction can occur.

Sometimes, it would be meaningful to select a different function implementation based on some load-time parameters – either based on the host system or on something coming from the database. In the case of more optimal generated code specific to CPUs, function multiversioning can alleviate this (subject to limitations of multiversioning, but for all other cases, there's no straightforward way to pick the right function when we load it.

PG_FUNCTION_INFO_V2 defines pg_finfo_FUNCTION_NAME symbol. Some languages make it much harder to construct names like this. This also makes it much harder to list all exported names (we need to find symbols!)

This is less critical than the ones above, but it is still a concern.

Linked discussion: https://github.com/orgs/pgedc/discussions/38


This change introduces a way to list all functions ahead of time, with their parameters.

PG_FUNCTION_INFO_V1 did quite well for Postgres over the years.

However, it has issues:

The safety of our extensions often relies on being defensive about the
types of arguments the functions receive. A Postgres user with
sufficient privileges can create a function with any parameters/return
signature calling our native function. Unless that function inspects
FunctionCallInfo deeply (which comes at a non-zero cost), all bets are
off in this case, and a crash or other malfunction can occur.

Sometimes, it would be meaningful to select a different function
implementation based on some load-time parameters – either based on the
host system or on something coming from the database. In the case of
more optimal generated code specific to CPUs, function multiversioning
can alleviate this (subject to limitations of multiversioning, but for
all other cases, there's no straightforward way to pick the right
function when we load it.

`PG_FUNCTION_INFO_V2` defines `pg_finfo_FUNCTION_NAME` symbol. Some
languages make it much harder to construct names like this. This also
makes it much harder to list all exported names (we need to find
symbols!)

This is less critical than the ones above, but it is still a concern.

---

This change introduces a way to list all functions ahead of time, with
their parameters.
@danolivo
Copy link

Interesting direction. I personally have two problems with UI routines of extensions:

  1. shlib binaries newer than functions in the database (it's ok)
  2. shlib binaries older than functions in the database.
    I want to do something meaningful in both cases, not just blow up the backend in segfault.
    Right now, maintaining it costs me additional coding (remember, different databases in the cluster can have different versions of extension UI). Sometimes I can bear legacy burden of old function implementations, sometimes not.
    It would be nice to transfer this responsibility to the core.

Generally, it may make sense to discover your cases (maybe someone else too) and some sketch implementation that could show how new call convention can resolve this issue.

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