Changeset: cba5fe23a059 for MonetDB
URL: https://dev.monetdb.org/hg/MonetDB/rev/cba5fe23a059
Modified Files:
sql/backends/monet5/sql.c
sql/backends/monet5/sql_cat.c
sql/backends/monet5/sql_statistics.c
sql/backends/monet5/sql_user.c
sql/server/sql_privileges.c
sql/storage/bat/bat_table.c
Branch: Jul2021
Log Message:
Adding missing transaction conflict checks
diffs (truncated from 361 to 300 lines):
diff --git a/sql/backends/monet5/sql.c b/sql/backends/monet5/sql.c
--- a/sql/backends/monet5/sql.c
+++ b/sql/backends/monet5/sql.c
@@ -1669,8 +1669,8 @@ mvc_append_column(sql_trans *t, sql_colu
{
sqlstore *store = t->store;
int res = store->storage_api.append_col(t, c, pos, ins, TYPE_bat, 0);
- if (res != LOG_OK)
- throw(SQL, "sql.append", SQLSTATE(42000) "Cannot append
values");
+ if (res != LOG_OK) /* the conflict case should never happen, but leave
it here */
+ throw(SQL, "sql.append", SQLSTATE(42000) "Append failed%s", res
== LOG_CONFLICT ? " due to conflict with another transaction" : "");
return MAL_SUCCEED;
}
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
@@ -1644,7 +1644,7 @@ SQLcomment_on(Client cntxt, MalBlkPtr mb
}
}
if (ok != LOG_OK)
- throw(SQL, "sql.comment_on", SQLSTATE(3F000) "operation
failed");
+ throw(SQL, "sql.comment_on", SQLSTATE(42000) "Comment on
failed%s", ok == LOG_CONFLICT ? " due to conflict with another transaction" :
"");
return MAL_SUCCEED;
}
diff --git a/sql/backends/monet5/sql_statistics.c
b/sql/backends/monet5/sql_statistics.c
--- a/sql/backends/monet5/sql_statistics.c
+++ b/sql/backends/monet5/sql_statistics.c
@@ -30,6 +30,7 @@ sql_drop_statistics(mvc *m, sql_table *t
sql_table *sysstats;
sql_column *statsid;
oid rid;
+ int log_res = LOG_OK;
tr = m->session->tr;
sys = mvc_bind_schema(m, "sys");
@@ -64,9 +65,8 @@ sql_drop_statistics(mvc *m, sql_table *t
sql_column *c = ncol->data;
rid = store->table_api.column_find_row(tr, statsid,
&c->base.id, NULL);
- if (!is_oid_nil(rid) &&
- store->table_api.table_delete(tr, sysstats, rid) !=
LOG_OK)
- throw(SQL, "sql_drop_statistics", "delete
failed");
+ if (!is_oid_nil(rid) && (log_res =
store->table_api.table_delete(tr, sysstats, rid)) != LOG_OK)
+ throw(SQL, "sql.sql_drop_statistics",
SQLSTATE(42000) "DROP STATISTICS: failed%s", log_res == LOG_CONFLICT ? " due to
conflict with another transaction" : "");
}
}
return MAL_SUCCEED;
@@ -89,7 +89,7 @@ sql_analyze(Client cntxt, MalBlkPtr mb,
int argc = pci->argc;
int width = 0;
int minmax = *getArgReference_int(stk, pci, 1);
- int sfnd = 0, tfnd = 0, cfnd = 0;
+ int sfnd = 0, tfnd = 0, cfnd = 0, log_res = LOG_OK;
sql_schema *sys;
sql_table *sysstats;
sql_column *statsid;
@@ -302,15 +302,15 @@ sql_analyze(Client cntxt, MalBlkPtr mb,
}
BBPunfix(bn->batCacheid);
ts = timestamp_current();
- if (!is_oid_nil(rid) &&
store->table_api.table_delete(tr, sysstats, rid) != LOG_OK) {
+ if (!is_oid_nil(rid) && (log_res =
store->table_api.table_delete(tr, sysstats, rid)) != LOG_OK) {
GDKfree(maxval);
GDKfree(minval);
- throw(SQL, "analyze", "delete
failed");
+ throw(SQL, "analyze",
SQLSTATE(42000) "ANALYZE: failed%s", log_res == LOG_CONFLICT ? " due to
conflict with another transaction" : "");
}
- if (store->table_api.table_insert(tr,
sysstats, &c->base.id, &c->type.type->sqlname, &width, &ts, samplesize ?
&samplesize : &sz, &sz, &uniq, &nils, &minval, &maxval, &sorted, &revsorted) !=
LOG_OK) {
+ if ((log_res =
store->table_api.table_insert(tr, sysstats, &c->base.id,
&c->type.type->sqlname, &width, &ts, samplesize ? &samplesize : &sz, &sz,
&uniq, &nils, &minval, &maxval, &sorted, &revsorted)) != LOG_OK) {
GDKfree(maxval);
GDKfree(minval);
- throw(SQL, "analyze", "insert
failed");
+ throw(SQL, "analyze",
SQLSTATE(42000) "ANALYZE: failed%s", log_res == LOG_CONFLICT ? " due to
conflict with another transaction" : "");
}
}
}
diff --git a/sql/backends/monet5/sql_user.c b/sql/backends/monet5/sql_user.c
--- a/sql/backends/monet5/sql_user.c
+++ b/sql/backends/monet5/sql_user.c
@@ -28,14 +28,22 @@ static int
monet5_drop_user(ptr _mvc, str user)
{
mvc *m = (mvc *) _mvc;
- oid rid;
- sql_schema *sys;
- sql_table *users;
- sql_column *users_name;
+ oid rid, grant_user;
+ sql_schema *sys = find_sql_schema(m->session->tr, "sys");
+ sql_table *users = find_sql_table(m->session->tr, sys, "db_user_info");
+ sql_column *users_name = find_sql_column(users, "name");
str err;
Client c = MCgetClient(m->clientid);
+ sqlstore *store = m->session->tr->store;
+ int log_res = LOG_OK;
- oid grant_user = c->user;
+ rid = store->table_api.column_find_row(m->session->tr, users_name,
user, NULL);
+ if (!is_oid_nil(rid) && (log_res =
store->table_api.table_delete(m->session->tr, users, rid)) != LOG_OK) {
+ (void) sql_error(m, 02, "DROP USER: failed%s", log_res ==
LOG_CONFLICT ? " due to conflict with another transaction" : "");
+ return FALSE;
+ }
+
+ grant_user = c->user;
c->user = MAL_ADMIN;
err = AUTHremoveUser(c, user);
c->user = grant_user;
@@ -44,14 +52,6 @@ monet5_drop_user(ptr _mvc, str user)
freeException(err);
return FALSE;
}
- sys = find_sql_schema(m->session->tr, "sys");
- users = find_sql_table(m->session->tr, sys, "db_user_info");
- users_name = find_sql_column(users, "name");
-
- sqlstore *store = m->session->tr->store;
- rid = store->table_api.column_find_row(m->session->tr, users_name,
user, NULL);
- if (!is_oid_nil(rid))
- store->table_api.table_delete(m->session->tr, users, rid);
/* FIXME: We have to ignore this inconsistency here, because the
* user was already removed from the system authorisation. Once
* we have warnings, we could issue a warning about this
diff --git a/sql/server/sql_privileges.c b/sql/server/sql_privileges.c
--- a/sql/server/sql_privileges.c
+++ b/sql/server/sql_privileges.c
@@ -215,8 +215,8 @@ sql_grant_func_privs( mvc *sql, char *gr
return NULL;
}
-static void
-sql_delete_priv(mvc *sql, sqlid auth_id, sqlid obj_id, int privilege, sqlid
grantor, int grantable)
+static char *
+sql_delete_priv(mvc *sql, sqlid auth_id, sqlid obj_id, int privilege, sqlid
grantor, int grantable, const char *op, const char *call)
{
sql_schema *ss = mvc_bind_schema(sql, "sys");
sql_table *privs = find_sql_table(sql->session->tr, ss, "privileges");
@@ -227,6 +227,7 @@ sql_delete_priv(mvc *sql, sqlid auth_id,
sqlstore *store = tr->store;
rids *A;
oid rid = oid_nil;
+ int log_res = LOG_OK;
(void) grantor;
(void) grantable;
@@ -235,9 +236,12 @@ sql_delete_priv(mvc *sql, sqlid auth_id,
A = store->table_api.rids_select(tr, priv_auth, &auth_id, &auth_id,
priv_priv, &privilege, &privilege, priv_obj, &obj_id, &obj_id, NULL );
/* remove them */
- for(rid = store->table_api.rids_next(A); !is_oid_nil(rid); rid =
store->table_api.rids_next(A))
- store->table_api.table_delete(tr, privs, rid);
+ for(rid = store->table_api.rids_next(A); !is_oid_nil(rid) && log_res ==
LOG_OK; rid = store->table_api.rids_next(A))
+ log_res = store->table_api.table_delete(tr, privs, rid);
store->table_api.rids_destroy(A);
+ if (log_res != LOG_OK)
+ throw(SQL, op, SQLSTATE(42000) "%s: failed%s", call, log_res ==
LOG_CONFLICT ? " due to conflict with another transaction" : "");
+ return NULL;
}
char *
@@ -257,8 +261,7 @@ sql_revoke_global_privs( mvc *sql, char
grantee_id = sql_find_auth(sql, grantee);
if (grantee_id <= 0)
throw(SQL, "sql.revoke_global", SQLSTATE(01006) "REVOKE:
User/role '%s' unknown", grantee);
- sql_delete_priv(sql, grantee_id, GLOBAL_OBJID, privs, grantor, grant);
- return NULL;
+ return sql_delete_priv(sql, grantee_id, GLOBAL_OBJID, privs, grantor,
grant, "sql.revoke_global", "REVOKE");
}
char *
@@ -269,6 +272,7 @@ sql_revoke_table_privs( mvc *sql, char *
bool allowed;
sqlid grantee_id;
int all = PRIV_SELECT | PRIV_UPDATE | PRIV_INSERT | PRIV_DELETE |
PRIV_TRUNCATE;
+ char *msg = NULL;
if (!(t = find_table_or_view_on_scope(sql, NULL, sname, tname,
"REVOKE", false)))
throw(SQL,"sql.revoke_table","%s", sql->errstr);
@@ -298,17 +302,18 @@ sql_revoke_table_privs( mvc *sql, char *
if (grantee_id <= 0)
throw(SQL,"sql.revoke_table", SQLSTATE(01006) "REVOKE:
User/role '%s' unknown", grantee);
if (privs == all) {
- sql_delete_priv(sql, grantee_id, t->base.id, PRIV_SELECT,
grantor, grant);
- sql_delete_priv(sql, grantee_id, t->base.id, PRIV_UPDATE,
grantor, grant);
- sql_delete_priv(sql, grantee_id, t->base.id, PRIV_INSERT,
grantor, grant);
- sql_delete_priv(sql, grantee_id, t->base.id, PRIV_DELETE,
grantor, grant);
- sql_delete_priv(sql, grantee_id, t->base.id, PRIV_TRUNCATE,
grantor, grant);
+ if ((msg = sql_delete_priv(sql, grantee_id, t->base.id,
PRIV_SELECT, grantor, grant, "sql.revoke_table", "REVOKE")) ||
+ (msg = sql_delete_priv(sql, grantee_id, t->base.id,
PRIV_UPDATE, grantor, grant, "sql.revoke_table", "REVOKE")) ||
+ (msg = sql_delete_priv(sql, grantee_id, t->base.id,
PRIV_INSERT, grantor, grant, "sql.revoke_table", "REVOKE")) ||
+ (msg = sql_delete_priv(sql, grantee_id, t->base.id,
PRIV_DELETE, grantor, grant, "sql.revoke_table", "REVOKE")) ||
+ (msg = sql_delete_priv(sql, grantee_id, t->base.id,
PRIV_TRUNCATE, grantor, grant, "sql.revoke_table", "REVOKE")))
+ return msg;
} else if (!c) {
- sql_delete_priv(sql, grantee_id, t->base.id, privs, grantor,
grant);
+ msg = sql_delete_priv(sql, grantee_id, t->base.id, privs,
grantor, grant, "sql.revoke_table", "REVOKE");
} else {
- sql_delete_priv(sql, grantee_id, c->base.id, privs, grantor,
grant);
+ msg = sql_delete_priv(sql, grantee_id, c->base.id, privs,
grantor, grant, "sql.revoke_table", "REVOKE");
}
- return NULL;
+ return msg;
}
char *
@@ -334,8 +339,7 @@ sql_revoke_func_privs( mvc *sql, char *g
grantee_id = sql_find_auth(sql, grantee);
if (grantee_id <= 0)
throw(SQL, "sql.revoke_func", SQLSTATE(01006) "REVOKE:
User/role '%s' unknown", grantee);
- sql_delete_priv(sql, grantee_id, f->base.id, privs, grantor, grant);
- return NULL;
+ return sql_delete_priv(sql, grantee_id, f->base.id, privs, grantor,
grant, "sql.revoke_func", "REVOKE");
}
static bool
@@ -376,18 +380,22 @@ sql_drop_role(mvc *m, str auth)
sqlstore *store = m->session->tr->store;
rids *A;
oid rid;
+ int log_res = LOG_OK;
rid = store->table_api.column_find_row(tr, find_sql_column(auths,
"name"), auth, NULL);
if (is_oid_nil(rid))
throw(SQL, "sql.drop_role", SQLSTATE(0P000) "DROP ROLE: no such
role '%s'", auth);
- store->table_api.table_delete(m->session->tr, auths, rid);
+ if ((log_res = store->table_api.table_delete(m->session->tr, auths,
rid)) != LOG_OK)
+ throw(SQL, "sql.drop_role", SQLSTATE(42000) "DROP ROLE:
failed%s", log_res == LOG_CONFLICT ? " due to conflict with another
transaction" : "");
/* select user roles of this role_id */
A = store->table_api.rids_select(tr, find_sql_column(user_roles,
"role_id"), &role_id, &role_id, NULL);
/* remove them */
- for(rid = store->table_api.rids_next(A); !is_oid_nil(rid); rid =
store->table_api.rids_next(A))
- store->table_api.table_delete(tr, user_roles, rid);
+ for(rid = store->table_api.rids_next(A); !is_oid_nil(rid) && log_res ==
LOG_OK; rid = store->table_api.rids_next(A))
+ log_res = store->table_api.table_delete(tr, user_roles, rid);
store->table_api.rids_destroy(A);
+ if (log_res != LOG_OK)
+ throw(SQL, "sql.drop_role", SQLSTATE(42000) "DROP ROLE:
failed%s", log_res == LOG_CONFLICT ? " due to conflict with another
transaction" : "");
return NULL;
}
@@ -553,6 +561,7 @@ sql_revoke_role(mvc *m, str grantee, str
sql_column *roles_login_id = find_sql_column(roles, "login_id");
sqlid role_id, grantee_id;
sqlstore *store = m->session->tr->store;
+ int log_res = LOG_OK;
rid = store->table_api.column_find_row(m->session->tr, auths_name,
grantee, NULL);
if (is_oid_nil(rid))
@@ -567,15 +576,17 @@ sql_revoke_role(mvc *m, str grantee, str
if (!admin) {
rid = store->table_api.column_find_row(m->session->tr,
roles_login_id, &grantee_id, roles_role_id, &role_id, NULL);
- if (!is_oid_nil(rid))
- store->table_api.table_delete(m->session->tr, roles,
rid);
- else
+ if (!is_oid_nil(rid)) {
+ if ((log_res =
store->table_api.table_delete(m->session->tr, roles, rid)) != LOG_OK)
+ throw(SQL, "sql.revoke_role", SQLSTATE(42000)
"REVOKE: failed%s", log_res == LOG_CONFLICT ? " due to conflict with another
transaction" : "");
+ } else
throw(SQL,"sql.revoke_role", SQLSTATE(01006) "REVOKE:
User '%s' does not have ROLE '%s'", grantee, role);
}
rid = sql_privilege_rid(m, grantee_id, role_id, PRIV_ROLE_ADMIN);
- if (!is_oid_nil(rid))
- store->table_api.table_delete(m->session->tr, privs, rid);
- else if (admin)
+ if (!is_oid_nil(rid)) {
+ if ((log_res = store->table_api.table_delete(m->session->tr,
privs, rid)) != LOG_OK)
+ throw(SQL, "sql.revoke_role", SQLSTATE(42000) "REVOKE:
failed%s", log_res == LOG_CONFLICT ? " due to conflict with another
transaction" : "");
+ } else if (admin)
throw(SQL,"sql.revoke_role", SQLSTATE(01006) "REVOKE: User '%s'
does not have ROLE '%s'", grantee, role);
return NULL;
}
@@ -769,6 +780,8 @@ sql_drop_granted_users(mvc *sql, sqlid u
sqlstore *store = tr->store;
rids *A;
oid rid;
+ int log_res = LOG_OK;
+ char *msg = NULL;
if (!list_find(deleted_users, &user_id, (fcmp) &id_cmp)) {
if (mvc_check_dependency(sql, user_id, OWNER_DEPENDENCY, NULL))
@@ -779,45 +792,54 @@ sql_drop_granted_users(mvc *sql, sqlid u
/* select privileges of this user_id */
A = store->table_api.rids_select(tr, find_sql_column(privs,
"auth_id"), &user_id, &user_id, NULL);
/* remove them */
- for(rid = store->table_api.rids_next(A); !is_oid_nil(rid); rid
= store->table_api.rids_next(A))
- store->table_api.table_delete(tr, privs, rid);
+ for(rid = store->table_api.rids_next(A); !is_oid_nil(rid) &&
log_res == LOG_OK; rid = store->table_api.rids_next(A))
+ log_res = store->table_api.table_delete(tr, privs, rid);
store->table_api.rids_destroy(A);
+ if (log_res != LOG_OK)
+ throw(SQL, "sql.drop_user", SQLSTATE(42000) "DROP USER:
failed%s", log_res == LOG_CONFLICT ? " due to conflict with another
transaction" : "");
/* select privileges granted by this user_id */
A = store->table_api.rids_select(tr, find_sql_column(privs,
"grantor"), &user_id, &user_id, NULL);
/* remove them */
- for(rid = store->table_api.rids_next(A); !is_oid_nil(rid); rid
= store->table_api.rids_next(A))
- store->table_api.table_delete(tr, privs, rid);
_______________________________________________
checkin-list mailing list
[email protected]
https://www.monetdb.org/mailman/listinfo/checkin-list
