Skip Menu |
 

This queue is for tickets about the BerkeleyDB CPAN distribution.

Report information
The Basics
Id: 38896
Status: resolved
Priority: 0/
Queue: BerkeleyDB

People
Owner: Nobody in particular
Requestors: NUFFIN [...] cpan.org
Cc:
AdminCc:

Bug Information
Severity: (no value)
Broken in: (no value)
Fixed in: 0.35



Subject: [PATCH] add support for DB_DBT_MULTIPLE
Download (untitled) / with headers
text/plain 517b
When associating a secondary database with a primary one, this patch will check if the skey_SV is a reference to an array and if so use it to insert multiple keys instead of just one per primary entry. The code path for scalar keys could be unified but IMHO the duplication is easier to read. If you prefer I can prepare a patch which unifies the 3 different cases to stringify in one place. BTW, have you considered using Test::More in the test suite? Twiddling the manual test counts is very time consuming.
Subject: bdb_dbt_multiple.patch
Mon Sep 1 14:57:41 IDT 2008 Yuval Kogman <nothingmuch@woobling.org> * Support for DB_DBT_MULTIPLE in secondary index callback diff -rN -Nud old-BerkeleyDB-0.34/BerkeleyDB.xs new-BerkeleyDB-0.34/BerkeleyDB.xs --- old-BerkeleyDB-0.34/BerkeleyDB.xs 2008-09-01 14:58:15.000000000 +0300 +++ new-BerkeleyDB-0.34/BerkeleyDB.xs 2008-09-01 14:58:15.000000000 +0300 @@ -1224,14 +1224,59 @@ /* retrieve the secondary key */ DBT_clear(*skey); - skey_ptr = SvPV(skey_SV, skey_len); - skey->flags = DB_DBT_APPMALLOC; - /* skey->size = SvCUR(skey_SV); */ - /* skey->data = (char*)safemalloc(skey->size); */ - skey->size = skey_len; - skey->data = (char*)safemalloc(skey_len); - memcpy(skey->data, skey_ptr, skey_len); - Trace(("key is %d -- %.*s\n", skey->size, skey->size, skey->data)); + if ( SvROK(skey_SV) ) { + SV *rv = SvRV(skey_SV); + + if ( SvTYPE(rv) == SVt_PVAV ) { + AV *av = (AV *)rv; + SV **svs = AvARRAY(av); + I32 len = av_len(av) + 1; + I32 i; + DBT *dbts; + + if ( len == 0 ) { + retval = DB_DONOTINDEX; + } else if ( len == 1 ) { + skey_ptr = SvPV(svs[0], skey_len); + skey->flags = DB_DBT_APPMALLOC; + skey->size = skey_len; + skey->data = (char*)safemalloc(skey_len); + memcpy(skey->data, skey_ptr, skey_len); + Trace(("key is %d -- %.*s\n", skey->size, skey->size, skey->data)); + } else { + skey->flags = DB_DBT_MULTIPLE | DB_DBT_APPMALLOC; + + /* FIXME this will leak if safemalloc fails later... do we care? */ + Newxz(dbts, len, DBT); + + skey->size = len; + skey->data = (char *)dbts; + + for ( i = 0; i < skey->size; i ++ ) { + skey_ptr = SvPV(svs[i], skey_len); + + dbts[i].flags = DB_DBT_APPMALLOC; + dbts[i].size = skey_len; + dbts[i].data = (char *)safemalloc(skey_len); + memcpy(dbts[i].data, skey_ptr, skey_len); + + Trace(("key is %d -- %.*s\n", dbts[i].size, dbts[i].size, dbts[i].data)); + } + Trace(("mkey has %d subkeys\n", skey->size)); + } + } else { + croak("Not an array reference"); + } + } else { + skey_ptr = SvPV(skey_SV, skey_len); + skey->flags = DB_DBT_APPMALLOC; + /* skey->size = SvCUR(skey_SV); */ + /* skey->data = (char*)safemalloc(skey->size); */ + skey->size = skey_len; + skey->data = (char*)safemalloc(skey_len); + memcpy(skey->data, skey_ptr, skey_len); + Trace(("key is %d -- %.*s\n", skey->size, skey->size, skey->data)); + } FREETMPS ; LEAVE ; diff -rN -Nud old-BerkeleyDB-0.34/t/db-3.3.t new-BerkeleyDB-0.34/t/db-3.3.t --- old-BerkeleyDB-0.34/t/db-3.3.t 2008-09-01 14:58:15.000000000 +0300 +++ new-BerkeleyDB-0.34/t/db-3.3.t 2008-09-01 14:58:15.000000000 +0300 @@ -18,7 +18,7 @@ umask(0); -print "1..130\n"; +print "1..175\n"; { # db->truncate @@ -170,8 +170,6 @@ } - # db->associate -- multiple secondary keys - # db->associate -- same again but when DB_DUP is specified. @@ -474,3 +472,128 @@ ok 130, countRecords($primary) == 1 ; } + +{ + # db->associate -- multiple secondary keys + + sub sec_key_mult + { + #print "in sec_key\n"; + my $pkey = shift ; + my $pdata = shift ; + + $_[0] = [ split ',', $pdata ] ; + return 0; + } + + my ($Dfile1, $Dfile2); + my $lex = new LexFile $Dfile1, $Dfile2 ; + my %hash ; + my $status; + my ($k, $v, $pk) = ('','',''); + + # create primary database + ok 131, my $primary = new BerkeleyDB::Hash -Filename => $Dfile1, + -Flags => DB_CREATE ; + + # create secondary database + ok 132, my $secondary = new BerkeleyDB::Hash -Filename => $Dfile2, + -Flags => DB_CREATE ; + + # associate primary with secondary + ok 133, $primary->associate($secondary, \&sec_key_mult) == 0; + + # add data to the primary + my %data = ( + "red" => "flag", + "green" => "house", + "blue" => "sea", + "foo" => "", + "bar" => "hello,goodbye", + ) ; + + my $ret = 0 ; + while (($k, $v) = each %data) { + my $r = $primary->db_put($k, $v) ; + $ret += $r; + } + ok 134, $ret == 0 ; + + # check the records in the secondary + ok 135, countRecords($secondary) == 5 ; + + ok 136, $secondary->db_get("house", $v) == 0; + ok 137, $v eq "house"; + + ok 138, $secondary->db_get("sea", $v) == 0; + ok 139, $v eq "sea"; + + ok 140, $secondary->db_get("flag", $v) == 0; + ok 141, $v eq "flag"; + + ok 142, $secondary->db_get("hello", $v) == 0; + ok 143, $v eq "hello,goodbye"; + + ok 144, $secondary->db_get("goodbye", $v) == 0; + ok 145, $v eq "hello,goodbye"; + + # pget to primary database is illegal + ok 146, $primary->db_pget('red', $pk, $v) != 0 ; + + # pget to secondary database is ok + ok 147, $secondary->db_pget('house', $pk, $v) == 0 ; + ok 148, $pk eq 'green'; + ok 149, $v eq 'house'; + + # pget to secondary database is ok + ok 150, $secondary->db_pget('hello', $pk, $v) == 0 ; + ok 151, $pk eq 'bar'; + ok 152, $v eq 'hello,goodbye'; + + ok 153, my $p_cursor = $primary->db_cursor(); + ok 154, my $s_cursor = $secondary->db_cursor(); + + # c_get from primary + $k = 'green'; + ok 155, $p_cursor->c_get($k, $v, DB_SET) == 0; + ok 156, $k eq 'green'; + ok 157, $v eq 'house'; + + # c_get from secondary + $k = 'sea'; + ok 158, $s_cursor->c_get($k, $v, DB_SET) == 0; + ok 159, $k eq 'sea'; + ok 160, $v eq 'sea'; + + # c_pget from primary database should fail + $k = 1; + ok 161, $p_cursor->c_pget($k, $pk, $v, DB_FIRST) != 0; + + # c_pget from secondary database + $k = 'flag'; + ok 162, $s_cursor->c_pget($k, $pk, $v, DB_SET) == 0; + ok 163, $k eq 'flag'; + ok 164, $pk eq 'red'; + ok 165, $v eq 'flag'; + + # check put to secondary is illegal + ok 166, $secondary->db_put("tom", "dick") != 0; + ok 167, countRecords($secondary) == 5 ; + + # delete from primary + ok 168, $primary->db_del("green") == 0 ; + ok 169, countRecords($primary) == 4 ; + + # check has been deleted in secondary + ok 170, $secondary->db_get("house", $v) != 0; + ok 171, countRecords($secondary) == 4 ; + + # delete from secondary + ok 172, $secondary->db_del('flag') == 0 ; + ok 173, countRecords($secondary) == 3 ; + + + # check deleted from primary + ok 174, $primary->db_get("red", $v) != 0; + ok 175, countRecords($primary) == 3 ; +}
On Mon Sep 01 08:00:07 2008, NUFFIN wrote: Show quoted text
> When associating a secondary database with a primary one, this patch > will check if the skey_SV > is a reference to an array and if so use it to insert multiple keys > instead of just one per primary > entry. > > The code path for scalar keys could be unified but IMHO the > duplication is easier to read. If you > prefer I can prepare a patch which unifies the 3 different cases to > stringify in one place.
The patch looks fine as it is. Interestingly this is the second patch I've received that implements DB_DBT_MULTIPLE. Of the two I prefer your's. You have checked all the edge cases, like the array reference being empty, and it it only containing one element. Show quoted text
> BTW, have you considered using Test::More in the test suite? Twiddling > the manual test counts > is very time consuming.
Yes, I have. BerkeleyDB is one of the few modules of mine that doesn't use Test::More. Been on my list of things to sort out for ages now :-) Thanks for the patch and sorry for the delay in getting back to you. cheers Paul
Download (untitled) / with headers
text/plain 720b
On Sun Sep 14 04:40:07 2008, PMQS wrote: Show quoted text
> The patch looks fine as it is. Interestingly this is the second patch > I've received that implements DB_DBT_MULTIPLE. Of the two I prefer > your's. You have checked all the edge cases, like the array reference > being empty, and it it only containing one element.
I looked at the TCL bindings to figure out how it should be done, so I blame them for any bugs it may have ;-) Show quoted text
> Yes, I have. BerkeleyDB is one of the few modules of mine that doesn't > use Test::More. Been on my list of things to sort out for ages now :-)
I might have a few more patches coming soon, will it be OK to submit them with Test::More based tests + patches to convert the existing tests? Cheers
Download (untitled) / with headers
text/plain 623b
... Show quoted text
>
> > Yes, I have. BerkeleyDB is one of the few modules of mine that
> doesn't
> > use Test::More. Been on my list of things to sort out for ages now
> :-) > > I might have a few more patches coming soon, will it be OK to submit > them with Test::More > based tests + patches to convert the existing tests?
I forgot that I had already partially converted the tests to use Test::More, so I've just finished the conversion off. That should make your life easier if you want to supply patches. I'll upload to CPAN later today or tomorrow sometime. The other new functionality included is support for sequences. Paul


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

Please report any issues with rt.cpan.org to rt-cpan-admin@bestpractical.com.