add check_for_overlap, and do not suppress SQLITE_DDL_MISUSE thrown b…

db/osqlsqlthr.c

@@ -32,6 +32,7 @@
  */
 
 #include <poll.h>
+#include <strings.h>
 #include <util.h>
 #include <unistd.h>
 #include "sql.h"
@@ -170,11 +171,12 @@ static inline int osql_should_restart(struct sqlclntstate *clnt, int rc,
             }                                                                                                          \
         }                                                                                                              \
         if (rc) {                                                                                                      \
-            logmsg(LOGMSG_ERROR,                                                                                       \
-                   "%s: error writting record to master in offload mode "                                              \
-                   "rc=%d!\n",                                                                                         \
-                   __func__, rc);                                                                                      \
-            if (rc != SQLITE_TOOBIG && rc != ERR_SC)                                                                   \
+            if (rc != SQLITE_DDL_MISUSE)                                                                               \
+                logmsg(LOGMSG_ERROR,                                                                                   \
+                       "%s: error writting record to master in offload mode "                                          \
+                       "rc=%d!\n",                                                                                     \
+                       __func__, rc);                                                                                  \
+            if (rc != SQLITE_TOOBIG && rc != ERR_SC && rc != SQLITE_DDL_MISUSE)                                        \
                 rc = SQLITE_INTERNAL;                                                                                  \
         } else {                                                                                                       \
             rc = SQLITE_OK;                                                                                            \
@@ -1428,6 +1430,33 @@ int osql_sock_abort(struct sqlclntstate *clnt, int type)
  * ***********************************************/
 int gbl_reject_mixed_ddl_dml = 1;
 
+static int check_for_overlap(struct sqlclntstate *clnt, struct schema_change_type *sc)
+{
+    if (!clnt->ddl_tables)
+        return SQLITE_OK;
+
+    if (hash_find_readonly(clnt->ddl_tables, sc->tablename))
+        return SQLITE_DDL_MISUSE;
+
+    /* sqlite can't hold a tablename and an aliasname that collide, so filter
+     * out introduced-name conflicts here before sending to master for SC_ALIASTABLE as well. */
+    const char *newname = NULL;
+    if (sc->kind == SC_ADDTABLE) {
+        newname = sc->tablename;
+    } else if ((sc->kind == SC_RENAMETABLE || sc->kind == SC_ALIASTABLE) &&
+               strcasecmp(sc->tablename, sc->newtable) != 0) {
+        newname = sc->newtable;
+    }
+    if (newname && clnt->ddl_new_tables && hash_find_readonly(clnt->ddl_new_tables, newname))
+        return SQLITE_DDL_MISUSE;
+
+    hash_add(clnt->ddl_tables, strdup(sc->tablename));
+    if (newname && clnt->ddl_new_tables)
+        hash_add(clnt->ddl_new_tables, strdup(newname));
+
+    return SQLITE_OK;
+}
+
 static int osql_send_usedb_logic_int(char *tablename, struct sqlclntstate *clnt,
                                      int nettype)
 {
@@ -1897,10 +1926,8 @@ int osql_schemachange_logic(struct schema_change_type *sc, int usedb)
         }
     }
 
