Fully test the GUC, fix http empty strings

theory · theory · commit 918c1c1dc139 · 2025-12-04T12:57:23.000-05:00
diff --git a/Makefile b/Makefile
@@ -70,7 +70,7 @@ ifneq ($(OS),Darwin)
 endif
 
 # Clean up the clickhouse-cpp build directory and generated files.
-EXTRA_CLEAN = $(CH_CPP_BUILD_DIR) sql/$(EXTENSION)--$(EXTVERSION).sql src/fdw.c compile_commands.json
+EXTRA_CLEAN = sql/$(EXTENSION)--$(EXTVERSION).sql src/fdw.c compile_commands.json
 
 # Import PGXS.
 PGXS := $(shell $(PG_CONFIG) --pgxs)
diff --git a/src/http.c b/src/http.c
@@ -209,12 +209,8 @@ ch_http_simple_query(ch_http_connection_t * conn, const ch_query * query)
 		curl_easy_setopt(conn->curl, CURLOPT_NOPROGRESS, 1L);
 	if (conn->dbname)
 	{
-		char	   *buf = palloc(strlen(conn->dbname) + strlen(DATABASE_HEADER) + 3);
-
-		sprintf(buf, "%s: %s", DATABASE_HEADER, conn->dbname);
-		headers = curl_slist_append(headers, buf);
+		headers = curl_slist_append(headers, psprintf("%s: %s", DATABASE_HEADER, conn->dbname));
 		curl_easy_setopt(conn->curl, CURLOPT_HTTPHEADER, headers);
-		pfree(buf);
 	}
 
 	curl_error_happened = false;
diff --git a/src/pglink.c b/src/pglink.c
@@ -303,7 +303,7 @@ http_fetch_row(ch_cursor * cursor, List * attrs, TupleDesc tupdesc, Datum * v, b
 		else if (state->val[0] != '\0')
 			values[i] = pstrdup(state->val);
 		else
-			values[i] = NULL;
+			values[i] = "";
 	}
 
 	if (attcount > 0 && rc != CH_EOL && rc != CH_EOF)
diff --git a/test/expected/gucs.out b/test/expected/gucs.out
@@ -1,4 +1,9 @@
 \unset ECHO
+ ch_noop_bigint 
+----------------
+               
+(1 row)
+
 NOTICE:  OK ``
 NOTICE:  OK `join_use_nulls=1`
 NOTICE:  OK `join_use_nulls=1, xyz=true`
@@ -9,4 +14,98 @@ NOTICE:  ERR 42601 - pg_clickhouse: missing "=" after "join_use_nulls" in option
 NOTICE:  ERR 42601 - pg_clickhouse: missing "=" after "join_use_nulls" in options string
 NOTICE:  ERR 42601 - pg_clickhouse: unterminated quoted string in options string
 NOTICE:  ERR 42601 - pg_clickhouse: missing comma after "join_use_nulls" value in options string
