Small fixes for remtran over cdb2api:

- nop remote txn do not try to open a connection (right now it crashes because no handle) - reset remsql_set struct on client reset (need to reset is_schema field) - fix tablename copy for remsql_set re: 177787824 Signed-off-by: Dorin Hogea <[email protected]>
bloomberg · Jan 6, 2025 · ab27b58 · ab27b58
1 parent e4e357f
commit ab27b58
Show file tree

Hide file tree

Showing 4 changed files with 70 additions and 53 deletions.
diff --git a/db/fdb_fend.c b/db/fdb_fend.c
@@ -4209,7 +4209,13 @@ int fdb_trans_commit(sqlclntstate *clnt, enum trans_clntcomm sideeffects)
             continue;
 
         if (tran->is_cdb2api) {
-            rc = cdb2_run_statement(tran->fcon.hndl, "commit");
+            if (tran->nwrites) {
+                /* handle is only created upon first remote write to this fdb */
+                assert(tran->fcon.hndl);
+                rc = cdb2_run_statement(tran->fcon.hndl, "commit");
+            } else {
+                rc = 0;
+            }
         } else {
             rc = fdb_send_commit(msg, tran, clnt->dbtran.mode, tran->fcon.sb);
         }
@@ -4324,7 +4330,13 @@ int fdb_trans_rollback(sqlclntstate *clnt)
     LISTC_FOR_EACH(&dtran->fdb_trans, tran, lnk)
     {
         if (tran->is_cdb2api) {
-            rc = cdb2_run_statement(tran->fcon.hndl, "rollback");
+            if (tran->nwrites) {
+                /* handle is only created upon first remote write to this fdb */
+                assert(tran->fcon.hndl);
+                rc = cdb2_run_statement(tran->fcon.hndl, "rollback");
+            } else {
+                rc = 0;
+            }
         } else {
             rc = fdb_send_rollback(msg, tran, clnt->dbtran.mode, tran->fcon.sb);
         }
@@ -5866,8 +5878,8 @@ int process_fdb_set_cdb2api(sqlclntstate *clnt, char *sqlstr, char *err,
             return -1;
         }
 
+        bzero(clnt->remsql_set.tablename, sizeof(clnt->remsql_set.tablename));
         memcpy(clnt->remsql_set.tablename, sqlstr, tbllen-1);
-        clnt->remsql_set.tablename[sizeof(clnt->remsql_set.tablename) - 1] = '\0';
 
         sqlstr =  ptr;
         if (sqlstr[0] != ' ') {

diff --git a/db/sqlinterfaces.c b/db/sqlinterfaces.c
@@ -5538,7 +5538,7 @@ void reset_clnt(struct sqlclntstate *clnt, int initial)
         bdb_unregister_modsnap(thedb->bdb_env, clnt->modsnap_registration);
         clnt->modsnap_registration = NULL;
     }
-    clnt->remsql_set.is_remsql = 0;
+    bzero(&clnt->remsql_set, sizeof(clnt->remsql_set));
 }
 
 void reset_clnt_flags(struct sqlclntstate *clnt)

diff --git a/tests/fdb_compat.test/output.log b/tests/fdb_compat.test/output.log
@@ -1,6 +1,8 @@
 Current fdb version 8
 (rows inserted=3)
 (rows inserted=3)
+(rows deleted=3)
+(rows inserted=3)
 TEST 1
 remsql run against same version
 (id=11)
@@ -22,10 +24,10 @@ Table "sqlite_stat1" Rootp 1073741832 Remrootp 4 Version=0')
 Table "sqlite_stat4" Rootp 1073741833 Remrootp 5 Version=0')
 Index "$ID_52596C31" for table "t" Rootp 1073741831 Remrootp 3 Version=0')
 Table "t" Rootp 1073741830 Remrootp 2 Version=0')
-Table "sqlite_stat1" Rootp 1073741828 Remrootp 4 Version=0')
-Table "sqlite_stat4" Rootp 1073741829 Remrootp 5 Version=0')
-Index "$ID_52596C31" for table "t" Rootp 1073741827 Remrootp 3 Version=0')
-Table "t" Rootp 1073741826 Remrootp 2 Version=0')
+Table "sqlite_stat1" Rootp 1073741832 Remrootp 4 Version=0')
+Table "sqlite_stat4" Rootp 1073741833 Remrootp 5 Version=0')
+Index "$ID_52596C31" for table "t" Rootp 1073741831 Remrootp 3 Version=0')
+Table "t" Rootp 1073741830 Remrootp 2 Version=0')
 TEST 3
 test against a too new version
 (id=11)
