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

add FunctionCallInfo, SortSupport, StringInfo #35

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

angaz
Copy link
Contributor

@angaz angaz commented Mar 24, 2024

Add a few PG C data types. This should be all that is needed to create a custom type extension.

I would like to add a uint example extension at a later date to copy the functionality found in https://github.com/petere/pguint to show how these functions can be used.

@@ -4,6 +4,7 @@ const includes = @cImport({
@cInclude("postgres.h");
@cInclude("postgres_ext.h");

@cInclude("access/hash.h");
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can't tell how this file is sorted, so I just put the new import at the top.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I might have put it after varatt.h :). But feel free to keep it here.

pub const PGDatum = ConvNoFail(c.Datum, idDatum, idDatum);
pub const PGFunctionCallInfo = ConvID(c.FunctionCallInfo);
pub const PGSortSupport = ConvID(c.SortSupport);
pub const PGStringInfo = Conv(c.StringInfo, datumGetStringInfo, c.PointerGetDatum);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I see why we want these types. But I don't think we should add them to datum conversions 🤔

We already have support to capture FunctionCallInfo via pgzx.fmgr.args module. Maybe that would be a better place for these argument types?

This and the other PRs also make me wonder if we rather want to introduce different function types. Right now we have PG_FUNCTION_V1, which can be any function. But maybe it would make sense to also introduce PG_OUT_FUNCTION or PG_SORT_FUNCTION exporters. These function types would ensure the correct number of argument and pass SortSupport or StringInfo right away.

For SortSupport one needs to functions like:

static int
btfloat8fastcmp(Datum x, Datum y, SortSupport ssup)
{
	float8		arg1 = DatumGetFloat8(x);
	float8		arg2 = DatumGetFloat8(y);

	return float8_cmp_internal(arg1, arg2);
}

Datum
btfloat8sortsupport(PG_FUNCTION_ARGS)
{
	SortSupport ssup = (SortSupport) PG_GETARG_POINTER(0);

	ssup->comparator = btfloat8fastcmp;
	PG_RETURN_VOID();
}

Using an "abstraction" in Zig this might become:

comptime {
  pgzx.PG_SORT_SUPPORT("btfloat8sortsupport", .{
    .comparator = btfloat8fastcmp,
  })
}

Maybe it is worth to open an issue to discuss functionality that you are missing for the kind of work you are planning. Then we can discuss what needs to be adapted where, or if it makes sense to introduce abstractions. WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok. I didn't know about the fmgr.args module. I'll have to check it out. I like the idea of different types of functions. It would simplify the definition boilerplate.

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