From: Howard Chu Date: Fri, 13 Jun 2003 06:09:45 +0000 (+0000) Subject: More fixes for modify - don't delete index keys that are still being used X-Git-Tag: OPENLDAP_REL_ENG_2_1_MP~884 X-Git-Url: https://git.sur5r.net/?a=commitdiff_plain;h=0c8a4b175635ffdcf79ae650a5f1e5a40c357c8a;p=openldap More fixes for modify - don't delete index keys that are still being used by other values of the attribute. Also, filter out duplicate keys to avoid redundant DB operations. Key collisions due to separate attributes indexed by supertype are still not safe. Yuck. --- diff --git a/servers/slapd/back-bdb/index.c b/servers/slapd/back-bdb/index.c index c4196495cb..271a92d40d 100644 --- a/servers/slapd/back-bdb/index.c +++ b/servers/slapd/back-bdb/index.c @@ -147,14 +147,15 @@ static int indexer( AttributeDescription *ad, struct berval *atname, BerVarray vals, + BerVarray xvals, ID id, int opid, slap_mask_t mask ) { - int rc, i; + int rc, i, j; const char *text; DB *db; - struct berval *keys; + struct berval *keys, *xkeys = NULL; void *mark; assert( mask ); @@ -174,11 +175,13 @@ static int indexer( return LDAP_OTHER; } -#if 0 /* No longer needed, our frees are in order so nothing accumulates */ mark = sl_mark(op->o_tmpmemctx); -#endif - if( IS_SLAP_INDEX( mask, SLAP_INDEX_PRESENT ) ) { + /* For a delete op, make sure we're deleting the entire + * attribute (xvals == NULL) before changing the presence + * index. xvals is only non-NULL when deleting part of an attribute. + */ + if( IS_SLAP_INDEX( mask, SLAP_INDEX_PRESENT ) && xvals == NULL ) { rc = bdb_key_change( op->o_bd, db, txn, &presence_key, id, opid ); if( rc ) { goto done; @@ -194,14 +197,39 @@ static int indexer( atname, vals, &keys, op->o_tmpmemctx ); if( rc == LDAP_SUCCESS && keys != NULL ) { + /* xvals is only provided on deletes. Generate the keys for + * xvals, representing all of the keys that will exist in + * the index when we're done. If we find a delete key that + * is in the xkeys, nullify the delete on that key. + */ + if( xvals ) { + rc = ad->ad_type->sat_equality->smr_indexer( + LDAP_FILTER_EQUALITY, mask, + ad->ad_type->sat_syntax, + ad->ad_type->sat_equality, + atname, xvals, &xkeys, + op->o_tmpmemctx ); + + for( i=0; keys[i].bv_val; i++ ) { + for( j=0; xkeys[j].bv_val != NULL; j++ ) { + if( bvmatch( &keys[i], &xkeys[j] ) ) { + keys[i].bv_len = 0; + } + } + } + } for( i=0; keys[i].bv_val != NULL; i++ ) { + /* ignore nullified keys */ + if( keys[i].bv_len == 0 ) continue; rc = bdb_key_change( op->o_bd, db, txn, &keys[i], id, opid ); - if( rc ) { - ber_bvarray_free_x( keys, op->o_tmpmemctx ); - goto done; - } + if( rc ) break; + } + if( xkeys ) { + ber_bvarray_free_x( xkeys, op->o_tmpmemctx ); + xkeys = NULL; } ber_bvarray_free_x( keys, op->o_tmpmemctx ); + if( rc ) goto done; } rc = LDAP_SUCCESS; } @@ -215,14 +243,44 @@ static int indexer( atname, vals, &keys, op->o_tmpmemctx ); if( rc == LDAP_SUCCESS && keys != NULL ) { + /* nullify duplicate keys */ + for( i=0; keys[i].bv_val; i++ ) { + if( !keys[i].bv_len ) continue; + for( j=i+1; keys[j].bv_val; j++ ) { + if( bvmatch( &keys[i], &keys[j] ) ) { + keys[j].bv_len = 0; + break; + } + } + } + if( xvals ) { + rc = ad->ad_type->sat_equality->smr_indexer( + LDAP_FILTER_APPROX, mask, + ad->ad_type->sat_syntax, + ad->ad_type->sat_approx, + atname, xvals, &xkeys, + op->o_tmpmemctx ); + + for( i=0; keys[i].bv_val; i++ ) { + for( j=0; xkeys[j].bv_val != NULL; j++ ) { + if( bvmatch( &keys[i], &xkeys[j] ) ) { + keys[i].bv_len = 0; + } + } + } + } for( i=0; keys[i].bv_val != NULL; i++ ) { + /* ignore nullified keys */ + if( keys[i].bv_len == 0 ) continue; rc = bdb_key_change( op->o_bd, db, txn, &keys[i], id, opid ); - if( rc ) { - ber_bvarray_free_x( keys, op->o_tmpmemctx ); - goto done; - } + if( rc ) break; + } + if( xkeys ) { + ber_bvarray_free_x( xkeys, op->o_tmpmemctx ); + xkeys = NULL; } ber_bvarray_free_x( keys, op->o_tmpmemctx ); + if( rc ) goto done; } rc = LDAP_SUCCESS; @@ -237,23 +295,51 @@ static int indexer( atname, vals, &keys, op->o_tmpmemctx ); if( rc == LDAP_SUCCESS && keys != NULL ) { + /* nullify duplicate keys */ + for( i=0; keys[i].bv_val; i++ ) { + if( !keys[i].bv_len ) continue; + for( j=i+1; keys[j].bv_val; j++ ) { + if( bvmatch( &keys[i], &keys[j] ) ) { + keys[j].bv_len = 0; + break; + } + } + } + if( xvals ) { + rc = ad->ad_type->sat_equality->smr_indexer( + LDAP_FILTER_SUBSTRINGS, mask, + ad->ad_type->sat_syntax, + ad->ad_type->sat_substr, + atname, xvals, &xkeys, + op->o_tmpmemctx ); + + for( i=0; keys[i].bv_val; i++ ) { + for( j=0; xkeys[j].bv_val != NULL; j++ ) { + if( bvmatch( &keys[i], &xkeys[j] ) ) { + keys[i].bv_len = 0; + } + } + } + } for( i=0; keys[i].bv_val != NULL; i++ ) { + /* ignore nullified keys */ + if ( keys[i].bv_len == 0 ) continue; bdb_key_change( op->o_bd, db, txn, &keys[i], id, opid ); - if( rc ) { - ber_bvarray_free_x( keys, op->o_tmpmemctx ); - goto done; - } + if( rc ) break; + } + if( xkeys ) { + ber_bvarray_free_x( xkeys, op->o_tmpmemctx ); + xkeys = NULL; } ber_bvarray_free_x( keys, op->o_tmpmemctx ); + if( rc ) goto done; } rc = LDAP_SUCCESS; } done: -#if 0 sl_release( mark, op->o_tmpmemctx ); -#endif return rc; } @@ -264,6 +350,7 @@ static int index_at_values( AttributeType *type, struct berval *tags, BerVarray vals, + BerVarray xvals, ID id, int opid ) { @@ -274,7 +361,7 @@ static int index_at_values( /* recurse */ rc = index_at_values( op, txn, NULL, type->sat_sup, tags, - vals, id, opid ); + vals, xvals, id, opid ); if( rc ) return rc; } @@ -287,7 +374,7 @@ static int index_at_values( if( mask ) { rc = indexer( op, txn, ad, &type->sat_cname, - vals, id, opid, + vals, xvals, id, opid, mask ); if( rc ) return rc; @@ -305,7 +392,7 @@ static int index_at_values( if( mask ) { rc = indexer( op, txn, desc, &desc->ad_cname, - vals, id, opid, + vals, xvals, id, opid, mask ); if( rc ) { @@ -322,6 +409,7 @@ int bdb_index_values( DB_TXN *txn, AttributeDescription *desc, BerVarray vals, + BerVarray xvals, ID id, int opid ) { @@ -329,7 +417,7 @@ int bdb_index_values( rc = index_at_values( op, txn, desc, desc->ad_type, &desc->ad_tags, - vals, id, opid ); + vals, xvals, id, opid ); return rc; } @@ -356,7 +444,7 @@ bdb_index_entry( /* add each attribute to the indexes */ for ( ; ap != NULL; ap = ap->a_next ) { rc = bdb_index_values( op, txn, ap->a_desc, - ap->a_nvals, e->e_id, opid ); + ap->a_nvals, NULL, e->e_id, opid ); if( rc != LDAP_SUCCESS ) { #ifdef NEW_LOGGING diff --git a/servers/slapd/back-bdb/modify.c b/servers/slapd/back-bdb/modify.c index d5db616a1f..fc7e1286f0 100644 --- a/servers/slapd/back-bdb/modify.c +++ b/servers/slapd/back-bdb/modify.c @@ -230,8 +230,10 @@ int bdb_modify_internal( switch ( ml->sml_op ) { case LDAP_MOD_DELETE: if ( ml->sml_bvalues ) { + ap = attr_find( e->e_attrs, ml->sml_desc ); rc = bdb_index_values( op, tid, ml->sml_desc, ml->sml_nvalues ? ml->sml_nvalues : ml->sml_bvalues, + ap ? ap->a_nvals : NULL, e->e_id, SLAP_INDEX_DELETE_OP ); break; } @@ -242,7 +244,7 @@ int bdb_modify_internal( ap = attr_find( save_attrs, ml->sml_desc ); if ( ap != NULL ) { rc = bdb_index_values( op, tid, ap->a_desc, - ap->a_nvals, + ap->a_nvals, NULL, e->e_id, SLAP_INDEX_DELETE_OP ); } else { rc = LDAP_SUCCESS; @@ -255,7 +257,7 @@ int bdb_modify_internal( case SLAP_MOD_SOFTADD: rc = bdb_index_values( op, tid, ml->sml_desc, ml->sml_nvalues ? ml->sml_nvalues : ml->sml_bvalues, - e->e_id, SLAP_INDEX_ADD_OP ); + NULL, e->e_id, SLAP_INDEX_ADD_OP ); break; } ml->sml_op &= ~NULLIFIED; diff --git a/servers/slapd/back-bdb/proto-bdb.h b/servers/slapd/back-bdb/proto-bdb.h index 27e10d5f4c..b7783d8819 100644 --- a/servers/slapd/back-bdb/proto-bdb.h +++ b/servers/slapd/back-bdb/proto-bdb.h @@ -294,6 +294,7 @@ bdb_index_values LDAP_P(( DB_TXN *txn, AttributeDescription *desc, BerVarray vals, + BerVarray xvals, ID id, int opid ));