@@ -38,10 +40,10 @@ Table "sqlite_stat1" Rootp 1073741836 Remrootp 4 Version=0')
 Table "sqlite_stat4" Rootp 1073741837 Remrootp 5 Version=0')
 Index "$ID_52596C31" for table "t" Rootp 1073741835 Remrootp 3 Version=0')
 Table "t" Rootp 1073741834 Remrootp 2 Version=0')
-Table "sqlite_stat1" Rootp 1073741832 Remrootp 4 Version=0')
-Table "sqlite_stat4" Rootp 1073741833 Remrootp 5 Version=0')
-Index "$ID_52596C31" for table "t" Rootp 1073741831 Remrootp 3 Version=0')
-Table "t" Rootp 1073741830 Remrootp 2 Version=0')
+Table "sqlite_stat1" Rootp 1073741836 Remrootp 4 Version=0')
+Table "sqlite_stat4" Rootp 1073741837 Remrootp 5 Version=0')
+Index "$ID_52596C31" for table "t" Rootp 1073741835 Remrootp 3 Version=0')
+Table "t" Rootp 1073741834 Remrootp 2 Version=0')
 TEST 4
 test against cdb2api remsql but not remtran
 (id=11)
@@ -54,10 +56,10 @@ Table "sqlite_stat1" Rootp 1073741840 Remrootp 4 Version=0')
 Table "sqlite_stat4" Rootp 1073741841 Remrootp 5 Version=0')
 Index "$ID_52596C31" for table "t" Rootp 1073741839 Remrootp 3 Version=0')
 Table "t" Rootp 1073741838 Remrootp 2 Version=0')
-Table "sqlite_stat1" Rootp 1073741836 Remrootp 4 Version=0')
-Table "sqlite_stat4" Rootp 1073741837 Remrootp 5 Version=0')
-Index "$ID_52596C31" for table "t" Rootp 1073741835 Remrootp 3 Version=0')
-Table "t" Rootp 1073741834 Remrootp 2 Version=0')
+Table "sqlite_stat1" Rootp 1073741840 Remrootp 4 Version=0')
+Table "sqlite_stat4" Rootp 1073741841 Remrootp 5 Version=0')
+Index "$ID_52596C31" for table "t" Rootp 1073741839 Remrootp 3 Version=0')
+Table "t" Rootp 1073741838 Remrootp 2 Version=0')
 TEST 5
 test insert, delete, update current version
 (rows inserted=10)
@@ -66,30 +68,30 @@ Table "sqlite_stat1" Rootp 1073741844 Remrootp 4 Version=0')
 Table "sqlite_stat4" Rootp 1073741845 Remrootp 5 Version=0')
 Index "$ID_52596C31" for table "t" Rootp 1073741843 Remrootp 3 Version=0')
 Table "t" Rootp 1073741842 Remrootp 2 Version=0')
-Table "sqlite_stat1" Rootp 1073741840 Remrootp 4 Version=0')
-Table "sqlite_stat4" Rootp 1073741841 Remrootp 5 Version=0')
-Index "$ID_52596C31" for table "t" Rootp 1073741839 Remrootp 3 Version=0')
-Table "t" Rootp 1073741838 Remrootp 2 Version=0')
+Table "sqlite_stat1" Rootp 1073741844 Remrootp 4 Version=0')
+Table "sqlite_stat4" Rootp 1073741845 Remrootp 5 Version=0')
+Index "$ID_52596C31" for table "t" Rootp 1073741843 Remrootp 3 Version=0')
+Table "t" Rootp 1073741842 Remrootp 2 Version=0')
 (rows updated=10)
 (rows updated=10)
 Table "sqlite_stat1" Rootp 1073741848 Remrootp 4 Version=0')
 Table "sqlite_stat4" Rootp 1073741849 Remrootp 5 Version=0')
 Index "$ID_52596C31" for table "t" Rootp 1073741847 Remrootp 3 Version=0')
 Table "t" Rootp 1073741846 Remrootp 2 Version=0')
-Table "sqlite_stat1" Rootp 1073741844 Remrootp 4 Version=0')
-Table "sqlite_stat4" Rootp 1073741845 Remrootp 5 Version=0')
-Index "$ID_52596C31" for table "t" Rootp 1073741843 Remrootp 3 Version=0')
-Table "t" Rootp 1073741842 Remrootp 2 Version=0')
+Table "sqlite_stat1" Rootp 1073741848 Remrootp 4 Version=0')
+Table "sqlite_stat4" Rootp 1073741849 Remrootp 5 Version=0')
+Index "$ID_52596C31" for table "t" Rootp 1073741847 Remrootp 3 Version=0')
+Table "t" Rootp 1073741846 Remrootp 2 Version=0')
 (rows deleted=10)
 (rows deleted=10)
 Table "sqlite_stat1" Rootp 1073741852 Remrootp 4 Version=0')
 Table "sqlite_stat4" Rootp 1073741853 Remrootp 5 Version=0')
 Index "$ID_52596C31" for table "t" Rootp 1073741851 Remrootp 3 Version=0')
 Table "t" Rootp 1073741850 Remrootp 2 Version=0')
