Remove dodgy skip_start hack for notebook protocol (#6849)

There is a hack in execute.execute so that notebook can keep everything in an implicit transaction as far as the dbview is concerned. Instead, just put it all in an explicit dbview transaction.
geldata · Feb 14, 2024 · 34bceb5 · 34bceb5
1 parent 1e9ab73
commit 34bceb5
Show file tree

Hide file tree

Showing 4 changed files with 6 additions and 15 deletions.
diff --git a/edb/server/dbview/dbview.pxd b/edb/server/dbview/dbview.pxd
@@ -179,7 +179,7 @@ cdef class DatabaseConnectionView:
     cdef tx_error(self)
 
     cdef start(self, query_unit)
-    cdef _start_tx(self)
+    cdef start_tx(self)
     cdef _apply_in_tx(self, query_unit)
     cdef start_implicit(self, query_unit)
     cdef on_error(self)

diff --git a/edb/server/dbview/dbview.pyx b/edb/server/dbview/dbview.pyx
@@ -750,15 +750,12 @@ cdef class DatabaseConnectionView:
 
         if query_unit.tx_id is not None:
             self._txid = query_unit.tx_id
-            self._start_tx()
-
-        if self._in_tx and not self._txid:
-            raise errors.InternalServerError('unset txid in transaction')
+            self.start_tx()
 
         if self._in_tx:
             self._apply_in_tx(query_unit)
 
-    cdef _start_tx(self):
+    cdef start_tx(self):
         state_serializer = self.get_state_serializer()
         self._in_tx = True
         self._in_tx_config = self._config
@@ -793,7 +790,7 @@ cdef class DatabaseConnectionView:
             self.raise_in_tx_error()
 
         if not self._in_tx:
-            self._start_tx()
+            self.start_tx()
 
         self._apply_in_tx(query_unit)
 

diff --git a/edb/server/protocol/execute.pyx b/edb/server/protocol/execute.pyx
@@ -65,9 +65,6 @@ async def execute(
     *,
     fe_conn: frontend.AbstractFrontendConnection = None,
     use_prep_stmt: bint = False,
-    # HACK: A hook from the notebook ext, telling us to skip dbview.start
-    # so that it can handle things differently.
-    skip_start: bint = False,
 ):
     cdef:
         bytes state = None, orig_state = None
@@ -89,8 +86,7 @@ async def execute(
             # the current status in be_conn is in sync with dbview, skip the
             # state restoring
             state = None
-        if not skip_start:
-            dbv.start(query_unit)
+        dbv.start(query_unit)
         if query_unit.create_db_template:
             await tenant.on_before_create_db_from_template(
                 query_unit.create_db_template,

diff --git a/edb/server/protocol/notebook_ext.pyx b/edb/server/protocol/notebook_ext.pyx
@@ -177,6 +177,7 @@ async def execute(db, tenant, queries: list):
     pgcon = await tenant.acquire_pgcon(db.name)
     try:
         await pgcon.sql_execute(b'START TRANSACTION;')
+        dbv.start_tx()
 
         for is_error, unit_or_error in units:
             if is_error:
@@ -202,13 +203,10 @@ async def execute(db, tenant, queries: list):
 
                     fe_conn = NotebookConnection()
 
-                    dbv.start_implicit(query_unit)
-
                     compiled = dbview.CompiledQuery(
                         query_unit_group=query_unit_group)
                     await p_execute.execute(
                         pgcon, dbv, compiled, b'', fe_conn=fe_conn,
-                        skip_start=True,
                     )
 
                 except Exception as ex: