Changeset: 01e5ce140c60 for MonetDB
URL: https://dev.monetdb.org/hg/MonetDB?cmd=changeset;node=01e5ce140c60
Modified Files:
        sql/backends/monet5/sql_cat.c
        sql/server/sql_mvc.c
        sql/storage/bat/bat_storage.c
        sql/storage/sql_storage.h
        sql/storage/store.c
Branch: default
Log Message:

added a lot more protection against transaction conflicts.


diffs (truncated from 1700 to 300 lines):

diff --git a/sql/backends/monet5/sql_cat.c b/sql/backends/monet5/sql_cat.c
--- a/sql/backends/monet5/sql_cat.c
+++ b/sql/backends/monet5/sql_cat.c
@@ -429,7 +429,8 @@ alter_table_del_table(mvc *sql, char *ms
        if (!(n = members_find_child_id(mt->members.set, pt->base.id)))
                throw(SQL,"sql.alter_table_del_table",SQLSTATE(42S02) "ALTER 
TABLE: table '%s.%s' isn't part of %s '%s.%s'", ps->base.name, ptname, 
errtable, ms->base.name, mtname);
 
-       sql_trans_del_table(sql->session->tr, mt, pt, drop_action);
+       if (sql_trans_del_table(sql->session->tr, mt, pt, drop_action))
+               throw(SQL,"sql.alter_table_del_table",SQLSTATE(42000) "ALTER 
TABLE: transaction conflict detected");
        return MAL_SUCCEED;
 }
 
diff --git a/sql/server/sql_mvc.c b/sql/server/sql_mvc.c
--- a/sql/server/sql_mvc.c
+++ b/sql/server/sql_mvc.c
@@ -1306,7 +1306,7 @@ mvc_drop_table(mvc *m, sql_schema *s, sq
        }
 
        if (sql_trans_drop_table(m->session->tr, s, t->base.name, drop_action ? 
DROP_CASCADE_START : DROP_RESTRICT))
-               throw(SQL, "sql.mvc_drop_table", SQLSTATE(HY013) 
MAL_MALLOC_FAIL);
+               throw(SQL, "sql.mvc_drop_table", SQLSTATE(42000) "Transaction 
conflict while dropping table %s.%s", s->base.name, t->base.name);
        return MAL_SUCCEED;
 }
 
diff --git a/sql/storage/bat/bat_storage.c b/sql/storage/bat/bat_storage.c
--- a/sql/storage/bat/bat_storage.c
+++ b/sql/storage/bat/bat_storage.c
@@ -806,7 +806,7 @@ update_col_prepare(sql_trans *tr, sql_al
                return NULL;
 
        assert(delta && delta->ts == tr->tid);
-       if ((!inTransaction(tr, c->t) && odelta != delta && isGlobal(c->t)) || 
(!isNew(c->t) && isLocalTemp(c->t)))
+       if ((!inTransaction(tr, c->t) && (odelta != delta || isTempTable(c->t)) 
&& isGlobal(c->t)) || (!isNew(c->t) && isLocalTemp(c->t)))
                trans_add(tr, &c->base, delta, &tc_gc_col, &commit_update_col, 
isLocalTemp(c->t)?NULL:&log_update_col);
        return make_cookie(sa, delta, isNew(c));
 }
@@ -880,7 +880,7 @@ update_idx_prepare(sql_trans *tr, sql_al
                return NULL;
 
        assert(delta && delta->ts == tr->tid);
-       if ((!inTransaction(tr, i->t) && odelta != delta && isGlobal(i->t)) || 
(!isNew(i->t) && isLocalTemp(i->t)))
+       if ((!inTransaction(tr, i->t) && (odelta != delta || isTempTable(i->t)) 
&& isGlobal(i->t)) || (!isNew(i->t) && isLocalTemp(i->t)))
                trans_add(tr, &i->base, delta, &tc_gc_idx, &commit_update_idx, 
isLocalTemp(i->t)?NULL:&log_update_idx);
        return make_cookie(sa, delta, isNew(i));
 }
@@ -1026,7 +1026,7 @@ append_col_prepare(sql_trans *tr, sql_al
                return NULL;
 
        assert(delta && delta->ts == tr->tid);
-       if ((!inTransaction(tr, c->t) && odelta != delta && isGlobal(c->t)) || 
(!isNew(c->t) && isLocalTemp(c->t)))
+       if ((!inTransaction(tr, c->t) && (odelta != delta || isTempTable(c->t)) 
&& isGlobal(c->t)) || (!isNew(c->t) && isLocalTemp(c->t)))
                trans_add(tr, &c->base, delta, &tc_gc_col, &commit_update_col, 
isLocalTemp(c->t)?NULL:&log_update_col);
        return make_cookie(sa, delta, false);
 }
@@ -1071,7 +1071,7 @@ append_idx_prepare(sql_trans *tr, sql_al
                return NULL;
 
        assert(delta && delta->ts == tr->tid);
-       if ((!inTransaction(tr, i->t) && odelta != delta && isGlobal(i->t)) || 
(!isNew(i->t) && isLocalTemp(i->t)))
+       if ((!inTransaction(tr, i->t) && (odelta != delta || isTempTable(i->t)) 
&& isGlobal(i->t)) || (!isNew(i->t) && isLocalTemp(i->t)))
                trans_add(tr, &i->base, delta, &tc_gc_idx, &commit_update_idx, 
isLocalTemp(i->t)?NULL:&log_update_idx);
        return make_cookie(sa, delta, false);
 }
@@ -1229,7 +1229,7 @@ delete_tab(sql_trans *tr, sql_table * t,
                ok = delta_delete_bat(bat, ib);
        else
                ok = delta_delete_val(bat, *(oid*)ib);
-       if ((!inTransaction(tr, t) && obat != bat && isGlobal(t)) || (!isNew(t) 
&& isLocalTemp(t)))
+       if ((!inTransaction(tr, t) && (obat != bat || isTempTable(t)) && 
isGlobal(t)) || (!isNew(t) && isLocalTemp(t)))
                trans_add(tr, &t->base, bat, &tc_gc_del, &commit_update_del, 
isLocalTemp(t)?NULL:&log_update_del);
        return ok;
 }
@@ -2200,7 +2200,7 @@ clear_col(sql_trans *tr, sql_column *c)
 
        if ((delta = bind_col_data(tr, c)) == NULL)
                return BUN_NONE;
-       if ((!inTransaction(tr, c->t) && odelta != delta && isGlobal(c->t)) || 
(!isNew(c->t) && isLocalTemp(c->t)))
+       if ((!inTransaction(tr, c->t) && (odelta != delta || isTempTable(c->t)) 
&& isGlobal(c->t)) || (!isNew(c->t) && isLocalTemp(c->t)))
                trans_add(tr, &c->base, delta, &tc_gc_col, &commit_update_col, 
isLocalTemp(c->t)?NULL:&log_update_col);
        if (delta)
                return clear_delta(tr, delta);
@@ -2216,7 +2216,7 @@ clear_idx(sql_trans *tr, sql_idx *i)
                return 0;
        if ((delta = bind_idx_data(tr, i)) == NULL)
                return BUN_NONE;
-       if ((!inTransaction(tr, i->t) && odelta != delta && isGlobal(i->t)) || 
(!isNew(i->t) && isLocalTemp(i->t)))
+       if ((!inTransaction(tr, i->t) && (odelta != delta || isTempTable(i->t)) 
&& isGlobal(i->t)) || (!isNew(i->t) && isLocalTemp(i->t)))
                trans_add(tr, &i->base, delta, &tc_gc_idx, &commit_update_idx, 
isLocalTemp(i->t)?NULL:&log_update_idx);
        if (delta)
                return clear_delta(tr, delta);
@@ -2256,7 +2256,7 @@ clear_del(sql_trans *tr, sql_table *t)
 
        if ((bat = bind_del_data(tr, t)) == NULL)
                return BUN_NONE;
-       if ((!inTransaction(tr, t) && obat != bat && isGlobal(t)) || (!isNew(t) 
&& isLocalTemp(t)))
+       if ((!inTransaction(tr, t) && (obat != bat || isTempTable(t)) && 
isGlobal(t)) || (!isNew(t) && isLocalTemp(t)))
                trans_add(tr, &t->base, bat, &tc_gc_del, &commit_update_del, 
isLocalTemp(t)?NULL:&log_update_del);
        return clear_dbat(tr, bat);
 }
