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