From: Howard Chu <hyc@openldap.org>
Date: Sun, 21 Sep 2003 10:11:46 +0000 (+0000)
Subject: Revert broken index optimizations
X-Git-Tag: OPENLDAP_REL_ENG_2_1_MP~690
X-Git-Url: https://git.sur5r.net/?a=commitdiff_plain;h=fddbe13c6eff4ddf099bd423e4e65d970b0de003;p=openldap

Revert broken index optimizations
---

diff --git a/servers/slapd/back-bdb/index.c b/servers/slapd/back-bdb/index.c
index 271a92d40d..c4196495cb 100644
--- a/servers/slapd/back-bdb/index.c
+++ b/servers/slapd/back-bdb/index.c
@@ -147,15 +147,14 @@ static int indexer(
 	AttributeDescription *ad,
 	struct berval *atname,
 	BerVarray vals,
-	BerVarray xvals,
 	ID id,
 	int opid,
 	slap_mask_t mask )
 {
-	int rc, i, j;
+	int rc, i;
 	const char *text;
 	DB *db;
-	struct berval *keys, *xkeys = NULL;
+	struct berval *keys;
 	void *mark;
 
 	assert( mask );
@@ -175,13 +174,11 @@ 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
 
-	/* 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 ) {
+	if( IS_SLAP_INDEX( mask, SLAP_INDEX_PRESENT ) ) {
 		rc = bdb_key_change( op->o_bd, db, txn, &presence_key, id, opid );
 		if( rc ) {
 			goto done;
@@ -197,39 +194,14 @@ 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 ) break;
-			}
-			if( xkeys ) {
-				ber_bvarray_free_x( xkeys, op->o_tmpmemctx );
-				xkeys = NULL;
+				if( rc ) {
+					ber_bvarray_free_x( keys, op->o_tmpmemctx );
+					goto done;
+				}
 			}
 			ber_bvarray_free_x( keys, op->o_tmpmemctx );
-			if( rc ) goto done;
 		}
 		rc = LDAP_SUCCESS;
 	}
@@ -243,44 +215,14 @@ 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 ) break;
-			}
-			if( xkeys ) {
-				ber_bvarray_free_x( xkeys, op->o_tmpmemctx );
-				xkeys = NULL;
+				if( rc ) {
+					ber_bvarray_free_x( keys, op->o_tmpmemctx );
+					goto done;
+				}
 			}
 			ber_bvarray_free_x( keys, op->o_tmpmemctx );
-			if( rc ) goto done;
 		}
 
 		rc = LDAP_SUCCESS;
@@ -295,51 +237,23 @@ 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 ) break;
-			}
-			if( xkeys ) {
-				ber_bvarray_free_x( xkeys, op->o_tmpmemctx );
-				xkeys = NULL;
+				if( rc ) {
+					ber_bvarray_free_x( keys, op->o_tmpmemctx );
+					goto done;
+				}
 			}
 			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;
 }
 
@@ -350,7 +264,6 @@ static int index_at_values(
 	AttributeType *type,
 	struct berval *tags,
 	BerVarray vals,
-	BerVarray xvals,
 	ID id,
 	int opid )
 {
@@ -361,7 +274,7 @@ static int index_at_values(
 		/* recurse */
 		rc = index_at_values( op, txn, NULL,
 			type->sat_sup, tags,
-			vals, xvals, id, opid );
+			vals, id, opid );
 
 		if( rc ) return rc;
 	}