-NOTICE:  drop cascades to foreign table remote_settings
+      name      | value 
+----------------+-------
+ join_use_nulls | 1
+(1 row)
+
+      name      | value 
+----------------+-------
+ join_use_nulls | 1
+(1 row)
+
+        pg_clickhouse.session_settings        
+----------------------------------------------
+                                             +
+     connect_timeout = 2,                    +
+     count_distinct_implementation = uniq,   +
+     join_algorithm = 'prefer_partial_merge',+
+     join_use_nulls = 0,                     +
+     join_use_nulls = 1,                     +
+     log_queries_min_type = QUERY_FINISH,    +
+     max_block_size = 32768,                 +
+     max_execution_time = 45,                +
+     max_result_rows = 1024,                 +
+     metrics_perf_events_list = 'this,that', +
+     network_compression_method = ZSTD,      +
+     poll_interval = 5,                      +
+     totals_mode = after_having_auto         +
+ 
+(1 row)
+
+             name              |        value         
+-------------------------------+----------------------
+ connect_timeout               | 2
+ count_distinct_implementation | uniq
+ join_algorithm                | prefer_partial_merge
+ join_use_nulls                | 1
+ log_queries_min_type          | QUERY_FINISH
+ max_block_size                | 32768
+ max_execution_time            | 45
+ max_result_rows               | 1024
+ metrics_perf_events_list      | this,that
+ network_compression_method    | ZSTD
+ poll_interval                 | 5
+ totals_mode                   | after_having_auto
+(12 rows)
+
+             name              |        value         
+-------------------------------+----------------------
+ connect_timeout               | 2
+ count_distinct_implementation | uniq
+ join_algorithm                | prefer_partial_merge
+ join_use_nulls                | 1
+ log_queries_min_type          | QUERY_FINISH
+ max_block_size                | 32768
+ max_execution_time            | 45
+ max_result_rows               | 1024
+ metrics_perf_events_list      | this,that
+ network_compression_method    | ZSTD
+ poll_interval                 | 5
+ totals_mode                   | after_having_auto
+(12 rows)
+
+             name              | ?column? 
+-------------------------------+----------
+ connect_timeout               | t
+ count_distinct_implementation | t
+ join_algorithm                | t
+ join_use_nulls                | t
+ log_queries_min_type          | t
+ max_block_size                | t
+ max_execution_time            | t
+ max_result_rows               | t
+ metrics_perf_events_list      | t
+ network_compression_method    | t
+ poll_interval                 | t
+ totals_mode                   | t
+(12 rows)
+
+             name              | ?column? 
+-------------------------------+----------
+ connect_timeout               | t
+ count_distinct_implementation | t
+ join_algorithm                | t
+ join_use_nulls                | t
+ log_queries_min_type          | t
+ max_block_size                | t
+ max_execution_time            | t
+ max_result_rows               | t
+ metrics_perf_events_list      | t
+ network_compression_method    | t
+ poll_interval                 | t
+ totals_mode                   | t
+(12 rows)
+
+NOTICE:  drop cascades to foreign table bin_remote_settings
+NOTICE:  drop cascades to foreign table http_remote_settings
diff --git a/test/expected/http.out b/test/expected/http.out
@@ -132,6 +132,13 @@ SELECT c3, (c3 = 'lf\ntab\t\b\f\r') AS true FROM ft3 WHERE c1 = 2;
  lf\ntab\t\b\f\r | t
 (1 row)
 
+INSERT INTO ft3 VALUES (3, '');
+SELECT c3, (c3 = '') AS true FROM ft3 WHERE c1 = 3;
+ c3 | true 
+----+------
+    | t
+(1 row)
+
 INSERT INTO ft4
 	SELECT id,
 	       id + 1,
diff --git a/test/expected/http_1.out b/test/expected/http_1.out
@@ -132,6 +132,13 @@ SELECT c3, (c3 = 'lf\ntab\t\b\f\r') AS true FROM ft3 WHERE c1 = 2;
  lf\ntab\t\b\f\r | t
 (1 row)
 
+INSERT INTO ft3 VALUES (3, '');
+SELECT c3, (c3 = '') AS true FROM ft3 WHERE c1 = 3;
+ c3 | true 
+----+------
+    | t
+(1 row)
+
 INSERT INTO ft4
 	SELECT id,
 	       id + 1,
diff --git a/test/expected/http_2.out b/test/expected/http_2.out
@@ -132,6 +132,13 @@ SELECT c3, (c3 = 'lf\ntab\t\b\f\r') AS true FROM ft3 WHERE c1 = 2;
  lf\ntab\t\b\f\r | t
 (1 row)
 
+INSERT INTO ft3 VALUES (3, '');
+SELECT c3, (c3 = '') AS true FROM ft3 WHERE c1 = 3;
+ c3 | true 
+----+------
+    | t
+(1 row)
+
 INSERT INTO ft4
 	SELECT id,
 	       id + 1,
diff --git a/test/expected/http_3.out b/test/expected/http_3.out
@@ -132,6 +132,13 @@ SELECT c3, (c3 = 'lf\ntab\t\b\f\r') AS true FROM ft3 WHERE c1 = 2;
  lf\ntab\t\b\f\r | t
 (1 row)
 
+INSERT INTO ft3 VALUES (3, '');
+SELECT c3, (c3 = '') AS true FROM ft3 WHERE c1 = 3;
+ c3 | true 
+----+------
+    | t
+(1 row)
+
 INSERT INTO ft4
 	SELECT id,
 	       id + 1,
