Re: user defined type

Hello

I am writing my own type for use in postgres. I want to be able to save a
value - an integer, and a string in this type. I have looked at the
documentation for how to do this. The insert into the type works, but when i
try to select all from the table, postgres crashes. I don't see what's wrong,
so I hope someone can help.

thanks
-Kjetil

the code:
typedef struct alignres {
int value;
char *fstring;
}alignres;
PG_FUNCTION_INFO_V1(alignres_in);

Datum alignres_in(PG_FUNCTION_ARGS) {
char *in = PG_GETARG_CSTRING(0);

int v;
char *first;

alignres *result;

if(sscanf(in, "(%d, %s)", &v, &first) != 2) {
ereport(ERROR, (errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
errmsg("invalid input type for alignment: \"%s\"", in)));
}
result = (alignres *) palloc(sizeof(alignres));
result->value = v;
result->fstring = first;

PG_RETURN_POINTER(result);
}

PG_FUNCTION_INFO_V1(alignres_out);

Datum alignres_out(PG_FUNCTION_ARGS) {
alignres *align = (alignres *) PG_GETARG_POINTER(0);
char *result;

char temp[4+sizeof(align->fstring)+4+4];

sprintf(temp, "(%d, %s)",
align->value,
align->fstring);

result = (char*) palloc(strlen(temp)+1);
strcpy(result, temp);
PG_RETURN_CSTRING(result);
}

On Mon, Nov 08, 2004 at 01:26:55PM +0100, Kjetil Haaland wrote:

> typedef struct alignres {
> int value;
> char *fstring;
> }alignres;