@@ -374,7 +287,7 @@ static int index_at_values(
 
 	if( mask ) {
 		rc = indexer( op, txn, ad, &type->sat_cname,
-			vals, xvals, id, opid,
+			vals, id, opid,
 			mask );
 
 		if( rc ) return rc;
@@ -392,7 +305,7 @@ static int index_at_values(
 
 		if( mask ) {
 			rc = indexer( op, txn, desc, &desc->ad_cname,
-				vals, xvals, id, opid,
+				vals, id, opid,
 				mask );
 
 			if( rc ) {
@@ -409,7 +322,6 @@ int bdb_index_values(
 	DB_TXN *txn,
 	AttributeDescription *desc,
 	BerVarray vals,
-	BerVarray xvals,
 	ID id,
 	int opid )
 {
@@ -417,7 +329,7 @@ int bdb_index_values(
 
 	rc = index_at_values( op, txn, desc,
 		desc->ad_type, &desc->ad_tags,
-		vals, xvals, id, opid );
+		vals, id, opid );
 
 	return rc;
 }
@@ -444,7 +356,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, NULL, e->e_id, opid );
+			ap->a_nvals, 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 2ff3184daf..61cb5f7908 100644
--- a/servers/slapd/back-bdb/modify.c
+++ b/servers/slapd/back-bdb/modify.c
@@ -14,9 +14,6 @@
 #include "back-bdb.h"
 #include "external.h"
 
-#define INDEXED	0x2000
-#define NULLIFIED	0x4000
-
 int bdb_modify_internal(
 	Operation *op,
 	DB_TXN *tid,
@@ -196,6 +193,17 @@ int bdb_modify_internal(
 		if ( mod->sm_desc == slap_schema.si_ad_objectClass ) {
 			e->e_ocflags = 0;
 		}
+
+		/* check if modified attribute was indexed
+		 * but not in case of NOOP... */
+		err = bdb_index_is_indexed( op->o_bd, mod->sm_desc );
+		if ( err == LDAP_SUCCESS && !op->o_noop ) {
+			ap = attr_find( save_attrs, mod->sm_desc );
+			if ( ap ) ap->a_flags |= SLAP_ATTR_IXDEL;
+
+			ap = attr_find( e->e_attrs, mod->sm_desc );
+			if ( ap ) ap->a_flags |= SLAP_ATTR_IXADD;
+		}
 	}
 
 	/* check that the entry still obeys the schema */
@@ -221,86 +229,52 @@ int bdb_modify_internal(
 	}
 
 	/* update the indices of the modified attributes */
-	if ( !op->o_noop ) {
-		Modifications *m2;
-
-		/* First look for any deletes that would nullify any adds
-		 * in this request. I.e., deleting an entire attribute after
-		 * assigning some values to it.
-		 */
-		for ( ml = modlist; ml != NULL; ml = ml->sml_next ) {
-			if (bdb_index_is_indexed( op->o_bd, ml->sml_desc ))
-				continue;
-			switch ( ml->sml_op ) {
-			case LDAP_MOD_DELETE:
-				/* If just deleting specific values, ignore */
-				if ( ml->sml_bvalues ) break;
-			case LDAP_MOD_REPLACE:
-				for ( m2 = modlist; m2 != ml; m2 = m2->sml_next ) {
-					if ( m2->sml_desc == ml->sml_desc &&
-						m2->sml_op != LDAP_MOD_DELETE )
-						m2->sml_op |= NULLIFIED;
-				}
-				break;
+
+	/* start with deleting the old index entries */
+	for ( ap = save_attrs; ap != NULL; ap = ap->a_next ) {
+		if ( ap->a_flags & SLAP_ATTR_IXDEL ) {
+			rc = bdb_index_values( op, tid, ap->a_desc,
+				ap->a_nvals,
+				e->e_id, SLAP_INDEX_DELETE_OP );
+			if ( rc != LDAP_SUCCESS ) {
+				attrs_free( e->e_attrs );
+				e->e_attrs = save_attrs;
+#ifdef NEW_LOGGING
+				LDAP_LOG ( OPERATION, ERR, 
+					"bdb_modify_internal: attribute index delete failure\n",
+					0, 0, 0 );
+#else
+				Debug( LDAP_DEBUG_ANY,
+				       "Attribute index delete failure",
+				       0, 0, 0 );
+#endif
+				return rc;
 			}
-			ml->sml_op |= INDEXED;
+			ap->a_flags &= ~SLAP_ATTR_IXDEL;
 		}
-		/* Now index the modifications */
-		for ( ml = modlist; ml != NULL; ml = ml->sml_next ) {
-			if ( ! (ml->sml_op & INDEXED) ) continue;
-			ml->sml_op ^= INDEXED;
-			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;
-				}
-				/* FALLTHRU */
-			case LDAP_MOD_REPLACE:
-			/* A nullified replace still does its delete action */
-			case LDAP_MOD_REPLACE | NULLIFIED:
-				ap = attr_find( save_attrs, ml->sml_desc );
-				if ( ap != NULL ) {
-					rc = bdb_index_values( op, tid, ap->a_desc,
-						ap->a_nvals, NULL,
-						e->e_id, SLAP_INDEX_DELETE_OP );
-				} else {
-					rc = LDAP_SUCCESS;
-				}
-				if ( rc || ml->sml_op == LDAP_MOD_DELETE ||
-					(ml->sml_op & NULLIFIED))
-					break;
-				/* FALLTHRU */
-			case LDAP_MOD_ADD:
-			case SLAP_MOD_SOFTADD:
-				rc = bdb_index_values( op, tid, ml->sml_desc,
-					ml->sml_nvalues ? ml->sml_nvalues : ml->sml_bvalues,
-					NULL, e->e_id, SLAP_INDEX_ADD_OP );
-				break;
-			}
-			ml->sml_op &= ~NULLIFIED;
+	}
+
+	/* add the new index entries */
+	for ( ap = e->e_attrs; ap != NULL; ap = ap->a_next ) {
+		if (ap->a_flags & SLAP_ATTR_IXADD) {
+			rc = bdb_index_values( op, tid, ap->a_desc,
+				ap->a_nvals,
+				e->e_id, SLAP_INDEX_ADD_OP );
 			if ( rc != LDAP_SUCCESS ) {
 				attrs_free( e->e_attrs );
 				e->e_attrs = save_attrs;
 #ifdef NEW_LOGGING
 				LDAP_LOG ( OPERATION, ERR, 
-					"bdb_modify_internal: attribute index update failure\n",
+					"bdb_modify_internal: attribute index add failure\n", 
 					0, 0, 0 );
 #else
 				Debug( LDAP_DEBUG_ANY,
-				       "Attribute index update failure",
+				       "Attribute index add failure",
 				       0, 0, 0 );
 #endif
-				/* reset our flags */
-				for (; ml; ml=ml->sml_next ) {
-					ml->sml_op &= ~(INDEXED | NULLIFIED);
-				}
-				break;
+				return rc;
 			}
+			ap->a_flags &= ~SLAP_ATTR_IXADD;
 		}
 	}
 
diff --git a/servers/slapd/back-bdb/proto-bdb.h b/servers/slapd/back-bdb/proto-bdb.h
index 58f1d92dc4..840008034e 100644
--- a/servers/slapd/back-bdb/proto-bdb.h
+++ b/servers/slapd/back-bdb/proto-bdb.h
@@ -308,7 +308,6 @@ bdb_index_values LDAP_P((
 	DB_TXN *txn,
 	AttributeDescription *desc,
 	BerVarray vals,
-	BerVarray xvals,
 	ID id,
 	int opid ));