diff --git a/test/expected/http_4.out b/test/expected/http_4.out
@@ -132,6 +132,13 @@ SELECT c3, (c3 = 'lf\ntab\t\b\f\r') AS true FROM ft3 WHERE c1 = 2;
  lf\ntab\t\b\f\r | t
 (1 row)
 
+INSERT INTO ft3 VALUES (3, '');
+SELECT c3, (c3 = '') AS true FROM ft3 WHERE c1 = 3;
+ c3 | true 
+----+------
+    | t
+(1 row)
+
 INSERT INTO ft4
 	SELECT id,
 	       id + 1,
diff --git a/test/expected/http_5.out b/test/expected/http_5.out
@@ -132,6 +132,13 @@ SELECT c3, (c3 = 'lf\ntab\t\b\f\r') AS true FROM ft3 WHERE c1 = 2;
  lf\ntab\t\b\f\r | t
 (1 row)
 
+INSERT INTO ft3 VALUES (3, '');
+SELECT c3, (c3 = '') AS true FROM ft3 WHERE c1 = 3;
+ c3 | true 
+----+------
+    | t
+(1 row)
+
 INSERT INTO ft4
 	SELECT id,
 	       id + 1,
diff --git a/test/sql/gucs.sql b/test/sql/gucs.sql
@@ -1,9 +1,8 @@
 \unset ECHO
 SET client_min_messages = notice;
 
-CREATE SERVER guc_loopback FOREIGN DATA WRAPPER clickhouse_fdw
-    OPTIONS(dbname 'system', driver 'binary');
-CREATE USER MAPPING FOR CURRENT_USER SERVER guc_loopback;
+-- Load pg_clickhouse by calling one of its functions.
+SELECT ch_noop_bigint('');
 
 -- Test parsing.
 DO $do$
@@ -34,13 +33,106 @@ BEGIN
 END;
 $do$ LANGUAGE plpgsql;
 
-CREATE FOREIGN TABLE remote_settings (
+-- Create servers for each engine.
+CREATE SERVER guc_bin_svr FOREIGN DATA WRAPPER clickhouse_fdw
+    OPTIONS(dbname 'system', driver 'binary');
+CREATE USER MAPPING FOR CURRENT_USER SERVER guc_bin_svr;
+
+CREATE SERVER guc_http_svr FOREIGN DATA WRAPPER clickhouse_fdw
+    OPTIONS(dbname 'system', driver 'http');
+CREATE USER MAPPING FOR CURRENT_USER SERVER guc_http_svr;
+
+-- Create foreign tables for each engine.
+CREATE FOREIGN TABLE bin_remote_settings (
+    name text,
+    value text
+)
+  SERVER guc_bin_svr
+  OPTIONS (table_name 'settings');
+
+CREATE FOREIGN TABLE http_remote_settings (
     name text,
     value text
 )
-  SERVER guc_loopback
+  SERVER guc_http_svr
   OPTIONS (table_name 'settings');
 
+-- Reset to defaults.
+RESET pg_clickhouse.session_settings;
+SELECT name, value FROM bin_remote_settings WHERE name = 'join_use_nulls';
+SELECT name, value FROM http_remote_settings WHERE name = 'join_use_nulls';
+
+-- List of seeings changed below.
+select '{
+    connect_timeout,
+    count_distinct_implementation,
+    join_algorithm,
+    join_use_nulls,
+    log_queries_min_type,
+    max_block_size,
+    max_execution_time,
+    max_result_rows,
+    metrics_perf_events_list,
+    network_compression_method,
+    poll_interval,
+    totals_mode
+}' set_list \gset
+
+-- Unset and get defaults.
+SET pg_clickhouse.session_settings TO '';
+
+CREATE TEMPORARY TABLE default_settings AS
+SELECT name, value
+  FROM bin_remote_settings
+ WHERE name = ANY(:'set_list');
+
+-- Customize all of the above settings.
+SET pg_clickhouse.session_settings TO $$
+    connect_timeout = 2,
+    count_distinct_implementation = uniq,
+    join_algorithm = 'prefer_partial_merge',
+    join_use_nulls = 0,
+    join_use_nulls = 1,
+    log_queries_min_type = QUERY_FINISH,
+    max_block_size = 32768,
+    max_execution_time = 45,
+    max_result_rows = 1024,
+    metrics_perf_events_list = 'this,that',
+    network_compression_method = ZSTD,
+    poll_interval = 5,
+    totals_mode = after_having_auto
+$$;
+
+SHOW pg_clickhouse.session_settings;
+
+-- Check the remote settings for both engines.
+SELECT name, value
+  FROM bin_remote_settings
+ WHERE name = ANY(:'set_list')
+ ORDER BY name;
+
+SELECT name, value
+  FROM http_remote_settings
+WHERE name = ANY(:'set_list')
+ORDER BY name;
+
+-- Unset back to defaults.
+SET pg_clickhouse.session_settings TO '';
+
+SELECT remote.name, remote.value IS NOT DISTINCT FROM def.value
+ FROM bin_remote_settings remote
+ JOIN default_settings def ON remote.name = def.name
+WHERE remote.name = ANY(:'set_list')
+ORDER BY remote.name;
+
+SELECT remote.name, remote.value IS NOT DISTINCT FROM def.value
+ FROM http_remote_settings remote
+ JOIN default_settings def ON remote.name = def.name
+WHERE remote.name = ANY(:'set_list')
+ORDER BY remote.name;
+
 -- Clean up.
