Changeset: f4cb487b1b1d for MonetDB
URL: https://dev.monetdb.org/hg/MonetDB?cmd=changeset;node=f4cb487b1b1d
Modified Files:
        gdk/gdk_align.c
        gdk/gdk_bat.c
        gdk/gdk_batop.c
Branch: Jun2020
Log Message:

Reimplemented VIEWreset to be more careful when things go wrong.


diffs (223 lines):

diff --git a/gdk/gdk_align.c b/gdk/gdk_align.c
--- a/gdk/gdk_align.c
+++ b/gdk/gdk_align.c
@@ -274,114 +274,99 @@ VIEWunlink(BAT *b)
 gdk_return
 VIEWreset(BAT *b)
 {
-       bat tp, tvp;
-       Heap tail, *th = NULL;
-       BAT *v = NULL;
-
        if (b == NULL)
                return GDK_FAIL;
-       assert(b->batCacheid > 0);
-       tp = VIEWtparent(b);
-       tvp = VIEWvtparent(b);
-       if (tp || tvp) {
-               BUN cnt;
-               const char *nme;
 
-               /* alloc heaps */
-               tail = (Heap) {0};
-
-               cnt = BATcount(b) + 1;
-               nme = BBP_physical(b->batCacheid);
-
-               assert(b->batCacheid > 0);
-               assert(tp || tvp || !b->ttype);
+       bat tp = VIEWtparent(b);
+       bat tvp = VIEWvtparent(b);
+       if (tp == 0 && tvp == 0)
+               return GDK_SUCCEED;
 
-               tail.farmid = BBPselectfarm(b->batRole, b->ttype, offheap);
-               strconcat_len(tail.filename, sizeof(tail.filename),
-                             nme, ".tail", NULL);
-               if (b->ttype && HEAPalloc(&tail, cnt, Tsize(b)) != GDK_SUCCEED)
-                       goto bailout;
-               if (b->tvheap) {
-                       th = GDKzalloc(sizeof(Heap));
-                       if (th == NULL)
-                               goto bailout;
-                       th->farmid = BBPselectfarm(b->batRole, b->ttype, 
varheap);
-                       strconcat_len(th->filename, sizeof(th->filename),
-                                     nme, ".theap", NULL);
-                       if (ATOMheap(b->ttype, th, cnt) != GDK_SUCCEED)
-                               goto bailout;
-               }
+       if (tp == 0) {
+               /* only sharing the vheap */
+               assert(ATOMstorage(b->ttype) == TYPE_str);
+               return unshare_string_heap(b);
+       }
 
-               v = VIEWcreate(b->hseqbase, b);
-               if (v == NULL)
-                       goto bailout;
+       BAT *v = VIEWcreate(b->hseqbase, b);
+       if (v == NULL)
+               return GDK_FAIL;
 
-               /* cut the link to your parents */
-               VIEWunlink(b);
-               if (tp) {
-                       BBPunshare(tp);
-                       BBPunfix(tp);
-               }
-               if (tvp) {
-                       BBPunshare(tvp);
-                       BBPunfix(tvp);
-               }
-
-               b->hseqbase = v->hseqbase;
+       assert(VIEWtparent(v) != b->batCacheid);
+       assert(VIEWvtparent(v) != b->batCacheid);
 
-               b->ttype = v->ttype;
-               b->tvarsized = v->tvarsized;
-               b->tshift = v->tshift;
-               b->twidth = v->twidth;
-               b->tseqbase = v->tseqbase;
-
-               b->theap.parentid = 0;
-               b->batRestricted = BAT_WRITE;
-
-               b->tkey = BATtkey(v);
+       const char *nme = BBP_physical(b->batCacheid);
+       BUN cnt = BATcount(b);
+       Heap tail = (Heap) {
+               .farmid = BBPselectfarm(b->batRole, b->ttype, offheap),
+       };
+       strconcat_len(tail.filename, sizeof(tail.filename), nme, ".tail", NULL);
+       if (b->ttype && HEAPalloc(&tail, cnt, Tsize(b)) != GDK_SUCCEED) {
+               BBPunfix(v->batCacheid);
+               return GDK_FAIL;
+       }
+       Heap *th = NULL;
+       if (b->tvheap) {
+               th = GDKmalloc(sizeof(Heap));
+               if (th == NULL) {
+                       HEAPfree(&tail, true);
+                       BBPunfix(v->batCacheid);
+                       return GDK_FAIL;
+               }
+               *th = (Heap) {
+                       .farmid = BBPselectfarm(b->batRole, b->ttype, varheap),
+                       .parentid = b->batCacheid,
+               };
+               strconcat_len(th->filename, sizeof(th->filename),
+                             nme, ".theap", NULL);
+               if (ATOMheap(b->ttype, th, cnt) != GDK_SUCCEED) {
+                       HEAPfree(&tail, true);
+                       GDKfree(th);
+                       BBPunfix(v->batCacheid);
+                       return GDK_FAIL;
+               }
+       }
 
-               /* copy the heaps */
-               b->theap = tail;
-
-               /* unshare from parents heap */
-               if (th) {
-                       assert(b->tvheap == NULL);
-                       b->tvheap = th;
-                       th = NULL;
-                       b->tvheap->parentid = b->batCacheid;
-               }
+       BAT bak = *b;           /* backup copy */
 
-               if (v->theap.parentid == b->batCacheid) {
-                       assert(tp == 0);
-                       assert(b->batSharecnt > 0);
-                       BBPunshare(b->batCacheid);
-                       BBPunfix(b->batCacheid);
-                       v->theap.parentid = 0;
+       /* start overwriting */
+       VIEWunlink(b);
+       b->tvheap = th;
+       b->theap = tail;
+       /* we empty out b completely and then fill it from v */
+       DELTAinit(b);
+       b->batCapacity = cnt;
+       b->batCopiedtodisk = false;
+       b->batDirtydesc = true;
+       b->batRestricted = BAT_WRITE;
+       b->tkey = true;
+       b->tsorted = b->trevsorted = true;
+       b->tnosorted = b->tnorevsorted = 0;
+       b->tnokey[0] = b->tnokey[1] = 0;
+       b->tnil = false;
+       b->tnonil = true;
+       if (BATappend2(b, v, NULL, false, false) != GDK_SUCCEED) {
+               /* clean up the mess */
+               if (th) {
+                       HEAPfree(th, true);
+                       GDKfree(th);
                }
-               b->batSharecnt = 0;
-               b->batCopiedtodisk = false;
-               b->batDirtydesc = true;
-
-               b->tkey = BATtkey(v);
-
-               /* make the BAT empty and insert all again */
-               DELTAinit(b);
-               /* reset capacity */
-               b->batCapacity = cnt;
-
-               /* insert all of v in b, and quit */
-               if (BATappend2(b, v, NULL, false, false) != GDK_SUCCEED) {
-                       GDKerror("appending view failed");
-                       goto bailout;
-               }
-               BBPreclaim(v);
+               HEAPfree(&tail, true);
+               *b = bak;
+               BBPunfix(v->batCacheid);
+               return GDK_FAIL;
        }
+       /* point of no return */
+       if (tp) {
+               BBPunshare(tp);
+               BBPunfix(tp);
+       }
+       if (tvp) {
+               BBPunshare(tvp);
+               BBPunfix(tvp);
+       }
+       BBPunfix(v->batCacheid);
        return GDK_SUCCEED;
-      bailout:
-       BBPreclaim(v);
-       HEAPfree(&tail, false);
-       GDKfree(th);
-       return GDK_FAIL;
 }
 
 /*
diff --git a/gdk/gdk_bat.c b/gdk/gdk_bat.c
--- a/gdk/gdk_bat.c
+++ b/gdk/gdk_bat.c
@@ -2171,6 +2171,8 @@ BATassertProps(BAT *b)
               b->tvheap == NULL ||
               (BBPfarms[b->tvheap->farmid].roles & (1 << b->batRole)));
 
+       assert(VIEWtparent(b) == 0 || b->tvheap == NULL || b->tvheap->parentid 
!= b->batCacheid);
+
        cmpf = ATOMcompare(b->ttype);
        nilp = ATOMnilptr(b->ttype);
 
diff --git a/gdk/gdk_batop.c b/gdk/gdk_batop.c
--- a/gdk/gdk_batop.c
+++ b/gdk/gdk_batop.c
@@ -22,7 +22,7 @@ gdk_return
 unshare_string_heap(BAT *b)
 {
        assert(b->batCacheid > 0);
-       if (b->ttype == TYPE_str &&
+       if (ATOMstorage(b->ttype) == TYPE_str &&
            b->tvheap->parentid != b->batCacheid) {
                Heap *h = GDKzalloc(sizeof(Heap));
                if (h == NULL)
_______________________________________________
checkin-list mailing list
checkin-list@monetdb.org
https://www.monetdb.org/mailman/listinfo/checkin-list

Reply via email to