From 6bbbbc37705ae9c22dba3de7788cce0d907b3be4 Mon Sep 17 00:00:00 2001
From: Jelte Fennema-Nio <github-tech@jeltef.nl>
Date: Fri, 10 Jan 2025 14:42:29 +0100
Subject: [PATCH] Move to other spot for less hacky approach

---
 src/pgduckdb_hooks.cpp | 43 +++++++++++++++++++++---------------------
 1 file changed, 21 insertions(+), 22 deletions(-)

diff --git a/src/pgduckdb_hooks.cpp b/src/pgduckdb_hooks.cpp
index 0e23edaa..3d165467 100644
--- a/src/pgduckdb_hooks.cpp
+++ b/src/pgduckdb_hooks.cpp
@@ -142,6 +142,26 @@ NeedsDuckdbExecution(Query *query) {
 	return ContainsDuckdbItems((Node *)query, NULL);
 }
 
+/*
+ * We only check ContainsFromClause if duckdb.force_execution is set to true.
+ *
+ * If there's no FROM clause, we're only selecting constants. From a
+ * performance perspective there's not really a point in using DuckDB. If we
+ * remove this check many common queries that are used to inspect postgres will
+ * throw a warning or return incorrect results. For example:
+ *
+ *    SELECT current_setting('work_mem');
+ *
+ * So even if a user sets duckdb.force_execution = true, we still won't
+ * forward such queries to DuckDB. With one exception: If the query
+ * requires duckdb e.g. due to a duckdb-only function being used, we'll
+ * still executing this in DuckDB.
+ */
+static bool
+ContainsFromClause(Query *query, bool throw_error = false) {
+	return query->rtable;
+}
+
 static bool
 IsAllowedStatement(Query *query, bool throw_error = false) {
 	int elevel = throw_error ? ERROR : DEBUG4;
@@ -166,27 +186,6 @@ IsAllowedStatement(Query *query, bool throw_error = false) {
 		}
 	}
 
-	/*
-	 * XXX: This is a bit of a weird one, because it is only allowed when
-	 * IsAllowedStatement is called with throw_error.
-	 *
-	 * If there's no rtable, we're only selecting constants. From a performance
-	 * perspective there's not really a point in using DuckDB. If we remove
-	 * this heck many common queries that are used to inspect postgres will
-	 * throw a warning or return incorrect results. For example:
-	 *
-	 *    SELECT current_setting('work_mem');
-	 *
-	 * So even if a user sets duckdb.force_execution = true, we still won't
-	 * forward such queries to DuckDB. With one exception: If the query
-	 * requires duckdb e.g. due to a duckdb-only function being used, we'll
-	 * still executing this in DuckDB.
-	 */
-	if (!query->rtable && !throw_error) {
-		elog(elevel, "DuckDB usage requires at least one table");
-		return false;
-	}
-
 	/*
 	 * If any table is from pg_catalog, we don't want to use DuckDB. This is
 	 * because DuckDB has its own pg_catalog tables that contain different data
@@ -216,7 +215,7 @@ DuckdbPlannerHook_Cpp(Query *parse, const char *query_string, int cursor_options
 			IsAllowedStatement(parse, true);
 
 			return DuckdbPlanNode(parse, query_string, cursor_options, bound_params, true);
-		} else if (duckdb_force_execution && IsAllowedStatement(parse)) {
+		} else if (duckdb_force_execution && IsAllowedStatement(parse) && ContainsFromClause(parse)) {
 			PlannedStmt *duckdbPlan = DuckdbPlanNode(parse, query_string, cursor_options, bound_params, false);
 			if (duckdbPlan) {
 				return duckdbPlan;