diff --git a/sql/storage/sql_storage.h b/sql/storage/sql_storage.h
--- a/sql/storage/sql_storage.h
+++ b/sql/storage/sql_storage.h
@@ -372,7 +372,7 @@ extern int sql_trans_add_value_partition
 
 extern sql_table *sql_trans_rename_table(sql_trans *tr, sql_schema *s, sqlid 
id, const char *new_name);
 extern sql_table *sql_trans_set_table_schema(sql_trans *tr, sqlid id, 
sql_schema *os, sql_schema *ns);
-extern sql_table *sql_trans_del_table(sql_trans *tr, sql_table *mt, sql_table 
*pt, int drop_action);
+extern int sql_trans_del_table(sql_trans *tr, sql_table *mt, sql_table *pt, 
int drop_action);
 
 extern int sql_trans_drop_table(sql_trans *tr, sql_schema *s, const char 
*name, int drop_action);
 extern int sql_trans_drop_table_id(sql_trans *tr, sql_schema *s, sqlid id, int 
drop_action);
@@ -411,7 +411,7 @@ extern int sql_trans_create_role(sql_tra
 
 extern sql_sequence *create_sql_sequence(struct sqlstore *store, sql_allocator 
*sa, sql_schema *s, const char *name, lng start, lng min, lng max, lng inc, lng 
cacheinc, bit cycle);
 extern sql_sequence * sql_trans_create_sequence(sql_trans *tr, sql_schema *s, 
const char *name, lng start, lng min, lng max, lng inc, lng cacheinc, bit 
cycle, bit bedropped);
-extern void sql_trans_drop_sequence(sql_trans *tr, sql_schema *s, sql_sequence 
*seq, int drop_action);
+extern int sql_trans_drop_sequence(sql_trans *tr, sql_schema *s, sql_sequence 
*seq, int drop_action);
 extern sql_sequence *sql_trans_alter_sequence(sql_trans *tr, sql_sequence 
*seq, lng min, lng max, lng inc, lng cache, bit cycle);
 extern sql_sequence *sql_trans_sequence_restart(sql_trans *tr, sql_sequence 
*seq, lng start);
 extern sql_sequence *sql_trans_seqbulk_restart(sql_trans *tr, seqbulk *sb, lng 
start);
@@ -453,9 +453,6 @@ extern sql_key *sql_trans_copy_key(sql_t
 extern sql_idx *sql_trans_copy_idx(sql_trans *tr, sql_table *t, sql_idx *i);
 extern sql_trigger *sql_trans_copy_trigger(sql_trans *tr, sql_table *t, 
sql_trigger *tri);
 
-extern void sql_trans_drop_any_comment(sql_trans *tr, sqlid id);
-extern void sql_trans_drop_obj_priv(sql_trans *tr, sqlid obj_id);
-
 #define inTransaction(tr,t) (isLocalTemp(t) || 
os_obj_intransaction(t->s->tables, tr, &t->base))
 #define TRANSACTION_ID_BASE    (1ULL<<63)
 
diff --git a/sql/storage/store.c b/sql/storage/store.c
--- a/sql/storage/store.c
+++ b/sql/storage/store.c
@@ -2582,7 +2582,8 @@ end:
  *
  * This function is not entirely safe as compared to for example mkstemp.
  */
-static str pick_tmp_name(str filename)
+static str
+pick_tmp_name(str filename)
 {
        str name = GDKmalloc(strlen(filename) + 10);
        if (name == NULL) {
@@ -2615,7 +2616,7 @@ static str pick_tmp_name(str filename)
        return name;
 }
 
-extern lng
+lng
 store_hot_snapshot_to_stream(sqlstore *store, stream *tar_stream)
 {
        int locked = 0;
@@ -2675,7 +2676,7 @@ end:
 }
 
 
-extern lng
+lng
 store_hot_snapshot(sqlstore *store, str tarfile)
 {
        lng result = 0;
@@ -3092,6 +3093,8 @@ sql_trans_copy_key( sql_trans *tr, sql_t
        node *n;
 
        t = new_table(tr, t);
+       if (!t)
+               return NULL;
        sql_key *nk = key_dup(tr, k, t);
        sql_fkey *fk = (sql_fkey*)nk;
        cs_add(&t->keys, nk, TR_NEW);
@@ -3099,7 +3102,8 @@ sql_trans_copy_key( sql_trans *tr, sql_t
        if (nk->type == fkey)
                action = (fk->on_update<<8) + fk->on_delete;
 
-       store->table_api.table_insert(tr, syskey, &nk->base.id, &t->base.id, 
&nk->type, nk->base.name, (nk->type == fkey) ? &((sql_fkey *) nk)->rkey : &neg, 
&action);
+       if (store->table_api.table_insert(tr, syskey, &nk->base.id, 
&t->base.id, &nk->type, nk->base.name, (nk->type == fkey) ? &((sql_fkey *) 
nk)->rkey : &neg, &action))
+               return NULL;
 
        if (nk->type == fkey)
                sql_trans_create_dependency(tr, ((sql_fkey *) nk)->rkey, 
nk->base.id, FKEY_DEPENDENCY);
@@ -3107,7 +3111,8 @@ sql_trans_copy_key( sql_trans *tr, sql_t
        for (n = nk->columns->h, nr = 0; n; n = n->next, nr++) {
                sql_kc *kc = n->data;
 
-               store->table_api.table_insert(tr, syskc, &nk->base.id, 
kc->c->base.name, &nr, ATOMnilptr(TYPE_int));
+               if (store->table_api.table_insert(tr, syskc, &nk->base.id, 
kc->c->base.name, &nr, ATOMnilptr(TYPE_int)))
+                       return NULL;
 
                if (nk->type == fkey)
                        sql_trans_create_dependency(tr, kc->c->base.id, 
nk->base.id, FKEY_DEPENDENCY);
@@ -3121,7 +3126,7 @@ sql_trans_copy_key( sql_trans *tr, sql_t
        return nk;
 }
 
-       sql_idx *
+sql_idx *
 sql_trans_copy_idx( sql_trans *tr, sql_table *t, sql_idx *i)
 {
        sqlstore *store = tr->store;
@@ -3133,6 +3138,8 @@ sql_trans_copy_idx( sql_trans *tr, sql_t
        sql_idx *ni = SA_ZNEW(tr->sa, sql_idx);
 
        t = new_table(tr, t);
+       if (!t)
+               return NULL;
        base_init(tr->sa, &ni->base, i->base.id?i->base.id:next_oid(tr->store), 
TR_NEW, i->base.name);
 
        ni->columns = list_new(tr->sa, (fdestroy) &kc_destroy);
@@ -3149,7 +3156,8 @@ sql_trans_copy_idx( sql_trans *tr, sql_t
                if (ic->c->unique != (unique & !okc->c->null))
                        okc->c->unique = ic->c->unique = (unique & 
(!okc->c->null));
 
-               store->table_api.table_insert(tr, sysic, &ni->base.id, 
ic->c->base.name, &nr, ATOMnilptr(TYPE_int));
+               if (store->table_api.table_insert(tr, sysic, &ni->base.id, 
ic->c->base.name, &nr, ATOMnilptr(TYPE_int)))
+                       return NULL;
 
                sql_trans_create_dependency(tr, ic->c->base.id, ni->base.id, 
INDEX_DEPENDENCY);
        }
@@ -3164,11 +3172,12 @@ sql_trans_copy_idx( sql_trans *tr, sql_t
                        if (store->storage_api.create_idx(tr, ni) != LOG_OK)
                                return NULL;
        if (!isDeclaredTable(t))
-               store->table_api.table_insert(tr, sysidx, &ni->base.id, 
&t->base.id, &ni->type, ni->base.name);
+               if (store->table_api.table_insert(tr, sysidx, &ni->base.id, 
&t->base.id, &ni->type, ni->base.name))
+                       return NULL;
        return ni;
 }
 
-       sql_trigger *
+sql_trigger *
 sql_trans_copy_trigger( sql_trans *tr, sql_table *t, sql_trigger *tri)
 {
        sqlstore *store = tr->store;
@@ -3200,7 +3209,8 @@ sql_trans_copy_trigger( sql_trans *tr, s
                sql_kc *okc = n->data, *ic;
 
                list_append(nt->columns, ic = kc_dup(tr, okc, t));
-               store->table_api.table_insert(tr, sysic, &nt->base.id, 
ic->c->base.name, &nr, ATOMnilptr(TYPE_int));
+               if (store->table_api.table_insert(tr, sysic, &nt->base.id, 
ic->c->base.name, &nr, ATOMnilptr(TYPE_int)))
+                       return NULL;
                sql_trans_create_dependency(tr, ic->c->base.id, nt->base.id, 
TRIGGER_DEPENDENCY);
        }
        cs_add(&t->triggers, nt, TR_NEW);
@@ -3210,13 +3220,14 @@ sql_trans_copy_trigger( sql_trans *tr, s
        }
 
        if (!isDeclaredTable(t))
-               store->table_api.table_insert(tr, systr, &nt->base.id, 
nt->base.name, &t->base.id, &nt->time, &nt->orientation,
+               if (store->table_api.table_insert(tr, systr, &nt->base.id, 
nt->base.name, &t->base.id, &nt->time, &nt->orientation,
                                &nt->event, (nt->old_name)?nt->old_name:nilptr, 
(nt->new_name)?nt->new_name:nilptr,
-                               (nt->condition)?nt->condition:nilptr, 
nt->statement);
+                               (nt->condition)?nt->condition:nilptr, 
nt->statement))
+                       return NULL;
        return nt;
 }
 
-       static int
+static int
 sql_trans_cname_conflict(sql_table *t, const char *extra, const char *cname)
 {
        int res = 0;
@@ -3236,7 +3247,7 @@ sql_trans_cname_conflict(sql_table *t, c
        return res;
 }
 
-       static int
+static int
 sql_trans_tname_conflict( sql_trans *tr, sql_schema *s, const char *extra, 
const char *tname, const char *cname)
 {
        char *tp;
@@ -3281,7 +3292,7 @@ sql_trans_tname_conflict( sql_trans *tr,
        return 0;
 }
 
-       static int
+static int
 sql_trans_name_conflict( sql_trans *tr, const char *sname, const char *tname, 
const char *cname)
 {
        char *sp;
@@ -3309,7 +3320,7 @@ sql_trans_name_conflict( sql_trans *tr, 
        return 0;
 }
 
-       sql_column *
+sql_column *
 sql_trans_copy_column( sql_trans *tr, sql_table *t, sql_column *c)
 {
        sqlstore *store = tr->store;
@@ -3319,6 +3330,8 @@ sql_trans_copy_column( sql_trans *tr, sq
        if (t->system && sql_trans_name_conflict(tr, t->s->base.name, 
t->base.name, c->base.name))
                return NULL;
        t = new_table(tr, t);
+       if (!t)
+               return NULL;
        sql_column *col = SA_ZNEW(tr->sa, sql_column);
        base_init(tr->sa, &col->base, 
c->base.id?c->base.id:next_oid(tr->store), TR_NEW, c->base.name);
        dup_sql_type(tr, t->s, &(c->type), &(col->type));
@@ -3351,7 +3364,7 @@ sql_trans_copy_column( sql_trans *tr, sq
        return col;
_______________________________________________
checkin-list mailing list
checkin-list@monetdb.org
https://www.monetdb.org/mailman/listinfo/checkin-list

Reply via email to