Fix a few motherduck and testing issues (#770)

When running the tests locally I found a few issues, both in the actual
code and in the testing code. So this does the following changes:

1. Use `FindState` instead of `GetState` in all functions that are
   called by connection backends. (also added an Assert to not use the
   wrong one in the future).
2. Fail tests in case of an abort/segfault
3. Don't try running background worker queries in DuckDB even when
   `duckdb.force_execution` is set to `true`.
4. Include logfile path in test output on failure, for easier manual
   debugging.

Partial fix for #746

---------

Co-authored-by: Yves <[email protected]>
Co-authored-by: Y. <[email protected]>

Author: JelteF

src/pgduckdb_background_worker.cpp

@@ -120,6 +120,7 @@ FindState() {
 
 BgwStatePerDB *
 GetState() {
+	Assert(is_background_worker);
 	auto state = FindState();
 	if (!state) {
 		SpinLockRelease(&BgwShmemStruct->lock);
@@ -421,7 +422,10 @@ UnclaimBgwSessionHint(int /*code*/, Datum /*arg*/) {
 	}
 
 	SpinLockAcquire(&BgwShmemStruct->lock);
-	GetState()->bgw_session_hint_is_reused = false;
+	auto *state = FindState();
+	if (state) {
+		state->bgw_session_hint_is_reused = false;
+	}
 	SpinLockRelease(&BgwShmemStruct->lock);
 	reused_bgw_session_hint = false;
 }
@@ -510,8 +514,8 @@ PossiblyReuseBgwSessionHint(void) {
 
 	const char *result = "";
 	SpinLockAcquire(&BgwShmemStruct->lock);
-	auto state = GetState();
-	if (!state->bgw_session_hint_is_reused) {
+	auto state = FindState();
+	if (state && !state->bgw_session_hint_is_reused) {
 		result = bgw_session_hint;
 		state->bgw_session_hint_is_reused = true;
 		reused_bgw_session_hint = true;

src/pgduckdb_hooks.cpp

@@ -163,6 +163,14 @@ ShouldTryToUseDuckdbExecution(Query *query) {
 		return false;
 	}
 
+	if (pgduckdb::is_background_worker) {
+		/* If we're the background worker, we don't want to force duckdb
+		 * execution. Some of the queries that we're doing depend on Postgres
+		 * execution, particularly the ones that use the regclsas type to
+		 * understand tablenames. */
+		return false;
+	}
+
 	return duckdb_force_execution && pgduckdb::IsAllowedStatement(query) && ContainsFromClause(query);
 }
 

test/pycheck/conftest.py

@@ -63,8 +63,20 @@ def pg(initialized_shared_pg, default_db_name):
             try:
                 shared_pg.cleanup_test_leftovers()
             finally:
-                print("\n\nPG_LOG\n")
-                print(f.read())
+                print(f"\n\nPG_LOG {shared_pg.log_path}\n")
+                logs = f.read()
+                print(logs)
+                # Usually when Postgres crashes we quickly notice due to a
+                # query failing. Sometimes though it restarts fast enough that
+                # there's no indictaion of the crash except for the logs. So
+                # here we check that postgres did not crash during the last
+                # test.
+                assert (
+                    "was terminated by signal 6" not in logs
+                ), "Postgres crashed! Check the logs above."
+                assert (
+                    "was terminated by signal 11" not in logs
+                ), "Postgres crashed! Check the logs above."
 
 
 @pytest.fixture

test/pycheck/utils.py

@@ -753,7 +753,7 @@ def start(self):
         try:
             self.pgctl(f'-o "-p {self.port}" -l {self.log_path} start')
         except Exception:
-            print("\n\nPG_LOG\n")
+            print(f"\n\nPG_LOG {self.log_path}\n")
             with self.log_path.open() as f:
                 print(f.read())
             raise