Incomplete support for "delete" in all DBM modules
Paul Marquess
conform to what the man page says that "delete" should do, namely
"Returns each element so deleted or the undefined
value if there was no such element."
At the moment it does neither - the return code for DB_File is the integer
return code from the underlying Berkeley DB call that deletes entries in the
DBM file. A quick look at NDBM_File, SDBM_File, GDBM_File and ODBM_File
confirms that they do exactly the same thing.
If I remember correctly, the reason for not returning the "element so
deleted" was efficiency. To implement this would mean an extra lookup from
the DBM file. I can't remember why we don't return undef if the element to
be deleted doesn't exist.
Can anyone remember if this is correct?
I'm in two minds about providing this functionality. Basically the DBM
modules have been like that since 5.000. There is bound to be code out there
that depends on the existing "broken" functionality.
Perhaps this could be controlled by a "strict_delete" method, like this:
my $db = tie %hash, 'SDBM_File',...
$db->strict_delete(1) ;
cheers
Paul
David Nicol
Speaking as someone who has proudly implemented working
DELETE methods in hash tieing modules (see what recent DirDB
gives when you delete a directory tree) I must remind that
C<wantarray> is undefined in void context, opposed to
merely false, so the extra work can be avoided except when
requested.
About breaking hypothetical code that depends on the return values
from the underlying library calls, a way to not do that is to
leave the currently named packages completely alone, document their
broken behavior, deprecate them, and replace them with new, working
packages with slightly different names -- perhaps without the
underscores. I'm always forgetting to type them, anyway.
Another approach would be to document a migration path including
what new line noise variable gets the integer return code -- perhaps
$_{DB_RETURN_CODE}
and make people who prototyped their DB* applications in Perl but
never bothered to rewrite them in C locate all occurances of
if (delete( $tied{something}) == SomeCode){ ...
and change them to
delete( $tied{something});
if ($_{DB_RETURN_CODE} == SomeCode){ ...
I forget -- is %_ per-thread?
--
david nicol
Where the hell did I put my coffee?
Nick Ing-Simmons
>I've had a couple of requests in the last couple of weeks to make DB_File
>conform to what the man page says that "delete" should do, namely
>
> "Returns each element so deleted or the undefined
> value if there was no such element."
>
>At the moment it does neither - the return code for DB_File is the integer
>return code from the underlying Berkeley DB call that deletes entries in the
>DBM file. A quick look at NDBM_File, SDBM_File, GDBM_File and ODBM_File
>confirms that they do exactly the same thing.
>
>If I remember correctly, the reason for not returning the "element so
>deleted" was efficiency. To implement this would mean an extra lookup from
>the DBM file. I can't remember why we don't return undef if the element to
>be deleted doesn't exist.
All the *DB*_File ties were 1st implemented before we have G_VOID hook
to find out if caller was interested in the return.
David Nicol
> > Perhaps this could be controlled by a "strict_delete" method, like this:
> >
> > my $db = tie %hash, 'SDBM_File',...
> > $db->strict_delete(1) ;
> >
> > cheers
> > Paul
I'm sorry -- are you suggesting that strict_delete would return the
deletia or that strict_delete would return the implementation-specific
library return code, as a simple patch target for legacy scripts?
The crisis maintainer would have to replace
if ($code == delete($hash{1})){...
with
if ($code == tied(%hash)->strict_delete(1)){...
after upgrading their library?
Paul Marquess
>
> Paul Marquess <Paul.M...@btinternet.com> writes:
> >I've had a couple of requests in the last couple of weeks to make DB_File
> >conform to what the man page says that "delete" should do, namely
> >
> > "Returns each element so deleted or the undefined
> > value if there was no such element."
> >
> >At the moment it does neither - the return code for DB_File is
> the integer
> >return code from the underlying Berkeley DB call that deletes
> entries in the
> >DBM file. A quick look at NDBM_File, SDBM_File, GDBM_File and ODBM_File
> >confirms that they do exactly the same thing.
> >
> >If I remember correctly, the reason for not returning the "element so
> >deleted" was efficiency. To implement this would mean an extra
> lookup from
> >the DBM file. I can't remember why we don't return undef if the
> element to
> >be deleted doesn't exist.
>
> All the *DB*_File ties were 1st implemented before we have G_VOID hook
> to find out if caller was interested in the return.
Yep, thats the root cause alright.
Paul
Paul Marquess
> Speaking as someone who has proudly implemented working
> DELETE methods in hash tieing modules (see what recent DirDB
> gives when you delete a directory tree) I must remind that
> C<wantarray> is undefined in void context, opposed to
> merely false, so the extra work can be avoided except when
> requested.
>
> About breaking hypothetical code that depends on the return values
> from the underlying library calls, a way to not do that is to
> leave the currently named packages completely alone, document their
> broken behavior, deprecate them, and replace them with new, working
> packages with slightly different names -- perhaps without the
> underscores. I'm always forgetting to type them, anyway.
Hmmmm, not sure this warrants a module renaming :-)
> Another approach would be to document a migration path including
> what new line noise variable gets the integer return code -- perhaps
> $_{DB_RETURN_CODE}
> and make people who prototyped their DB* applications in Perl but
> never bothered to rewrite them in C locate all occurances of
>
> if (delete( $tied{something}) == SomeCode){ ...
>
> and change them to
>
> delete( $tied{something});
> if ($_{DB_RETURN_CODE} == SomeCode){ ...
>
> I forget -- is %_ per-thread?
Pass.
I don't think the *DB*_File modules are thread clean. I've never run DB_File
multi-threaded.
Paul
Paul Marquess
>
Sorry, I wasn't clear about my intentions.
Nope. I'm suggesting that "strict_delete" would change the behaviour of the
*DB*_File module to conform to the strict manpage definition of delete.
Given that the broken behaviour has been out in the field for so long, it
has become a de facto standard. The delete manpage (and/or the *DB*_File
modules magpages) would need updated to document the reality
Paul