-Table "sqlite_stat1" Rootp 1073741848 Remrootp 4 Version=0')
-Table "sqlite_stat4" Rootp 1073741849 Remrootp 5 Version=0')
-Index "$ID_52596C31" for table "t" Rootp 1073741847 Remrootp 3 Version=0')
-Table "t" Rootp 1073741846 Remrootp 2 Version=0')
+Table "sqlite_stat1" Rootp 1073741852 Remrootp 4 Version=0')
+Table "sqlite_stat4" Rootp 1073741853 Remrootp 5 Version=0')
+Index "$ID_52596C31" for table "t" Rootp 1073741851 Remrootp 3 Version=0')
+Table "t" Rootp 1073741850 Remrootp 2 Version=0')
 TEST 6
 remtran run against a pre cdb2api version
 (rows inserted=10)
@@ -98,30 +100,30 @@ Table "sqlite_stat1" Rootp 1073741856 Remrootp 4 Version=0')
 Table "sqlite_stat4" Rootp 1073741857 Remrootp 5 Version=0')
 Index "$ID_52596C31" for table "t" Rootp 1073741855 Remrootp 3 Version=0')
 Table "t" Rootp 1073741854 Remrootp 2 Version=0')
-Table "sqlite_stat1" Rootp 1073741852 Remrootp 4 Version=0')
-Table "sqlite_stat4" Rootp 1073741853 Remrootp 5 Version=0')
-Index "$ID_52596C31" for table "t" Rootp 1073741851 Remrootp 3 Version=0')
-Table "t" Rootp 1073741850 Remrootp 2 Version=0')
+Table "sqlite_stat1" Rootp 1073741856 Remrootp 4 Version=0')
+Table "sqlite_stat4" Rootp 1073741857 Remrootp 5 Version=0')
+Index "$ID_52596C31" for table "t" Rootp 1073741855 Remrootp 3 Version=0')
+Table "t" Rootp 1073741854 Remrootp 2 Version=0')
 (rows updated=10)
 (rows updated=10)
 Table "sqlite_stat1" Rootp 1073741860 Remrootp 4 Version=0')
 Table "sqlite_stat4" Rootp 1073741861 Remrootp 5 Version=0')
 Index "$ID_52596C31" for table "t" Rootp 1073741859 Remrootp 3 Version=0')
 Table "t" Rootp 1073741858 Remrootp 2 Version=0')
-Table "sqlite_stat1" Rootp 1073741856 Remrootp 4 Version=0')
-Table "sqlite_stat4" Rootp 1073741857 Remrootp 5 Version=0')
-Index "$ID_52596C31" for table "t" Rootp 1073741855 Remrootp 3 Version=0')
-Table "t" Rootp 1073741854 Remrootp 2 Version=0')
+Table "sqlite_stat1" Rootp 1073741860 Remrootp 4 Version=0')
+Table "sqlite_stat4" Rootp 1073741861 Remrootp 5 Version=0')
+Index "$ID_52596C31" for table "t" Rootp 1073741859 Remrootp 3 Version=0')
+Table "t" Rootp 1073741858 Remrootp 2 Version=0')
 (rows deleted=10)
 (rows deleted=10)
 Table "sqlite_stat1" Rootp 1073741864 Remrootp 4 Version=0')
 Table "sqlite_stat4" Rootp 1073741865 Remrootp 5 Version=0')
 Index "$ID_52596C31" for table "t" Rootp 1073741863 Remrootp 3 Version=0')
 Table "t" Rootp 1073741862 Remrootp 2 Version=0')
-Table "sqlite_stat1" Rootp 1073741860 Remrootp 4 Version=0')
-Table "sqlite_stat4" Rootp 1073741861 Remrootp 5 Version=0')
-Index "$ID_52596C31" for table "t" Rootp 1073741859 Remrootp 3 Version=0')
-Table "t" Rootp 1073741858 Remrootp 2 Version=0')
+Table "sqlite_stat1" Rootp 1073741864 Remrootp 4 Version=0')
+Table "sqlite_stat4" Rootp 1073741865 Remrootp 5 Version=0')
+Index "$ID_52596C31" for table "t" Rootp 1073741863 Remrootp 3 Version=0')
+Table "t" Rootp 1073741862 Remrootp 2 Version=0')
 TEST 7
 remtran test against a too new version
 (rows inserted=10)