-DROP USER MAPPING FOR CURRENT_USER SERVER guc_loopback;
-DROP SERVER guc_loopback CASCADE;
+DROP USER MAPPING FOR CURRENT_USER SERVER guc_bin_svr;
+DROP SERVER guc_bin_svr CASCADE;
+DROP USER MAPPING FOR CURRENT_USER SERVER guc_http_svr;
+DROP SERVER guc_http_svr CASCADE;
diff --git a/test/sql/http.sql b/test/sql/http.sql
@@ -99,6 +99,8 @@ INSERT INTO ft3 VALUES (1, E'lf\ntab\t\b\f\r');
 SELECT c3, (c3 = E'lf\ntab\t\b\f\r') AS true FROM ft3 WHERE c1 = 1;
 INSERT INTO ft3 VALUES (2, 'lf\ntab\t\b\f\r');
 SELECT c3, (c3 = 'lf\ntab\t\b\f\r') AS true FROM ft3 WHERE c1 = 2;
+INSERT INTO ft3 VALUES (3, '');
+SELECT c3, (c3 = '') AS true FROM ft3 WHERE c1 = 3;
 
 INSERT INTO ft4
 	SELECT id,

Original file line number	Diff line number	Diff line change
`@@ -209,12 +209,8 @@ ch_http_simple_query(ch_http_connection_t * conn, const ch_query * query)`
`209`	`209`	`curl_easy_setopt(conn->curl, CURLOPT_NOPROGRESS, 1L);`
`210`	`210`	`if (conn->dbname)`
`211`	`211`	`{`
`212`		`- char *buf = palloc(strlen(conn->dbname) + strlen(DATABASE_HEADER) + 3);`
`213`		`-`
`214`		`- sprintf(buf, "%s: %s", DATABASE_HEADER, conn->dbname);`
`215`		`- headers = curl_slist_append(headers, buf);`
	`212`	`+ headers = curl_slist_append(headers, psprintf("%s: %s", DATABASE_HEADER, conn->dbname));`
`216`	`213`	`curl_easy_setopt(conn->curl, CURLOPT_HTTPHEADER, headers);`
`217`		`- pfree(buf);`
`218`	`214`	`}`
`219`	`215`
`220`	`216`	`curl_error_happened = false;`
Original file line number	Diff line number	Diff line change
`@@ -303,7 +303,7 @@ http_fetch_row(ch_cursor * cursor, List * attrs, TupleDesc tupdesc, Datum * v, b`
`303`	`303`	`else if (state->val[0] != '\0')`
`304`	`304`	`values[i] = pstrdup(state->val);`
`305`	`305`	`else`
`306`		`- values[i] = NULL;`
	`306`	`+ values[i] = "";`
`307`	`307`	`}`
`308`	`308`
`309`	`309`	`if (attcount > 0 && rc != CH_EOL && rc != CH_EOF)`