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

Feat: add PG_FUNCTION_V1_RAW #34

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

angaz
Copy link
Contributor

@angaz angaz commented Mar 23, 2024

PG_FUNCTION_V1_RAW is for registering a standard Postgres V1 funcion, without any type conversions.

This will come in handy for functions where we don't have a pgzx abstraction for the types yet.

PG_FUNCTION_V1_RAW is for registering a standard
Postgres V1 funcion, without any type conversions.
@angaz
Copy link
Contributor Author

angaz commented Mar 24, 2024

I might have found a better way adding FunctionCallInfo, and other types to directMappings. But this might still be useful for types not yet provided.

@urso
Copy link
Collaborator

urso commented Mar 25, 2024

Thank you for contributing.

I'm just not sure I understand why we need the PR. The pgzx.fmgr.args module wraps the type conversion and has a special case for FunctionCallInfo. See: https://github.com/xataio/pgzx/blob/main/src/pgzx/fmgr/args.zig#L48

That is it should be possible to write this already:

const pgzx = @import("pgzx");
const pg = pgzx.c;

comptime {
  PG_FUNCTION_V1("my_func", my_func);
}

fn my_func(fcinfo pg.FunctionCallInfo) c.Datum {
  ...
}

Unfortunately we don't have a test case (sample extension) doing this as of today. But if that pattern does not work I would consider this a bug.

Assuming that this functions, do we still need PG_FUNCTION_V1_RAW ?

@angaz
Copy link
Contributor Author

angaz commented Mar 25, 2024

Yeah, this was my first approach, and I understand the code a lot more now. So I don't think this won't be needed.

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.

None yet

2 participants