@@ -130,30 +132,30 @@ Table "sqlite_stat1" Rootp 1073741868 Remrootp 4 Version=0')
 Table "sqlite_stat4" Rootp 1073741869 Remrootp 5 Version=0')
 Index "$ID_52596C31" for table "t" Rootp 1073741867 Remrootp 3 Version=0')
 Table "t" Rootp 1073741866 Remrootp 2 Version=0')
-Table "sqlite_stat1" Rootp 1073741864 Remrootp 4 Version=0')
-Table "sqlite_stat4" Rootp 1073741865 Remrootp 5 Version=0')
-Index "$ID_52596C31" for table "t" Rootp 1073741863 Remrootp 3 Version=0')
-Table "t" Rootp 1073741862 Remrootp 2 Version=0')
+Table "sqlite_stat1" Rootp 1073741868 Remrootp 4 Version=0')
+Table "sqlite_stat4" Rootp 1073741869 Remrootp 5 Version=0')
+Index "$ID_52596C31" for table "t" Rootp 1073741867 Remrootp 3 Version=0')
+Table "t" Rootp 1073741866 Remrootp 2 Version=0')
 (rows updated=10)
 (rows updated=10)
 Table "sqlite_stat1" Rootp 1073741872 Remrootp 4 Version=0')
 Table "sqlite_stat4" Rootp 1073741873 Remrootp 5 Version=0')
 Index "$ID_52596C31" for table "t" Rootp 1073741871 Remrootp 3 Version=0')
 Table "t" Rootp 1073741870 Remrootp 2 Version=0')
-Table "sqlite_stat1" Rootp 1073741868 Remrootp 4 Version=0')
-Table "sqlite_stat4" Rootp 1073741869 Remrootp 5 Version=0')
-Index "$ID_52596C31" for table "t" Rootp 1073741867 Remrootp 3 Version=0')
-Table "t" Rootp 1073741866 Remrootp 2 Version=0')
+Table "sqlite_stat1" Rootp 1073741872 Remrootp 4 Version=0')
+Table "sqlite_stat4" Rootp 1073741873 Remrootp 5 Version=0')
+Index "$ID_52596C31" for table "t" Rootp 1073741871 Remrootp 3 Version=0')
+Table "t" Rootp 1073741870 Remrootp 2 Version=0')
 (rows deleted=10)
 (rows deleted=10)
 Table "sqlite_stat1" Rootp 1073741876 Remrootp 4 Version=0')
 Table "sqlite_stat4" Rootp 1073741877 Remrootp 5 Version=0')
 Index "$ID_52596C31" for table "t" Rootp 1073741875 Remrootp 3 Version=0')
 Table "t" Rootp 1073741874 Remrootp 2 Version=0')
-Table "sqlite_stat1" Rootp 1073741872 Remrootp 4 Version=0')
-Table "sqlite_stat4" Rootp 1073741873 Remrootp 5 Version=0')
-Index "$ID_52596C31" for table "t" Rootp 1073741871 Remrootp 3 Version=0')
-Table "t" Rootp 1073741870 Remrootp 2 Version=0')
+Table "sqlite_stat1" Rootp 1073741876 Remrootp 4 Version=0')
+Table "sqlite_stat4" Rootp 1073741877 Remrootp 5 Version=0')
+Index "$ID_52596C31" for table "t" Rootp 1073741875 Remrootp 3 Version=0')
+Table "t" Rootp 1073741874 Remrootp 2 Version=0')
 TEST 8
 remtran test for client transactions
 (id=11)

diff --git a/tests/fdb_compat.test/test_fdb.sh b/tests/fdb_compat.test/test_fdb.sh
@@ -51,8 +51,11 @@ ver=`$R_SQLT "select value from comdb2_tunables where name='fdb_default_version'
 echo "Current fdb version $ver" >> $output 2>&1
 
 # populate table on remote
+$S_SQL "insert into t select * from generate_series(11, 13)" >> $output 2>&1
+$R_SQL "insert into t select * from LOCAL_${a_dbname}.t" >> $output 2>&1
+$R_SQL "exec procedure sys.cmd.send('fdb init')"
+$S_SQL "delete from t" >> $output 2>&1
 $S_SQL "insert into t select * from generate_series(1, 3)" >> $output 2>&1
-$R_SQL "insert into t select * from generate_series(11, 13)" >> $output 2>&1
 
 #gonna test remsql, disable push
 $S_SQL "put tunable foreign_db_push_remote 0"