Skip to content

Commit

Permalink
Add tests for GUC of PGC_STRING type value rollback
Browse files Browse the repository at this point in the history 
GUC values of PGC_STRING type should be quoted or parsing
at GUC value rollback will fail otherwise.

As failed SET command leads to transaction abort that leads to
backend exit that triggers RemoveTempRelationsCallback to remove
temp relations, the new tests verify that after GUC PGC_STRING value
rollback no temp relation is removed.

Test for empty search_path GUC value from 620bdbd
replaced with new one that required no Fault Injector and keeps the
source code clean.
  • Loading branch information
@seryozhasmirnov @gfphoenix78
seryozhasmirnov authored and gfphoenix78 committed Oct 30, 2024
1 parent 2365a97 commit 8dffe76
Show file tree
Hide file tree
Showing 4 changed files with 486 additions and 76 deletions.
10 changes: 0 additions & 10 deletions src/backend/commands/createas.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,6 @@
#include "cdb/cdbvars.h"
#include "cdb/memquota.h"
#include "utils/metrics_utils.h"
#include "utils/faultinjector.h"

typedef struct
{
Expand Down Expand Up @@ -346,15 +345,6 @@ ExecCreateTableAs(ParseState *pstate, CreateTableAsStmt *stmt,
if (CreateTableAsRelExists(stmt))
return InvalidObjectAddress;

#ifdef FAULT_INJECTOR
if (SIMPLE_FAULT_INJECTOR("change_string_guc") == FaultInjectorTypeSkip)
{
(void) set_config_option("search_path", "public",
PGC_USERSET, PGC_S_SESSION,
GUC_ACTION_SAVE, true, 0, false);
}
#endif

/*
* Create the tuple receiver object and insert info it will need
*/
Expand Down
4 changes: 0 additions & 4 deletions src/backend/tcop/postgres.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1621,10 +1621,6 @@ restore_guc_to_QE(void )
}
PG_CATCH();
{

#ifdef FAULT_INJECTOR
SIMPLE_FAULT_INJECTOR("restore_string_guc");
#endif
/* if some guc can not restore successful
* we can not keep alive gang anymore.
*/
Expand Down
346 changes: 309 additions & 37 deletions src/test/regress/expected/guc_gp.out
View file
Original file line number Diff line number Diff line change
Expand Up @@ -408,55 +408,327 @@ SHOW search_path;
(1 row)

RESET search_path;
-- when the original string guc is empty, we change the guc to new value during executing a command.
-- this guc will be added to gp_guc_restore_list, and they will be restored
-- to original value to qe when the next command is executed.
-- however, the dispatch command is "set xxx to ;" that is wrong.
create extension if not exists gp_inject_fault;
create table public.restore_guc_test(tc1 int);
NOTICE: Table doesn't have 'DISTRIBUTED BY' clause -- Using column named 'tc1' as the Cloudberry Database data distribution key for this table.
-- Test single query default_tablespace GUC rollback
-- Function just to save default_tablespace GUC to gp_guc_restore_list
CREATE OR REPLACE FUNCTION set_conf_param() RETURNS VOID
AS $$
BEGIN
EXECUTE 'SELECT 1;';
END;
$$ LANGUAGE plpgsql
SET default_tablespace TO '';
-- Create temp table to create temp schema
CREATE TEMP TABLE just_a_temp_table (a int);
NOTICE: Table doesn't have 'DISTRIBUTED BY' clause -- Using column named 'a' as the Greenplum Database data distribution key for this table.
HINT: The 'DISTRIBUTED BY' clause determines the distribution of data. Make sure column(s) chosen are the optimal data distribution key to minimize skew.
-- inject fault to change the value of search_path during creating materialized view
SELECT gp_inject_fault('change_string_guc', 'skip', 1);
gp_inject_fault
-----------------
Success:
-- Temp schema should be created for each segment
SELECT count(nspname) FROM gp_dist_random('pg_namespace') WHERE nspname LIKE 'pg_temp%';
count
-------
3
(1 row)

-- inject fault when dispatch guc restore command occur errors, we throw an error.
SELECT gp_inject_fault('restore_string_guc', 'error', 1);
gp_inject_fault
-----------------
Success:
-- Save default_tablespace GUC to gp_guc_restore_list
SELECT set_conf_param();
set_conf_param
----------------

(1 row)

-- set search_path to '';
SELECT pg_catalog.set_config('search_path', '', false);
set_config
------------
-- Trigger default_tablespace GUC restore from gp_guc_restore_list
SELECT 1;
?column?
----------
1
(1 row)

-- When default_tablespace GUC is restored from gp_guc_restore_list
-- successfully no RemoveTempRelationsCallback is called.
-- So check that segments still have temp schemas
SELECT count(nspname) FROM gp_dist_random('pg_namespace') WHERE nspname LIKE 'pg_temp%';
count
-------
3
(1 row)

-- Cleanup
DROP TABLE just_a_temp_table;
-- Test single query gp_default_storage_options GUC rollback
-- Function just to save gp_default_storage_options to gp_guc_restore_list
CREATE OR REPLACE FUNCTION set_conf_param() RETURNS VOID
AS $$
BEGIN
EXECUTE 'SELECT 1;';
END;
$$ LANGUAGE plpgsql
SET gp_default_storage_options TO 'blocksize=32768,compresstype=none,checksum=false';
-- Create temp table to create temp schema
CREATE TEMP TABLE just_a_temp_table (a int);
NOTICE: Table doesn't have 'DISTRIBUTED BY' clause -- Using column named 'a' as the Greenplum Database data distribution key for this table.
HINT: The 'DISTRIBUTED BY' clause determines the distribution of data. Make sure column(s) chosen are the optimal data distribution key to minimize skew.
-- Temp schema should be created for each segment
SELECT count(nspname) FROM gp_dist_random('pg_namespace') WHERE nspname LIKE 'pg_temp%';
count
-------
3
(1 row)

-- Save gp_default_storage_options GUC to gp_guc_restore_list
SELECT set_conf_param();
set_conf_param
----------------

(1 row)

-- trigger inject fault of change_string_guc, and add this guc to gp_guc_restore_list
create MATERIALIZED VIEW public.view_restore_guc_test as select * from public.restore_guc_test;
NOTICE: Table doesn't have 'DISTRIBUTED BY' clause. Creating a NULL policy entry.
--we should restore gucs in gp_guc_restore_list to qe, no error occurs.
drop MATERIALIZED VIEW public.view_restore_guc_test;
drop table public.restore_guc_test;
--cleanup
reset search_path;
SELECT gp_inject_fault('change_string_guc', 'reset', 1);
gp_inject_fault
-----------------
Success:
-- Trigger gp_default_storage_options GUC restore from gp_guc_restore_list
SELECT 1;
?column?
----------
1
(1 row)

SELECT gp_inject_fault('restore_string_guc', 'reset', 1);
gp_inject_fault
-----------------
Success:
-- When gp_default_storage_options GUC is restored from gp_guc_restore_list
-- successfully no RemoveTempRelationsCallback is called.
-- So check that segments still have temp schemas
SELECT count(nspname) FROM gp_dist_random('pg_namespace') WHERE nspname LIKE 'pg_temp%';
count
-------
3
(1 row)

-- Cleanup
DROP TABLE just_a_temp_table;
-- Test single query lc_numeric GUC rollback
-- Set lc_numeric to OS-friendly value
SET lc_numeric TO 'C';
-- Function just to save lc_numeric GUC to gp_guc_restore_list
CREATE OR REPLACE FUNCTION set_conf_param() RETURNS VOID
AS $$
BEGIN
EXECUTE 'SELECT 1;';
END;
$$ LANGUAGE plpgsql
SET lc_numeric TO 'C';
-- Create temp table to create temp schema
CREATE TEMP TABLE just_a_temp_table (a int);
NOTICE: Table doesn't have 'DISTRIBUTED BY' clause -- Using column named 'a' as the Greenplum Database data distribution key for this table.
HINT: The 'DISTRIBUTED BY' clause determines the distribution of data. Make sure column(s) chosen are the optimal data distribution key to minimize skew.
-- Temp schema should be created for each segment
SELECT count(nspname) FROM gp_dist_random('pg_namespace') WHERE nspname LIKE 'pg_temp%';
count
-------
3
(1 row)

-- Save lc_numeric GUC to gp_guc_restore_list
SELECT set_conf_param();
set_conf_param
----------------

(1 row)

-- Trigger lc_numeric GUC restore from gp_guc_restore_list
SELECT 1;
?column?
----------
1
(1 row)

-- When lc_numeric GUC is restored from gp_guc_restore_list
-- successfully no RemoveTempRelationsCallback is called.
-- So check that segments still have temp schemas
SELECT count(nspname) FROM gp_dist_random('pg_namespace') WHERE nspname LIKE 'pg_temp%';
count
-------
3
(1 row)

-- Cleanup
DROP TABLE just_a_temp_table;
-- Test single query pljava_classpath GUC rollback
-- Function just to save pljava_classpath GUC to gp_guc_restore_list
CREATE OR REPLACE FUNCTION set_conf_param() RETURNS VOID
AS $$
BEGIN
EXECUTE 'SELECT 1;';
END;
$$ LANGUAGE plpgsql
SET pljava_classpath TO '';
-- Create temp table to create temp schema
CREATE TEMP TABLE just_a_temp_table (a int);
NOTICE: Table doesn't have 'DISTRIBUTED BY' clause -- Using column named 'a' as the Greenplum Database data distribution key for this table.
HINT: The 'DISTRIBUTED BY' clause determines the distribution of data. Make sure column(s) chosen are the optimal data distribution key to minimize skew.
-- Temp schema should be created for each segment
SELECT count(nspname) FROM gp_dist_random('pg_namespace') WHERE nspname LIKE 'pg_temp%';
count
-------
3
(1 row)

-- Save pljava_classpath GUC to gp_guc_restore_list
SELECT set_conf_param();
set_conf_param
----------------

(1 row)

-- Trigger pljava_classpath GUC restore from gp_guc_restore_list
SELECT 1;
?column?
----------
1
(1 row)

-- When pljava_classpath GUC is restored from gp_guc_restore_list
-- successfully no RemoveTempRelationsCallback is called.
-- So check that segments still have temp schemas
SELECT count(nspname) FROM gp_dist_random('pg_namespace') WHERE nspname LIKE 'pg_temp%';
count
-------
3
(1 row)

-- Cleanup
DROP TABLE just_a_temp_table;
-- Test single query pljava_vmoptions GUC rollback
-- Function just to save pljava_vmoptions GUC to gp_guc_restore_list
CREATE OR REPLACE FUNCTION set_conf_param() RETURNS VOID
AS $$
BEGIN
EXECUTE 'SELECT 1;';
END;
$$ LANGUAGE plpgsql
SET pljava_vmoptions TO '';
-- Create temp table to create temp schema
CREATE TEMP TABLE just_a_temp_table (a int);
NOTICE: Table doesn't have 'DISTRIBUTED BY' clause -- Using column named 'a' as the Greenplum Database data distribution key for this table.
HINT: The 'DISTRIBUTED BY' clause determines the distribution of data. Make sure column(s) chosen are the optimal data distribution key to minimize skew.
-- Temp schema should be created for each segment
SELECT count(nspname) FROM gp_dist_random('pg_namespace') WHERE nspname LIKE 'pg_temp%';
count
-------
3
(1 row)

-- Save pljava_vmoptions GUC to gp_guc_restore_list
SELECT set_conf_param();
set_conf_param
----------------

(1 row)

-- Trigger pljava_vmoptions GUC restore from gp_guc_restore_list
SELECT 1;
?column?
----------
1
(1 row)

-- When pljava_vmoptions GUC is restored from gp_guc_restore_list
-- successfully no RemoveTempRelationsCallback is called.
-- So check that segments still have temp schemas
SELECT count(nspname) FROM gp_dist_random('pg_namespace') WHERE nspname LIKE 'pg_temp%';
count
-------
3
(1 row)

-- Cleanup
DROP TABLE just_a_temp_table;
-- Test single query GUC TimeZone rollback
-- Set TimeZone to value that has to be quoted due to slash
SET TimeZone TO 'Africa/Mbabane';
-- Function just to save TimeZone to gp_guc_restore_list
CREATE OR REPLACE FUNCTION set_conf_param() RETURNS VOID
AS $$
BEGIN
EXECUTE 'SELECT 1;';
END;
$$ LANGUAGE plpgsql
SET TimeZone TO 'UTC';
-- Create temp table to create temp schema
CREATE TEMP TABLE just_a_temp_table (a int);
NOTICE: Table doesn't have 'DISTRIBUTED BY' clause -- Using column named 'a' as the Greenplum Database data distribution key for this table.
HINT: The 'DISTRIBUTED BY' clause determines the distribution of data. Make sure column(s) chosen are the optimal data distribution key to minimize skew.
-- Temp schema should be created for each segment
SELECT count(nspname) FROM gp_dist_random('pg_namespace') WHERE nspname LIKE 'pg_temp%';
count
-------
3
(1 row)

-- Save TimeZone GUC to gp_guc_restore_list
SELECT set_conf_param();
set_conf_param
----------------

(1 row)

-- Trigger TimeZone GUC restore from gp_guc_restore_list
SELECT 1;
?column?
----------
1
(1 row)

-- When TimeZone GUC is restored from gp_guc_restore_list
-- successfully no RemoveTempRelationsCallback is called.
-- So check that segments still have temp schemas
SELECT count(nspname) FROM gp_dist_random('pg_namespace') WHERE nspname LIKE 'pg_temp%';
count
-------
3
(1 row)

-- Cleanup
DROP TABLE just_a_temp_table;
-- Test single query search_path GUC rollback
-- Add empty value to search_path that caused issues before
-- to verify that rollback it it will be successful.
SET search_path TO public, '';
-- Function just to save default_tablespace GUC to gp_guc_restore_list
CREATE OR REPLACE FUNCTION set_conf_param() RETURNS VOID
AS $$
BEGIN
EXECUTE 'SELECT 1;';
END;
$$ LANGUAGE plpgsql
SET search_path TO "public";
-- Create temp table to create temp schema
CREATE TEMP TABLE just_a_temp_table (a int);
-- Temp schema should be created for each segment
SELECT count(nspname) FROM gp_dist_random('pg_namespace') WHERE nspname LIKE 'pg_temp%';
count
-------
3
(1 row)

-- Save default_tablespace GUC to gp_guc_restore_list
SELECT set_conf_param();
set_conf_param
----------------

(1 row)

-- Trigger default_tablespace GUC restore from gp_guc_restore_list
SELECT 1;
?column?
----------
1
(1 row)

-- When search_path GUC is restored from gp_guc_restore_list
-- successfully no RemoveTempRelationsCallback is called.
-- So check that segments still have temp schemas
SELECT count(nspname) FROM gp_dist_random('pg_namespace') WHERE nspname LIKE 'pg_temp%';
count
-------
3
(1 row)

-- Cleanup
DROP TABLE just_a_temp_table;
RESET search_path;
-- enabling gp_force_random_redistribution makes sure random redistribution happens
-- only relevant to postgres optimizer
set optimizer = false;
Expand Down
Loading

0 comments on commit 8dffe76

Please sign in to comment.