-
Notifications
You must be signed in to change notification settings - Fork 13
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
base: main
Are you sure you want to change the base?
Conversation
@@ -4,6 +4,7 @@ const includes = @cImport({ | |||
@cInclude("postgres.h"); | |||
@cInclude("postgres_ext.h"); | |||
|
|||
@cInclude("access/hash.h"); |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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); |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
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.