Any reason to have heap_(de)formtuple?

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:

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