From 22b64e5505a40921f1959cebcf61e84da29ec12d Mon Sep 17 00:00:00 2001 From: Heikki Levanto Date: Mon, 1 Nov 2004 15:53:57 +0000 Subject: [PATCH] Added debug info and comments to hunt bug #202 Seems like this needs one more rewrite! Comments at the end of file indicate the way I want to do it next --- rset/rsbetween.c | 142 ++++++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 107 insertions(+), 35 deletions(-) diff --git a/rset/rsbetween.c b/rset/rsbetween.c index d099701..321903b 100644 --- a/rset/rsbetween.c +++ b/rset/rsbetween.c @@ -1,4 +1,4 @@ -/* $Id: rsbetween.c,v 1.28 2004-10-22 10:12:51 heikki Exp $ +/* $Id: rsbetween.c,v 1.29 2004-11-01 15:53:57 heikki Exp $ Copyright (C) 1995,1996,1997,1998,1999,2000,2001,2002 Index Data Aps @@ -23,11 +23,17 @@ Free Software Foundation, 59 Temple Place - Suite 330, Boston, MA /* rsbetween is (mostly) used for xml searches. It returns the hits of the * "middle" rset, that are in between the "left" and "right" rsets. For - * example "Shakespeare" in between "" and . The thing is + * example "Shakespeare" in between "" and . The thing is * complicated by the inclusion of attributes (from their own rset). If attrs * specified, they must match the "left" rset (start tag). "Hamlet" between * "" and "". (This assumes that the attributes are * indexed to the same seqno as the tags). + * + * Currently fails to return all hits from a record, which breaks the ranking. + * This is bug #202. Seems like there is no decent way to get that working + * with the current implementation. I have planned a new method for doing + * this, see at the end of this file. + * */ #include @@ -88,21 +94,19 @@ struct rset_between_rfd { void *buf_attr; TERMID term_m; /* we only return terms for the mid argument */ int level; /* counting start/end tags */ + int attr_match; /* did we have a matching attr for L */ zint hits; }; #if RSBETWEEN_DEBUG -static void log2 (struct rset_between_rfd *p, char *msg, int cmp_l, int cmp_r) +static void log2 (RSFD rfd, char *msg, int cmp_l, int cmp_r) { - char buf_l[32]; - char buf_m[32]; - char buf_r[32]; - logf(LOG_DEBUG,"btw: %s l=%s(%d/%d) m=%s(%d) r=%s(%d/%d), lev=%d", - msg, - (*p->info->printer)(p->buf_l, buf_l), p->more_l, cmp_l, - (*p->info->printer)(p->buf_m, buf_m), p->more_m, - (*p->info->printer)(p->buf_r, buf_r), p->more_r, cmp_r, - p->level); + struct rset_between_rfd *p=(struct rset_between_rfd *)rfd->priv; + RSET ct = rfd->rset; + logf(LOG_LOG,"between: %s cmp_l=%d cmp_r=%d", msg, cmp_l, cmp_r); + (*ct->keycontrol->key_logdump_txt)(LOG_LOG, p->buf_l, "between: L"); + (*ct->keycontrol->key_logdump_txt)(LOG_LOG, p->buf_m, "between: M"); + (*ct->keycontrol->key_logdump_txt)(LOG_LOG, p->buf_r, "between: R"); } #endif @@ -168,7 +172,9 @@ static RSFD r_open_between (RSET ct, int flag) { p->rfd_attr = rset_open (info->rset_attr, RSETF_READ); p->more_attr = rset_read (p->rfd_attr, p->buf_attr, NULL); - } + p->attr_match=0; + } else + p->attr_match=1; p->level=0; p->hits=0; return rfd; @@ -179,6 +185,9 @@ static void r_close_between (RSFD rfd) struct rset_between_info *info =(struct rset_between_info *)rfd->rset->priv; struct rset_between_rfd *p=(struct rset_between_rfd *)rfd->priv; +#if RSBETWEEN_DEBUG + log2( rfd, "fwd: close. hits:", p->hits,0); +#endif rset_close (p->rfd_l); rset_close (p->rfd_m); rset_close (p->rfd_r); @@ -195,18 +204,18 @@ static int r_forward_between(RSFD rfd, void *buf, struct rset_between_rfd *p=(struct rset_between_rfd *)rfd->priv; int rc; #if RSBETWEEN_DEBUG - log2( p, "fwd: before forward", 0,0); + log2( rfd, "fwd: before forward", 0,0); #endif /* It is enough to forward the m pointer here, the read will */ /* naturally forward the l, m, and attr pointers */ if (p->more_m) p->more_m=rset_forward(p->rfd_m, p->buf_m, term, untilbuf); #if RSBETWEEN_DEBUG - log2( p, "fwd: after forward M", 0,0); + log2( rfd, "fwd: after forward M", 0,0); #endif rc = r_read_between(rfd, buf, term); #if RSBETWEEN_DEBUG - log2( p, "fwd: after forward", 0,0); + log2( rfd, "fwd: after forward", 0,0); #endif return rc; } @@ -221,12 +230,14 @@ static int r_read_between (RSFD rfd, void *buf, TERMID *term) const struct key_control *kctrl=rfd->rset->keycontrol; int cmp_l=0; int cmp_r=0; - int attr_match = 0; +/* int attr_match = 0; */ while (p->more_m) { #if RSBETWEEN_DEBUG - log2( p, "start of loop", cmp_l, cmp_r); + log2( rfd, "start of loop", cmp_l, cmp_r); + logf(LOG_LOG,"level=%d. more=%d/%d/%d", + p->level,p->more_l,p->more_m, p->more_r); #endif /* forward L until past m, count levels, note rec boundaries */ @@ -238,7 +249,9 @@ static int r_read_between (RSFD rfd, void *buf, TERMID *term) cmp_l=rfd->rset->scope; /* past this record */ } #if RSBETWEEN_DEBUG - log2( p, "after first L", cmp_l, cmp_r); + log2( rfd, "after first L", cmp_l, cmp_r); + logf(LOG_LOG,"level=%d. more=%d/%d/%d", + p->level,p->more_l,p->more_m, p->more_r); #endif while (cmp_l < 0) /* l before m */ @@ -250,17 +263,17 @@ static int r_read_between (RSFD rfd, void *buf, TERMID *term) p->level++; /* relevant start tag */ if (!info->rset_attr) - attr_match = 1; + p->attr_match = 1; else { int cmp_attr; - attr_match = 0; + p->attr_match = 0; while (p->more_attr) { cmp_attr = (*kctrl->cmp)(p->buf_attr, p->buf_l); if (cmp_attr == 0) { - attr_match = 1; + p->attr_match = 1; break; } else if (cmp_attr > 0) @@ -276,7 +289,7 @@ static int r_read_between (RSFD rfd, void *buf, TERMID *term) p->more_attr = rset_forward( p->rfd_attr, p->buf_attr, NULL, p->buf_l); #if RSBETWEEN_DEBUG - logf(LOG_DEBUG, "btw: after frowarding attr m=%d", + logf(LOG_LOG, "btw: after frowarding attr m=%d", p->more_attr); #endif } @@ -295,7 +308,9 @@ static int r_read_between (RSFD rfd, void *buf, TERMID *term) else cmp_l=rfd->rset->scope; /*2*/ #if RSBETWEEN_DEBUG - log2( p, "after forwarding L", cmp_l, cmp_r); + log2( rfd, "after forwarding L", cmp_l, cmp_r); + logf(LOG_LOG,"level=%d. more=%d/%d/%d", + p->level,p->more_l,p->more_m, p->more_r); #endif } } else @@ -312,21 +327,27 @@ static int r_read_between (RSFD rfd, void *buf, TERMID *term) else cmp_l=rfd->rset->scope; /*2*/ #if RSBETWEEN_DEBUG - log2( p, "end of L loop", cmp_l, cmp_r); + log2( rfd, "end of L loop", cmp_l, cmp_r); + logf(LOG_LOG,"level=%d. more=%d/%d/%d", + p->level,p->more_l,p->more_m, p->more_r); #endif } /* forward L */ /* forward R until past m, count levels */ #if RSBETWEEN_DEBUG - log2( p, "Before moving R", cmp_l, cmp_r); + log2( rfd, "Before moving R", cmp_l, cmp_r); + logf(LOG_LOG,"level=%d. more=%d/%d/%d", + p->level,p->more_l,p->more_m, p->more_r); #endif if (p->more_r) cmp_r= (*kctrl->cmp)(p->buf_r, p->buf_m); else cmp_r=rfd->rset->scope; /*2*/ #if RSBETWEEN_DEBUG - log2( p, "after first R", cmp_l, cmp_r); + log2( rfd, "after first R", cmp_l, cmp_r); + logf(LOG_LOG,"level=%d. more=%d/%d/%d", + p->level,p->more_l,p->more_m, p->more_r); #endif while (cmp_r < 0) /* r before m */ { @@ -354,22 +375,40 @@ static int r_read_between (RSFD rfd, void *buf, TERMID *term) else cmp_r=rfd->rset->scope; /*2*/ #if RSBETWEEN_DEBUG - log2( p, "End of R loop", cmp_l, cmp_r); + log2( rfd, "End of R loop", cmp_l, cmp_r); + logf(LOG_LOG,"level=%d. more=%d/%d/%d am=%d", + p->level,p->more_l,p->more_m, p->more_r, p->attr_match); #endif } /* forward R */ if ( ( p->level <= 0 ) && ! p->more_l) + { +#if RSBETWEEN_DEBUG + logf(LOG_LOG,"no more_l, returning zero"); +#endif return 0; /* no more start tags, nothing more to find */ + } - if ( attr_match && p->level > 0) /* within a tag pair (or deeper) */ +#if RSBETWEEN_DEBUG + log2( rfd, "Considering M", cmp_l, cmp_r); + logf(LOG_LOG,"level=%d. more=%d/%d/%d am=%d", + p->level,p->more_l,p->more_m, p->more_r, p->attr_match); +#endif + if ( p->attr_match && p->level > 0) /* within a tag pair (or deeper) */ { memcpy (buf, p->buf_m, kctrl->key_size); if (term) *term=p->term_m; #if RSBETWEEN_DEBUG - log2( p, "Returning a hit (and forwarding m)", cmp_l, cmp_r); + log2( rfd, "Returning a hit (and forwarding m)", cmp_l, cmp_r); #endif p->more_m = rset_read (p->rfd_m, p->buf_m, NULL); +#if RSBETWEEN_DEBUG + logf(LOG_LOG,"read m. more=%d level=%d " + "cmp_l=%d scope=%d hits="ZINT_FORMAT, + p->more_m, p->level, + cmp_l, rfd->rset->scope, p->hits); +#endif if (cmp_l >= rfd->rset->scope) /* == 2 */ p->level = 0; p->hits++; @@ -378,7 +417,7 @@ static int r_read_between (RSFD rfd, void *buf, TERMID *term) else if ( ! p->more_l ) /* not in data, no more starts */ { #if RSBETWEEN_DEBUG - log2( p, "no more starts, exiting without a hit", cmp_l, cmp_r); + log2( rfd, "no more starts, exiting without a hit", cmp_l, cmp_r); #endif return 0; /* ergo, nothing can be found. stop scanning */ } @@ -394,15 +433,19 @@ static int r_read_between (RSFD rfd, void *buf, TERMID *term) #else if (cmp_l >= rfd->rset->scope ) /* == 2 */ p->level = 0; - p->more_m = rset_read (p->rfd_m, p->buf_m); + p->more_m = rset_read (p->rfd_m, p->buf_m, &p->term_m); #endif #if RSBETWEEN_DEBUG - log2( p, "End of M loop", cmp_l, cmp_r); + log2( rfd, "End of M loop", cmp_l, cmp_r); + logf(LOG_LOG,"level=%d. more=%d/%d/%d", + p->level,p->more_l,p->more_m, p->more_r); #endif } /* while more_m */ #if RSBETWEEN_DEBUG - log2( p, "Exiting, nothing more in m", cmp_l, cmp_r); + log2( rfd, "Exiting, nothing more in m", cmp_l, cmp_r); + logf(LOG_LOG,"level=%d. more=%d/%d/%d", + p->level,p->more_l,p->more_m, p->more_r); #endif return 0; /* no more data possible */ @@ -444,9 +487,12 @@ static void r_pos_between (RSFD rfd, double *current, double *total) *current=p->hits; *total=*current/r ; #if RSBETWEEN_DEBUG - yaz_log(LOG_DEBUG,"betw_pos: (%s/%s) %0.1f/%0.1f= %0.4f ", + { + struct rset_between_info *info =(struct rset_between_info *)rfd->rset->priv; + yaz_log(LOG_LOG,"betw_pos: (%s/%s) %0.1f/%0.1f= %0.4f ", info->rset_l->control->desc, info->rset_r->control->desc, *current, *total, r); + } #endif } @@ -456,3 +502,29 @@ static void r_get_terms(RSET ct, TERMID *terms, int maxterms, int *curterm) rset_getterms(info->rset_m, terms, maxterms, curterm); } +/* Better algorithm + * One of the major problems with rsbetween is the complexity of keeping track + * of start tags, stop tags, hits, and attributes, together with record + * boundaries etc. + * + * Things can be divided into finding the right records, and then processing + * hits inside the record. + * + * Finding the record is mostly a matter of forwarding until we have a start + * tag and a hit in the same record. + * + * Handling stuff inside a record, we can simplify things by implementing a + * glorified OR operator that returns all the occurrences in proper order, + * together with info on what type it was. Then the main logic can just keep + * reading, and consider each type separately: + * - if a start tag, increment level (some trickery with attributes!) + * - if a stop tag, decrement level + * - if a hit, and we have a level, return it + * - if a hit, but no level, ignore it + * + * The attributes can be detected when ever reading start tags. The main + * routine needs to keep a stack of attribute match bits, so when ever we read + * a start tag, we must report back if we have a matching attribute or not. + * + */ + -- 1.7.10.4