-    if (clnt->dml_tables &&
-        hash_find_readonly(clnt->dml_tables, sc->tablename)) {
-        return SQLITE_DDL_MISUSE;
-    }
+    if ((rc = check_for_overlap(clnt, sc)) != SQLITE_OK)
+        return rc;
 
     if (clnt->remsql_set.is_remsql == IS_REMCREATE) {
         /* this is a distributed create for a partition, creating individual shard here, info passed from
@@ -1930,13 +1957,6 @@ int osql_schemachange_logic(struct schema_change_type *sc, int usedb)
         }
     }
 
-    if (clnt->ddl_tables) {
-        if (hash_find_readonly(clnt->ddl_tables, sc->tablename)) {
-            return SQLITE_DDL_MISUSE;
-        } else
-            hash_add(clnt->ddl_tables, strdup(sc->tablename));
-    }
-
     sc->usedbtablevers = comdb2_table_version(sc->tablename);
 
     if (thd->clnt->dbtran.mode == TRANLEVEL_SOSQL) {

db/sql.h

@@ -949,6 +949,11 @@ struct sqlclntstate {
     hash_t *ddl_tables;
     hash_t *dml_tables;
     hash_t *ddl_contexts;
+    /* Names introduced as brand-new tables in this transaction (CREATE target
+     * and RENAME target). Used to reject two statements introducing the same
+     * name (e.g. "rename a to c; create c"), which otherwise deadlocks the
+     * abort handler at finalize. */
+    hash_t *ddl_new_tables;
 
     int statement_query_effects;
 

db/sqlinterfaces.c

@@ -1610,10 +1610,11 @@ static void sql_update_usertran_state(struct sqlclntstate *clnt)
 
             clnt->in_client_trans = 1;
 
-            assert(clnt->ddl_tables == NULL && clnt->dml_tables == NULL &&
-                   clnt->ddl_contexts == NULL);
+            assert(clnt->ddl_tables == NULL && clnt->dml_tables == NULL && clnt->ddl_contexts == NULL &&
+                   clnt->ddl_new_tables == NULL);
             clnt->ddl_tables = hash_init_strcase(0);
             clnt->dml_tables = hash_init_strcase(0);
+            clnt->ddl_new_tables = hash_init_strcase(0);
             clnt->ddl_contexts = hash_init_strcaseptr(offsetof(struct clnt_ddl_context, name));
         }
     } else if (meta == TSMC_COMMIT) {
@@ -2438,8 +2439,10 @@ int handle_sql_commitrollback(struct sqlthdstate *thd,
 
     destroy_hash(clnt->ddl_tables, free_it);
     destroy_hash(clnt->dml_tables, free_it);
+    destroy_hash(clnt->ddl_new_tables, free_it);
     clnt->ddl_tables = NULL;
     clnt->dml_tables = NULL;
+    clnt->ddl_new_tables = NULL;
     destroy_hash(clnt->ddl_contexts, free_clnt_ddl_context);
     clnt->ddl_contexts = NULL;
 
@@ -5414,8 +5417,10 @@ void cleanup_clnt(struct sqlclntstate *clnt)
 
     destroy_hash(clnt->ddl_tables, free_it);
     destroy_hash(clnt->dml_tables, free_it);
+    destroy_hash(clnt->ddl_new_tables, free_it);
     clnt->ddl_tables = NULL;
     clnt->dml_tables = NULL;
+    clnt->ddl_new_tables = NULL;
     destroy_hash(clnt->ddl_contexts, free_clnt_ddl_context);
     clnt->ddl_contexts = NULL;
 

tests/renametable.test/reqoutput.txt

@@ -113,6 +113,8 @@
 (name='$1_5DD5C692')
 (name='$2_E5D0B3CA')
 (name='talias2')
+[alter table ra rename to ra2] rc 0
+[alter table ra2 rename to ra] rc 0
 [begin] rc 0
 [drop table t] rc 0
 [alter table t2 rename to t] rc 0
@@ -127,3 +129,19 @@
 [alter table t rename to dummy] rc 0
 [alter table t2 rename to t] rc 0
 [commit] rc 0
+[commit] failed with rc -5 Transactional DDL Error: Overlapping Tables
+[begin] rc 0
+[create table tr2 {schema{int i}}] rc 0
+[alter table tr rename to tr2] rc 0
+[commit] failed with rc -5 Transactional DDL Error: Overlapping Tables
+[begin] rc 0
+[alter table tr rename to tr2] rc 0
+[create table tr2(i int)] rc 0
+[commit] failed with rc -5 Transactional DDL Error: Overlapping Tables
+[begin] rc 0
+[alter table tr rename to tr2] rc 0
+[truncate tr] rc 0
+[commit] failed with rc -5 Transactional DDL Error: Overlapping Tables
+[begin] rc 0
+[truncate tr] rc 0
+[alter table tr rename to tr2] rc 0

tests/renametable.test/reqoutput_lightweight.txt

@@ -113,6 +113,8 @@
 (name='$1_5DD5C692')
 (name='$2_E5D0B3CA')
 (name='talias2')
+[alter table ra rename to ra2] rc 0
+[alter table ra2 rename to ra] rc 0
 [commit] failed with rc -3 New table name already exists
 [begin] rc 0
 [drop table t] rc 0
@@ -127,3 +129,19 @@
 [begin] rc 0
 [alter table t rename to dummy] rc 0
 [alter table t2 rename to t] rc 0
+[commit] failed with rc -5 Transactional DDL Error: Overlapping Tables
+[begin] rc 0
+[create table tr2 {schema{int i}}] rc 0
+[alter table tr rename to tr2] rc 0
+[commit] failed with rc -5 Transactional DDL Error: Overlapping Tables
+[begin] rc 0
+[alter table tr rename to tr2] rc 0
+[create table tr2(i int)] rc 0
+[commit] failed with rc -5 Transactional DDL Error: Overlapping Tables
+[begin] rc 0
+[alter table tr rename to tr2] rc 0
+[truncate tr] rc 0
+[commit] failed with rc -5 Transactional DDL Error: Overlapping Tables
+[begin] rc 0
+[truncate tr] rc 0
+[alter table tr rename to tr2] rc 0

tests/renametable.test/runit

@@ -107,6 +107,17 @@ if (( $? != 0 )) ; then
    exit 1
 fi
 
+# Tests "removing an alias" exception in sqlite3AlterRenameTable
+# (comdb2build.c): get_dbtable_by_name(target) finds the same table via its
+# physical name, which must NOT be flagged as a collision.
+cdb2sql ${CDB2_OPTIONS} $dbnm default "drop table if exists ra"
+cdb2sql ${CDB2_OPTIONS} $dbnm default "create table ra(i int)"
+cat <<EOF | cdb2sql ${CDB2_OPTIONS} $dbnm default - >> $OUT 2>&1
+alter table ra rename to ra2\$\$
+alter table ra2 rename to ra\$\$
+EOF
+cdb2sql ${CDB2_OPTIONS} $dbnm default "drop table if exists ra"
+
 # Rename after dropping table with target name in txn
 cdb2sql ${CDB2_OPTIONS} $dbnm default "create table t2(i int)"
 cat <<EOF | cdb2sql ${CDB2_OPTIONS} $dbnm default - >> $OUT 2>&1
@@ -141,6 +152,53 @@ alter table t2 rename to t\$\$
 commit
 EOF
 
+# TEST TRAN: CREATE TR2; RENAME TR->TR2 (expected: fail, must not deadlock)
+cdb2sql ${CDB2_OPTIONS} $dbnm default "drop table if exists tr"
+cdb2sql ${CDB2_OPTIONS} $dbnm default "drop table if exists tr2"
+cdb2sql ${CDB2_OPTIONS} $dbnm default "create table tr {schema{int i}}"
+cat <<EOF | timeout --kill-after=5s 60s cdb2sql ${CDB2_OPTIONS} $dbnm default - >> $OUT 2>&1
+begin
+create table tr2 {schema{int i}}\$\$
+alter table tr rename to tr2\$\$
+commit
+EOF
+
+# TEST TRAN: RENAME TR->TR2; CREATE TR2 (expected: fail, must not deadlock)
+# should fail, check comment in osqlsqlthr.c:check_for_overlap
+# this creates a sqlaliasname tr2 for tr, but when we create tr2, sqlite will err
+cdb2sql ${CDB2_OPTIONS} $dbnm default "drop table if exists tr"
+cdb2sql ${CDB2_OPTIONS} $dbnm default "drop table if exists tr2"
+cdb2sql ${CDB2_OPTIONS} $dbnm default "create table tr {schema{int i}}"
+cat <<EOF | timeout --kill-after=5s 60s cdb2sql ${CDB2_OPTIONS} $dbnm default - >> $OUT 2>&1
+begin
+alter table tr rename to tr2\$\$
+create table tr2(i int)\$\$
+commit
+EOF
+
+# TEST TRAN: RENAME TR->TR2; TRUNCATE TR (DDL on introduced name) (expected: fail, must not deadlock)
+cdb2sql ${CDB2_OPTIONS} $dbnm default "drop table if exists tr"
+cdb2sql ${CDB2_OPTIONS} $dbnm default "drop table if exists tr2"
+cdb2sql ${CDB2_OPTIONS} $dbnm default "create table tr {schema{int i}}"
+cat <<EOF | timeout --kill-after=5s 60s cdb2sql ${CDB2_OPTIONS} $dbnm default - >> $OUT 2>&1
+begin
+alter table tr rename to tr2\$\$
+truncate tr
+commit
+EOF
+
+# TEST TRAN: TRUNCATE TR; RENAME TR->TR2 (DDL on introduced name) (expected: fail, must not deadlock)
+cdb2sql ${CDB2_OPTIONS} $dbnm default "drop table if exists tr"
+cdb2sql ${CDB2_OPTIONS} $dbnm default "drop table if exists tr2"
+cdb2sql ${CDB2_OPTIONS} $dbnm default "create table tr {schema{int i}}"
+cat <<EOF | timeout --kill-after=5s 60s cdb2sql ${CDB2_OPTIONS} $dbnm default - >> $OUT 2>&1
+begin
+truncate tr
+alter table tr rename to tr2\$\$
+commit
+EOF
+
+
 df=$(diff $OUT $EXP)
 if [ $? -ne 0 ] ; then
      echo "  ^^^^^^^^^^^^"

tests/sc_transactional.test/runit

@@ -3,7 +3,7 @@ bash -n "$0" | exit 1
 
 source ${TESTSROOTDIR}/tools/runit_common.sh
 #PS4='$(date +%Y%m%d-%H:%M:%S): '
-set -x
+
 
 dbnm=$1
 
@@ -317,4 +317,34 @@ do_verify t1
 do_verify t2
 do_verify t3
 
+# skip this test if rowlocks, cause this will set the isolation level to SNAPISOL and 
+# so this will skip SOSQL DDL/DML conflict detection step
+if [[ x"$dbnm" != x*"rowlocks"* ]]; then
+
+echo "TEST TRAN: TRUNCATE T; INSERT T (same table, DDL then DML) (expected: fail, must not deadlock)"
+cdb2sql ${CDB2_OPTIONS} $dbnm default "drop table if exists tdml"
+cdb2sql ${CDB2_OPTIONS} $dbnm default "create table tdml(i int)"
+cdb2sql ${CDB2_OPTIONS} $dbnm default "insert into tdml values(1)"
+assertcnt tdml 1
+
+# ddl-dml overlap
+out=$(timeout --kill-after=5s 60s cdb2sql ${CDB2_OPTIONS} $dbnm default - <<"EOF" 2>&1
+begin
+truncate tdml
+insert into tdml values(2)
+commit
+EOF
+)
+echo "$out"
+if ! echo "$out" | grep -q "Transactional DDL Error"; then
+    failexit "TRUNCATE T; INSERT T did not report a transactional DDL error: $out"
+fi
+# table must survive with original row intact
+assertcnt tdml 1
+do_verify tdml
+
+else
+    echo "TEST TRAN: TRUNCATE T; INSERT T skipped (rowlocks/SNAPISOL mode bypasses SOSQL DDL/DML conflict detection)"
+fi
+
 echo "Success"