If you want to store character data, I think the characters must
be stored in the structure itself; I don't think using a pointer
will work (somebody please correct me if I'm mistaken). So instead
of "char *fstring" you'll need "char fstring[x]", where "x" is the
maximum size of the character data. You could also use "char fstring[1]"
and allocate the alignres structure to be as big as necessary; if
you do that then you might want to review the paragraph regarding
TOAST-able types near the end of the "User-Defined Types" section
of the documentation.

http://www.postgresql.org/docs/7.4/static/xtypes.html

Make sure your CREATE TYPE statement has "internallength" set
correctly: either to the fixed size of the structure (including
character data) or to "variable". If you use "variable" then make
sure you add the required length field to the beginning of the
structure. See the CREATE TYPE documentation for more info.

http://www.postgresql.org/docs/7.4/static/sql-createtype.html

> if(sscanf(in, "(%d, %s)", &v, &first) != 2) {

The above line has several problems:

* For a %s conversion, sscanf() expects a pointer to character;
earlier you declared first as a char *, so you're passing a pointer
to a pointer to character. sscanf() also expects the pointer to
point to a buffer sufficiently large to hold the result; you haven't
allocated any space.

* %s is greedy, so it'll grab the closing parenthesis in addition
to what precedes it. Consider using %[ instead or make sure you
strip extraneous characters from the end of the result string.

* Your %s conversion doesn't specify a maximum field width, so
you risk a buffer overflow if the result buffer isn't at least
as large as the input string.

Additionally, consider using %[ instead of %d for the first field
and store the result in a character array so you can do better
validity checking than sscanf(). For example, you could then use
strtol() to check for overflow -- with sscanf() you'd just get bogus
numbers that don't match the input.

> char temp[4+sizeof(align->fstring)+4+4];

This line in your output function might not be allocating as much
space as you think. The sizeof expression evaluates to the size
of a pointer, not to the length of the data pointed to. You're
probably allocating about 16 bytes here, which might be less
than you need.

> sprintf(temp, "(%d, %s)",
> align->value,
> align->fstring);

By using sprintf() instead of snprintf() you again risk a buffer
overflow, especially since temp might be smaller than you were
expecting.

There might be other problems but those stood out when I looked
through the code. If you still have trouble then please post
the updated code and describe what happens when you try to use
the type.

Hope this helps.

--
Michael Fuhr
http://www.fuhr.org/~mfuhr/

On Monday 08 November 2004 22:24, Michael Fuhr wrote:
> On Mon, Nov 08, 2004 at 01:26:55PM +0100, Kjetil Haaland wrote:
> > typedef struct alignres {
> > int value;
> > char *fstring;
> > }alignres;
>
> If you want to store character data, I think the characters must
> be stored in the structure itself; I don't think using a pointer
> will work (somebody please correct me if I'm mistaken). So instead
> of "char *fstring" you'll need "char fstring[x]", where "x" is the
> maximum size of the character data. You could also use "char fstring[1]"
> and allocate the alignres structure to be as big as necessary; if
> you do that then you might want to review the paragraph regarding
> TOAST-able types near the end of the "User-Defined Types" section
> of the documentation.
>
> http://www.postgresql.org/docs/7.4/static/xtypes.html
>
> Make sure your CREATE TYPE statement has "internallength" set
> correctly: either to the fixed size of the structure (including
> character data) or to "variable". If you use "variable" then make
> sure you add the required length field to the beginning of the
> structure. See the CREATE TYPE documentation for more info.

Thanks for the answer. There are some stuff i don't understand. I want to use
the fstring[1] way, because i don't know how large my string will be. You say
that i can allocate the hole structure, where should i do this? in the
in-data-function? It also says in the documentation that if I use the random
for internallength when i create my type I have to set the length in the
structure. Is this set in the declaration of the structure or in some of the
methods?

-Kjetil

On Tue, Nov 09, 2004 at 01:38:29PM +0100, Kjetil Haaland wrote:
> On Monday 08 November 2004 22:24, Michael Fuhr wrote:
> > On Mon, Nov 08, 2004 at 01:26:55PM +0100, Kjetil Haaland wrote:
> > > typedef struct alignres {
> > > int value;
> > > char *fstring;
> > > }alignres;
> >
> > If you want to store character data, I think the characters must
> > be stored in the structure itself; I don't think using a pointer
> > will work (somebody please correct me if I'm mistaken). So instead
> > of "char *fstring" you'll need "char fstring[x]", where "x" is the
> > maximum size of the character data. You could also use "char fstring[1]"
> > and allocate the alignres structure to be as big as necessary; if
> > you do that then you might want to review the paragraph regarding
> > TOAST-able types near the end of the "User-Defined Types" section
> > of the documentation.
> >
> > http://www.postgresql.org/docs/7.4/static/xtypes.html
> >
> > Make sure your CREATE TYPE statement has "internallength" set
> > correctly: either to the fixed size of the structure (including
> > character data) or to "variable". If you use "variable" then make
> > sure you add the required length field to the beginning of the
> > structure. See the CREATE TYPE documentation for more info.

> I want to use the fstring[1] way, because i don't know how large
> my string will be. You say that i can allocate the hole structure,
> where should i do this? in the in-data-function?

Yes, in alignres_in() you'd allocate enough memory to hold all of
the data: the size of the structure plus the length of the string.
If you don't mind a little waste then here's a simple way to do it:

char *in = PG_GETARG_CSTRING(0);
alignres *result;

result = palloc(sizeof(*result) + strlen(in));

(Note that the compiler evalutes sizeof(*result), so you don't have
to worry that you're dereferencing an unassigned pointer at run
time. This syntax removes the need to specify the type name again,
which eases maintenance.)

I don't think you need to add an extra byte to allow for the string's
terminating NUL because sizeof(*result) already accounts for one
byte of the string (and probably a little extra, due to alignment).
Double-check my thinking, though, and add an extra byte if you think
it's necessary.

> It also says in the documentation that if I use the random for
> internallength when i create my type I have to set the length in
> the structure. Is this set in the declaration of the structure or
> in some of the methods?

The internallength specification is "variable", not "random." The
length is an int32 at the beginning of the data:

typedef struct alignres {
int32 length;
int value;
char fstring[1];
} alignres;

Before returning, set the length field to the total size of the data.
Here's where I think you could eliminate any wasted space that you
allocated -- after you've parsed the data you know exactly how long
fstring is, so you could add only that much to sizeof(*result):

result->length = sizeof(*result) + strlen(fstring);

Again, I don't think you need to add an extra byte to account for
the string's terminating NUL because sizeof(*result) already includes
one byte for the string.

--
Michael Fuhr
http://www.fuhr.org/~mfuhr/

hello again

Thanks to you i have no managed to store a value and a string of variable
length in the structure. I thought it would be easy to expand it to two
strings and a value, by doing it the same way, but that didn't work out. When
i insert something into the type this is what happends:

1) the first string in the structure has one character( or the size of the
table that holds the first string) from the first string i gave, a blank
space and the second string i gave.
2)the second string is correct - has the second string i gave.

Is there anyway to have two or more strings in one type or do i have to save
them both in one string? This is my type and the in function so far:

typedef struct alignres {
int32 length;
int value;
char fstring[1];
char sstring[1];
}alignres;

Datum alignres_in(PG_FUNCTION_ARGS) {
char *in = PG_GETARG_CSTRING(0);
char *workstr = pstrdup(in);
char *svalue = NULL;
char *first = NULL;
char *second = NULL;

svalue = strtok(workstr, ", ");
first = strtok(NULL, ", ");
second = strtok(NULL, ")");

int value = atoi(++svalue);
alignres *result;
result = (alignres *) palloc(sizeof(*result)+strlen(in));

result->value = value;
strcpy(result->fstring, first);
strcpy(result->sstring, second);
result->length=sizeof(*result)+strlen(result->fstring)+strlen(result->sstring);

PG_RETURN_POINTER(result);
}

-Kjetil

On Wed, Nov 10, 2004 at 02:37:49PM +0100, Kjetil Haaland wrote:

> Is there anyway to have two or more strings in one type or do i have to save
> them both in one string?

PostgreSQL needs all the data in a single buffer. If you want to
work with multiple variable-length fields, then you could define a
structure that's easy for your code to work with and serialize it
before returning it to PostgreSQL, or one that uses pointers that
point to the correct places in the buffer. Your alignres_out()
function will need to reverse that effect so it can work with the
data that PostgreSQL passes it.

--
Michael Fuhr
http://www.fuhr.org/~mfuhr/

On Wednesday 10 November 2004 19:30, Michael Fuhr wrote:
> On Wed, Nov 10, 2004 at 02:37:49PM +0100, Kjetil Haaland wrote:
> > Is there anyway to have two or more strings in one type or do i have to
> > save them both in one string?
>
> PostgreSQL needs all the data in a single buffer. If you want to
> work with multiple variable-length fields, then you could define a
> structure that's easy for your code to work with and serialize it
> before returning it to PostgreSQL, or one that uses pointers that
> point to the correct places in the buffer. Your alignres_out()
> function will need to reverse that effect so it can work with the
> data that PostgreSQL passes it.

Hello again
Thanks a lot for all the help!
I have one more question about user defined types. When i create a char
pointer in the alignres_out (the out function of the type) function why is it
always default "alignres_out" ? Is it supposed to be that or have i done
something wrong?

an example:
Datum alignres_out(PG_FUNCTION_ARGS) {
char *result;
elog(NOTICE, "result=%s", result);
}
This will give me NOTICE: result=alignres_out.
It is not just in the output function this happens, it is the same in the
input - alignres_in gives default alignres_in.

- Kjetil

Kjetil Haaland <kjetil(dot)haaland(at)student(dot)uib(dot)no> writes:
> an example:
> Datum alignres_out(PG_FUNCTION_ARGS) {
> char *result;
> elog(NOTICE, "result=%s", result);
> }

This code is wrong on its face. If your compiler doesn't give a warning
along the lines of "use of uninitialized value", get a better compiler.
(Note: when using gcc I think you must specify -O to get this warning.
I always use at least -O -Wall when developing with gcc.)

> This will give me NOTICE: result=alignres_out.

Pure luck that it doesn't crash instead. Apparently you're picking up a
value that happened to be left in a register by the function-call
manager, but on another machine or after any slight mod to fmgr.c that
register might contain something that's not a pointer to string at all.

regards, tom lane

On Friday 19 November 2004 16:16, Tom Lane wrote:
> This code is wrong on its face. If your compiler doesn't give a warning
> along the lines of "use of uninitialized value", get a better compiler.
> (Note: when using gcc I think you must specify -O to get this warning.
> I always use at least -O -Wall when developing with gcc.)
>
> > This will give me NOTICE: result=alignres_out.
>
> Pure luck that it doesn't crash instead. Apparently you're picking up a
> value that happened to be left in a register by the function-call
> manager, but on another machine or after any slight mod to fmgr.c that
> register might contain something that's not a pointer to string at all.
>
> regards, tom lane

Ok, the example wasnt complete, i just tried to show what i meant, so i try
again with the hole example. Now i use the \O and it gives no warnings.

Datum alignres_out(PG_FUNCTION_ARGS) {
alignres *align = (alignres *) PG_GETARG_POINTER(0);
char *result = NULL;
int secondStart=align->secondString;
elog(NOTICE, "secondStart=%d", secondStart);
char temp[4+strlen(align->stringBuffer)+4+4];
char tempBuffer[strlen(align->stringBuffer)+1];

char *first = NULL;
elog(NOTICE, "1:first=%s", first);
first = palloc(sizeof(char)*secondStart);
elog(NOTICE, "2:first=%s", first);
elog(NOTICE, "strlen(first)=%d", strlen(first));

snprintf(tempBuffer, sizeof(tempBuffer), "%s", align->stringBuffer);
first = strncpy(first, tempBuffer, secondStart-1);
snprintf(temp, sizeof(temp), "(%d, %s)",
align->value,
first);
result = (char*) palloc(strlen(temp)+1);
strcpy(result, temp);
PG_RETURN_CSTRING(result);
}

On the first elog i get (null) and thats correct. After the allocation i get
alignres_out. When i print out the variable secondString it is 5, but when i
print out the length of the first it is set to 12, the length of
"alignres_out".

Am i still doing something wrong? If so, how am i supposed to do it to get it
correct?

-Kjetil

Kjetil Haaland <kjetil(dot)haaland(at)student(dot)uib(dot)no> writes:
> first = palloc(sizeof(char)*secondStart);
> elog(NOTICE, "2:first=%s", first);
> elog(NOTICE, "strlen(first)=%d", strlen(first));

This is bogus because you've only allocated space, not filled it with
anything. It will contain whatever that RAM was last used for.

regards, tom lane