Обсуждение: Bugs: Programming Language Functions
First my apologies if this is out of date. I'm working with PgSQL 6.5.2 and not the latest betas: a) through lack of time b) it's a production system, I can't really afford to be playing with a beta. Consequently the following may no longer be true and may have been fixed. However, they certainly were broken in 6.5.2. It is actually only one problem, but spreads across docs and the programming interface. Apologies also for the cross-posting! First the documentation problem. In the programmer docs, Chapter 4. Extensing SQL: Functins (page x414.htm), under the C examples, the text refers to the 'TUPLE' type. This certainly used to be the case but has now been superceeded by the type 'TupleTableSlot' This then links onto the code installation problems, as the header files in the installed include directory do not have a typedef for TupleTableSlot. This is defined in .../src/include/executor/tuptable.h Just copying this across to the installation directory causes no end of other files to be needed as well (e.g. storgae/buf.h, access/tupdesc.h, access/htup.h - and these in turn require loads of other files.....) Surely TupleTableSlot should be defined in libpq-fe.h as should the prototype for GetAttributeByName(). It would be nice to have TUPLE #defined as TupleTableSlot so that old code doesn't break!!!! Andrew -- Dr. Andrew C.R. Martin EMail: a.c.r.martin@reading.ac.uk (work) Lecturer in Bioinformatics andrew@stagleys.demon.co.uk (home) University of Reading Tel.: +44 (0)118 987 5123x7022 Fax: +44 (0)118 931 0180
"Andrew C.R. Martin" <a.c.r.martin@reading.ac.uk> writes:
> This then links onto the code installation problems, as the header
> files in the installed include directory do not have a typedef for
> TupleTableSlot. This is defined in .../src/include/executor/tuptable.h
> Just copying this across to the installation directory causes no end
> of other files to be needed as well (e.g. storgae/buf.h,
> access/tupdesc.h, access/htup.h - and these in turn require loads of
> other files.....)
Yes, this has been complained of before: trying to write an extension
that touches anything but the most basic system types requires including
a mess of files that we don't normally export ... and we don't really
*want* to export the whole set of include files. For the moment,
I'd suggest compiling with a -I pointing at a source tree rather than
the install tree. (If you're doing any serious extension development,
you surely have a source tree around for reference, so I don't think
this is totally unreasonable.)
Bruce did some work a while ago to eliminate unnecessary cross-includes,
so it could be that in 7.0 it would be easier to deal with the problem.
But really, someone needs to go through the backend header files and
figure out what makes sense to export.
> Surely TupleTableSlot should be defined in libpq-fe.h as should the
> prototype for GetAttributeByName().
Surely *not*. Neither the type nor the function exist in the frontend.
We may need to export more backend header files, but confusing backend
and frontend isn't going to help anyone.
> It would be nice to have TUPLE
> #defined as TupleTableSlot so that old code doesn't break!!!!
If that were the only thing breaking old code, I'd agree with you,
but extensions usually need to be looked at for every version anyway...
regards, tom lane
On Tue, 11 Apr 2000, Tom Lane wrote: > "Andrew C.R. Martin" <a.c.r.martin@reading.ac.uk> writes: > > This then links onto the code installation problems, as the header > > files in the installed include directory do not have a typedef for > > TupleTableSlot. This is defined in .../src/include/executor/tuptable.h > > Just copying this across to the installation directory causes no end > > of other files to be needed as well (e.g. storgae/buf.h, > > access/tupdesc.h, access/htup.h - and these in turn require loads of > > other files.....) > > Yes, this has been complained of before: trying to write an extension > that touches anything but the most basic system types requires including > a mess of files that we don't normally export ... and we don't really > *want* to export the whole set of include files. For the moment, > I'd suggest compiling with a -I pointing at a source tree rather than > the install tree. (If you're doing any serious extension development, > you surely have a source tree around for reference, so I don't think > this is totally unreasonable.) yup, that's exactly what I did, but the last version of PgSQL I worked with used TUPLE rather than TupleTableSlot, although I had to give my own prototype for GetAttributeByName(). The previous version I worked with gave my both the typedef and the prototype :-) I'm certainly not doing anything that I would call a serious amount of extension development - just one simple function which used to work by following the simple instructions in the docs and no longer does. The ability to add this type of extension to PgSQL has always been one of it's major plus points and is one of the reasons I've been using it since the Postgres95 days. > > Bruce did some work a while ago to eliminate unnecessary cross-includes, > so it could be that in 7.0 it would be easier to deal with the problem. > But really, someone needs to go through the backend header files and > figure out what makes sense to export. > > > Surely TupleTableSlot should be defined in libpq-fe.h as should the > > prototype for GetAttributeByName(). > > Surely *not*. Neither the type nor the function exist in the frontend. > We may need to export more backend header files, but confusing backend > and frontend isn't going to help anyone. OK, sorry that was a bad choice of header file, but it is, I'm pretty sure, where GetAttributeByName() *used* to be defined. I just feel there should be some nice easy header file for people to include who want to write a simple C function which can access data from the database and return a valid type. > > > It would be nice to have TUPLE > > #defined as TupleTableSlot so that old code doesn't break!!!! > > If that were the only thing breaking old code, I'd agree with you, > but extensions usually need to be looked at for every version anyway... > Depends on the complexity of the extension :-) If it's something which only has to pull values out and return a value then this is likely to be the only problem. My own particular problem simply pulls out an integer and a (text *) from a varchar. It then goes away and does some calculations based on these values and another external file (in fact it does a system() to run an external program which does these calculations). It then build a 'text' structure containing the result. With the exception of the prototypes and the typedef for TUPLE this has worked without change since Postgres95. Andrew -- Dr. Andrew C.R. Martin EMail: a.c.r.martin@reading.ac.uk (work) Lecturer in Bioinformatics andrew@stagleys.demon.co.uk (home) University of Reading Tel.: +44 (0)118 987 5123x7022 Fax: +44 (0)118 931 0180
"Andrew C.R. Martin" <a.c.r.martin@reading.ac.uk> writes:
> My own particular problem simply pulls out an integer and a (text *)
> from a varchar. It then goes away and does some calculations based on
> these values and another external file (in fact it does a system() to
> run an external program which does these calculations). It then build
> a 'text' structure containing the result.
Hm. I'm not too clear on why that would need to worry about either
TupleTableSlot or GetAttributeByName ... shouldn't it be a simple
function consuming a text Datum and producing another?
regards, tom lane
On Tue, 11 Apr 2000, Tom Lane wrote:
> "Andrew C.R. Martin" <a.c.r.martin@reading.ac.uk> writes:
> > My own particular problem simply pulls out an integer and a (text *)
> > from a varchar. It then goes away and does some calculations based on
> > these values and another external file (in fact it does a system() to
> > run an external program which does these calculations). It then build
> > a 'text' structure containing the result.
>
> Hm. I'm not too clear on why that would need to worry about either
> TupleTableSlot or GetAttributeByName ... shouldn't it be a simple
> function consuming a text Datum and producing another?
Errrmmm, just following the instructions in the docs:
Programmer's Manual, Chapter 4. Extensing SQL: Functins (page x414.htm), under
the C examples.
My function looks something like:
text *mppfunc(TUPLE tuple)
{ text *attrib; int resnum; bool isnull; char my_result[400];
if(((attrib = (text *)GetAttributeByName(tuple, "substitution", &isnull)) ==NULL) || isnull)
return(pg_text("Invalid1")); strncpy(newres, VARDATA(attrib), 40);
resnum = (int)GetAttributeByName(tuple, "codon", &isnull); if(isnull) return(pg_text("Invalid3"));
/* Do some stuff here putting a result string in my_result */ return(pg_text(my_result));
}
pg_text() is a little function to put a string into a PostgreSQL text data type.
Are you suggesting there is now an undocumented(?) easier way of doing this???
Best wishes,
Andrew
--
Dr. Andrew C.R. Martin EMail: a.c.r.martin@reading.ac.uk (work)
Lecturer in Bioinformatics andrew@stagleys.demon.co.uk (home)
University of Reading
Tel.: +44 (0)118 987 5123x7022 Fax: +44 (0)118 931 0180
"Andrew C.R. Martin" <a.c.r.martin@reading.ac.uk> writes:
>> Hm. I'm not too clear on why that would need to worry about either
>> TupleTableSlot or GetAttributeByName ... shouldn't it be a simple
>> function consuming a text Datum and producing another?
> Errrmmm, just following the instructions in the docs:
> Programmer's Manual, Chapter 4. Extensing SQL: Functins (page
> x414.htm), under the C examples.
OK, I see. You've got a function that accepts an entire tuple and
has hard-wired assumptions about which fields to extract from the tuple.
Certainly there are cases where that's what's needed, but I'd be
inclined to define the function as taking two parameters int4 and text*,
then calling it as myfunc(table.codon, table.substitution). That pushes
the need for tuple field access out of your code.
That doesn't affect the validity of your point, of course. We have done
wholesale renamings of backend types, fields, and functions, as part of
an ongoing effort to clean up the code; and I expect there will be more
in future. Perhaps we should pay more attention to not breaking user
functions unnecessarily when we do this.
regards, tom lane
On Tue, 11 Apr 2000, Tom Lane wrote: > "Andrew C.R. Martin" <a.c.r.martin@reading.ac.uk> writes: > >> Hm. I'm not too clear on why that would need to worry about either > >> TupleTableSlot or GetAttributeByName ... shouldn't it be a simple > >> function consuming a text Datum and producing another? > > > Errrmmm, just following the instructions in the docs: > > > Programmer's Manual, Chapter 4. Extensing SQL: Functins (page > > x414.htm), under the C examples. > > OK, I see. You've got a function that accepts an entire tuple and > has hard-wired assumptions about which fields to extract from the tuple. > Certainly there are cases where that's what's needed, but I'd be > inclined to define the function as taking two parameters int4 and text*, > then calling it as myfunc(table.codon, table.substitution). That pushes > the need for tuple field access out of your code. Yes that's a good point. I guess 4 years or so ago when I first wrote this bit of code, I simply followed the instructions in the manual and didn't think around other ways of doing things > > That doesn't affect the validity of your point, of course. We have done > wholesale renamings of backend types, fields, and functions, as part of > an ongoing effort to clean up the code; and I expect there will be more > in future. Perhaps we should pay more attention to not breaking user > functions unnecessarily when we do this. Not so much that, but we should make sure that the docs keep pace with such changes. That to me is more important! I don't mind too much if the user functions have to be changed, but I'd like the docs to tell me what to do and things such as this which have been supported previously just from the .../include path (rather than .../src/include) should remain so :-) Can we make sure that this becomes a formal item in the TODO lists for code and docs?? Best wishes, Andrew -- Dr. Andrew C.R. Martin EMail: a.c.r.martin@reading.ac.uk (work) Lecturer in Bioinformatics andrew@stagleys.demon.co.uk (home) University of Reading Tel.: +44 (0)118 987 5123x7022 Fax: +44 (0)118 931 0180
On Thu, 13 Apr 2000, Andrew C.R. Martin wrote: > On Tue, 11 Apr 2000, Tom Lane wrote: > > "Andrew C.R. Martin" <a.c.r.martin@reading.ac.uk> writes: > > >> Hm. I'm not too clear on why that would need to worry about either > > >> TupleTableSlot or GetAttributeByName ... shouldn't it be a simple > > >> function consuming a text Datum and producing another? > > > > > Errrmmm, just following the instructions in the docs: > > > > > Programmer's Manual, Chapter 4. Extensing SQL: Functins (page > > > x414.htm), under the C examples. > > Eeek! Forget my idea of #define'ing TupleTableSlot as TUPLE for backwards compatibility. I've just realised that GetAttributeByName() now takes a TupleTableSlot *pointer* whereas it used to take a TUPLE structure (not a pointer). Personally whenever I make a code change like this I try to create a new function with the altered parameters and then make a wrapper with the old name and calling style for backwards compatibility :-) It seems that the .../src/tutorial/func.* examples have been updated correctly The create_function man page however has not been updated (it even refers to libpq-fe.h still as well as to TUPLE) and the main printable manual needs to be updated. Best wishes, Andrew -- Dr. Andrew C.R. Martin EMail: a.c.r.martin@reading.ac.uk (work) Lecturer in Bioinformatics andrew@stagleys.demon.co.uk (home) University of Reading Tel.: +44 (0)118 987 5123x7022 Fax: +44 (0)118 931 0180