Обсуждение: 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
Zdenek Kotala <Zdenek.Kotala@Sun.COM> 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
Tom Lane napsal(a):
> Zdenek Kotala <Zdenek.Kotala@Sun.COM> 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
Zdenek Kotala <Zdenek.Kotala@Sun.COM> 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
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 Herrera http://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support
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
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 Herrera http://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support
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
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 <books@ejurka.com> 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
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
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
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
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
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
Kris Jurka <books@ejurka.com> 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