Re: [HACKERS] Any reason to have heap_(de)formtuple?
Kris Jurka [EMAIL PROTECTED] writes: On Thu, 23 Oct 2008, Kris Jurka wrote: The problem with trying to deprecate it is that the vast majority of the backend is still using the old interfaces, so people looking for inspiration for their external modules will likely end up using the old interface. Like Alvaro I started this conversion a while ago, got bored, and forgot about it. If people do want this conversion done while keeping the old interface around, I can track down that patch, update it and finish it up for the next CommitFest. Here's a patch that changes everything over to the the new API and implements the old API by calling the new API. Applied with small corrections (I caught a couple of mistakes :-(). I notice that the SPI API is still largely dependent on the 'n'/' ' convention for null flags. Now that there are not so many examples of that in the core code, I think this poses a threat of serious confusion for newbie writers of add-on modules. Does anyone want to look at cleaning that up? I suppose we'd have to do it in much the same way, adding new parallel functions and deprecating the old ones. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Any reason to have heap_(de)formtuple?
Kris Jurka napsal(a): On Thu, 23 Oct 2008, Kris Jurka wrote: The problem with trying to deprecate it is that the vast majority of the backend is still using the old interfaces, so people looking for inspiration for their external modules will likely end up using the old interface. Like Alvaro I started this conversion a while ago, got bored, and forgot about it. If people do want this conversion done while keeping the old interface around, I can track down that patch, update it and finish it up for the next CommitFest. Here's a patch that changes everything over to the the new API and implements the old API by calling the new API. It seems to me OK. I have only one comment. I prefer to pfree allocated memory for temporary nulls array. I think that caller could call old API many times without memory context cleanup. Zdenek -- Zdenek Kotala Sun Microsystems Prague, Czech Republic http://sun.com/postgresql -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Any reason to have heap_(de)formtuple?
On Tue, 28 Oct 2008, Zdenek Kotala wrote:
Kris Jurka napsal(a):
Here's a patch that changes everything over to the the new API and
implements the old API by calling the new API.
It seems to me OK. I have only one comment. I prefer to pfree allocated
memory for temporary nulls array. I think that caller could call old API
many times without memory context cleanup.
Here's an incremental patch to add the suggested pfreeing.
Kris Jurka*** a/src/backend/access/common/heaptuple.c
--- b/src/backend/access/common/heaptuple.c
***
*** 801,806 heap_formtuple(TupleDesc tupleDescriptor,
--- 801,808
tuple = heap_form_tuple(tupleDescriptor, values, boolNulls);
+ pfree(boolNulls);
+
return tuple;
}
***
*** 894,899 heap_modifytuple(HeapTuple tuple,
--- 896,902
{
int numberOfAttributes = tupleDesc-natts;
int attnum;
+ HeapTuple result;
bool *replNulls = (bool *) palloc(numberOfAttributes *
sizeof(bool));
bool *replActions = (bool *) palloc(numberOfAttributes *
sizeof(bool));
***
*** 903,909 heap_modifytuple(HeapTuple tuple,
replActions[attnum] = replCharActions[attnum] == 'r';
}
! return heap_modify_tuple(tuple, tupleDesc, replValues, replNulls,
replActions);
}
/*
--- 906,917
replActions[attnum] = replCharActions[attnum] == 'r';
}
! result = heap_modify_tuple(tuple, tupleDesc, replValues, replNulls,
replActions);
!
! pfree(replNulls);
! pfree(replActions);
!
! return result;
}
/*
***
*** 1051,1056 heap_deformtuple(HeapTuple tuple,
--- 1059,1066
charNulls[attnum] = boolNulls[attnum] ? 'n' : ' ';
}
+ pfree(boolNulls);
+
}
/*
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Any reason to have heap_(de)formtuple?
Kris Jurka napsal(a): On Tue, 28 Oct 2008, Zdenek Kotala wrote: Kris Jurka napsal(a): Here's a patch that changes everything over to the the new API and implements the old API by calling the new API. It seems to me OK. I have only one comment. I prefer to pfree allocated memory for temporary nulls array. I think that caller could call old API many times without memory context cleanup. Here's an incremental patch to add the suggested pfreeing. I think it is ready to commit now. Thanks Zdenek -- Zdenek Kotala Sun Microsystems Prague, Czech Republic http://sun.com/postgresql -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Any reason to have heap_(de)formtuple?
On Thu, 23 Oct 2008, Kris Jurka wrote: The problem with trying to deprecate it is that the vast majority of the backend is still using the old interfaces, so people looking for inspiration for their external modules will likely end up using the old interface. Like Alvaro I started this conversion a while ago, got bored, and forgot about it. If people do want this conversion done while keeping the old interface around, I can track down that patch, update it and finish it up for the next CommitFest. Here's a patch that changes everything over to the the new API and implements the old API by calling the new API. Kris Jurka use-new-heaptuple-api.patch.gz Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Any reason to have heap_(de)formtuple?
Kris Jurka napsal(a): The problem with trying to deprecate it is that the vast majority of the backend is still using the old interfaces, so people looking for inspiration for their external modules will likely end up using the old interface. Like Alvaro I started this conversion a while ago, got bored, and forgot about it. If people do want this conversion done while keeping the old interface around, I can track down that patch, update it and finish it up for the next CommitFest. Yes, Please. thanks -- Zdenek Kotala Sun Microsystems Prague, Czech Republic http://sun.com/postgresql -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Any reason to have heap_(de)formtuple?
Currently in heaptuple.c we have duplicated code. heap_deformtuple and heap_formtuple are mark as a obsolete interface. Is any reason to have still them? I know that they are still used on many places, but is there any stopper to keep these function alive? Zdenek -- Zdenek Kotala Sun Microsystems Prague, Czech Republic http://sun.com/postgresql -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Any reason to have heap_(de)formtuple?
Zdenek Kotala [EMAIL PROTECTED] writes: Currently in heaptuple.c we have duplicated code. heap_deformtuple and heap_formtuple are mark as a obsolete interface. Is any reason to have still them? I know that they are still used on many places, but is there any stopper to keep these function alive? Well, aside from the gruntwork needed to convert all the core code that still uses the old APIs, there's the prospect of breaking extension modules that still use the old APIs. It's kind of annoying to have two copies of that code, but less annoying than removing it would be ... regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Any reason to have heap_(de)formtuple?
Tom Lane napsal(a): Zdenek Kotala [EMAIL PROTECTED] writes: Currently in heaptuple.c we have duplicated code. heap_deformtuple and heap_formtuple are mark as a obsolete interface. Is any reason to have still them? I know that they are still used on many places, but is there any stopper to keep these function alive? Well, aside from the gruntwork needed to convert all the core code that still uses the old APIs, there's the prospect of breaking extension modules that still use the old APIs. It's kind of annoying to have two copies of that code, but less annoying than removing it would be ... What's about convert null array to boolean and call heap_form_tuple? Zdenek -- Zdenek Kotala Sun Microsystems Prague, Czech Republic http://sun.com/postgresql -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Any reason to have heap_(de)formtuple?
Zdenek Kotala [EMAIL PROTECTED] writes: Tom Lane napsal(a): Well, aside from the gruntwork needed to convert all the core code that still uses the old APIs, there's the prospect of breaking extension modules that still use the old APIs. It's kind of annoying to have two copies of that code, but less annoying than removing it would be ... What's about convert null array to boolean and call heap_form_tuple? Yeah, that's a thought. We'd want to be sure we'd converted any call sites that are performance-critical, but surely the vast majority are not. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Any reason to have heap_(de)formtuple?
Zdenek Kotala wrote: Tom Lane napsal(a): Well, aside from the gruntwork needed to convert all the core code that still uses the old APIs, there's the prospect of breaking extension modules that still use the old APIs. It's kind of annoying to have two copies of that code, but less annoying than removing it would be ... What's about convert null array to boolean and call heap_form_tuple? Agreed, I started doing that some time ago ... it doesn't seem all that complicated. We could try to add a #warning if an external module uses the deprecated interface, for a couple of releases, and then perhaps drop it. -- Alvaro Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Any reason to have heap_(de)formtuple?
Alvaro Herrera napsal(a): Zdenek Kotala wrote: Tom Lane napsal(a): Well, aside from the gruntwork needed to convert all the core code that still uses the old APIs, there's the prospect of breaking extension modules that still use the old APIs. It's kind of annoying to have two copies of that code, but less annoying than removing it would be ... What's about convert null array to boolean and call heap_form_tuple? Agreed, I started doing that some time ago ... it doesn't seem all that complicated. Do you have any half patch? We could try to add a #warning if an external module uses the deprecated interface, for a couple of releases, and then perhaps drop it. +1 Zdenek -- Zdenek Kotala Sun Microsystems Prague, Czech Republic http://sun.com/postgresql -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Any reason to have heap_(de)formtuple?
Zdenek Kotala wrote: Alvaro Herrera napsal(a): Agreed, I started doing that some time ago ... it doesn't seem all that complicated. Do you have any half patch? Couldn't find it here, sorry :-( -- Alvaro Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Any reason to have heap_(de)formtuple?
On Thu, 23 Oct 2008, Tom Lane wrote: Well, aside from the gruntwork needed to convert all the core code that still uses the old APIs, there's the prospect of breaking extension modules that still use the old APIs. It's kind of annoying to have two copies of that code, but less annoying than removing it would be ... The problem with trying to deprecate it is that the vast majority of the backend is still using the old interfaces, so people looking for inspiration for their external modules will likely end up using the old interface. Like Alvaro I started this conversion a while ago, got bored, and forgot about it. If people do want this conversion done while keeping the old interface around, I can track down that patch, update it and finish it up for the next CommitFest. Kris Jurka -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Any reason to have heap_(de)formtuple?
The sad thing us that I also did a patch for this and lost it. I think it wouldn't be too hard to convert all the call sites in the backend and provide a wrapper for other users. greg On 23 Oct 2008, at 08:59 PM, Kris Jurka [EMAIL PROTECTED] wrote: On Thu, 23 Oct 2008, Tom Lane wrote: Well, aside from the gruntwork needed to convert all the core code that still uses the old APIs, there's the prospect of breaking extension modules that still use the old APIs. It's kind of annoying to have two copies of that code, but less annoying than removing it would be ... The problem with trying to deprecate it is that the vast majority of the backend is still using the old interfaces, so people looking for inspiration for their external modules will likely end up using the old interface. Like Alvaro I started this conversion a while ago, got bored, and forgot about it. If people do want this conversion done while keeping the old interface around, I can track down that patch, update it and finish it up for the next CommitFest. Kris Jurka -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
