mirror of https://github.com/citusdata/citus.git
Fix: schema leak onto create index statement cache (#2964)
DESCRIPTION: Fix schema leak on CREATE INDEX statement When a CREATE INDEX is cached between execution we might leak the schema name onto the cached statement of an earlier execution preventing the right index to be created. Even though the cache is cleared when the search_path changes we can trigger this behaviour by having the schema already on the search path before a colliding table is created in a schema earlier on the `search_path`. When calling an unqualified create index via a function (used to trigger the caching behaviour) we see that the index is created on the wrong table after the schema leaked onto the statement. By copying the complete `PlannedStmt` and `utilityStmt` during our planning phase for distributed ddls we make sure we are not leaking the schema name onto a cached data structure. Caveat; COPY statements already have a lot of parsestree copying ongoing without directly putting it back on the `pstmt`. We should verify that copies modify the statement and potentially copy the complete `pstmt` there already.pull/2968/head
parent
1f84056b83
commit
05f0668cdc
|
@ -290,6 +290,14 @@ multi_ProcessUtility(PlannedStmt *pstmt,
|
|||
{
|
||||
return;
|
||||
}
|
||||
|
||||
/*
|
||||
* we need to set the parsetree here already as we copy and replace the original
|
||||
* parsetree during ddl propagation. In reality we need to refactor the code above
|
||||
* to not juggle copy the parsetree and leak it to a potential cache above the
|
||||
* utillity hook.
|
||||
*/
|
||||
pstmt->utilityStmt = parsetree;
|
||||
}
|
||||
|
||||
/* we're mostly in DDL (and VACUUM/TRUNCATE) territory at this point... */
|
||||
|
@ -312,16 +320,12 @@ multi_ProcessUtility(PlannedStmt *pstmt,
|
|||
/* only generate worker DDLJobs if propagation is enabled */
|
||||
if (EnableDDLPropagation)
|
||||
{
|
||||
/* copy planned statement since we might scribble on it or its utilityStmt */
|
||||
pstmt = copyObject(pstmt);
|
||||
parsetree = pstmt->utilityStmt;
|
||||
|
||||
if (IsA(parsetree, IndexStmt))
|
||||
{
|
||||
MemoryContext oldContext = MemoryContextSwitchTo(GetMemoryChunkContext(
|
||||
parsetree));
|
||||
|
||||
/* copy parse tree since we might scribble on it to fix the schema name */
|
||||
parsetree = copyObject(parsetree);
|
||||
|
||||
MemoryContextSwitchTo(oldContext);
|
||||
|
||||
ddlJobs = PlanIndexStmt((IndexStmt *) parsetree, queryString);
|
||||
}
|
||||
|
||||
|
|
|
@ -1212,6 +1212,50 @@ SELECT ddl_in_plpgsql();
|
|||
|
||||
(1 row)
|
||||
|
||||
-- test prepared ddl with multi search path to make sure the schema name doesn't leak on
|
||||
-- to the cached statement
|
||||
CREATE OR REPLACE FUNCTION ddl_in_plpgsql()
|
||||
RETURNS VOID AS
|
||||
$BODY$
|
||||
BEGIN
|
||||
CREATE INDEX prepared_index ON prepare_ddl(x);
|
||||
END;
|
||||
$BODY$ LANGUAGE plpgsql;
|
||||
CREATE SCHEMA otherschema;
|
||||
SET search_path TO otherschema, public;
|
||||
SELECT ddl_in_plpgsql();
|
||||
ddl_in_plpgsql
|
||||
----------------
|
||||
|
||||
(1 row)
|
||||
|
||||
DROP INDEX prepared_index;
|
||||
-- this creates the same table it 'otherschema'. If there is a leak the index will not be
|
||||
-- created on this table, but instead on the table in the public schema
|
||||
CREATE TABLE prepare_ddl (x int, y int);
|
||||
SELECT create_distributed_table('prepare_ddl', 'x');
|
||||
create_distributed_table
|
||||
--------------------------
|
||||
|
||||
(1 row)
|
||||
|
||||
SELECT ddl_in_plpgsql();
|
||||
ddl_in_plpgsql
|
||||
----------------
|
||||
|
||||
(1 row)
|
||||
|
||||
-- verify the index is created in the correct schema
|
||||
SELECT schemaname, indexrelname FROM pg_stat_all_indexes WHERE indexrelname = 'prepared_index';
|
||||
schemaname | indexrelname
|
||||
-------------+----------------
|
||||
otherschema | prepared_index
|
||||
(1 row)
|
||||
|
||||
-- cleanup
|
||||
DROP TABLE prepare_ddl;
|
||||
DROP SCHEMA otherschema;
|
||||
RESET search_path;
|
||||
-- test prepared COPY
|
||||
CREATE OR REPLACE FUNCTION copy_in_plpgsql()
|
||||
RETURNS VOID AS
|
||||
|
|
|
@ -555,6 +555,37 @@ $BODY$ LANGUAGE plpgsql;
|
|||
SELECT ddl_in_plpgsql();
|
||||
SELECT ddl_in_plpgsql();
|
||||
|
||||
-- test prepared ddl with multi search path to make sure the schema name doesn't leak on
|
||||
-- to the cached statement
|
||||
CREATE OR REPLACE FUNCTION ddl_in_plpgsql()
|
||||
RETURNS VOID AS
|
||||
$BODY$
|
||||
BEGIN
|
||||
CREATE INDEX prepared_index ON prepare_ddl(x);
|
||||
END;
|
||||
$BODY$ LANGUAGE plpgsql;
|
||||
|
||||
CREATE SCHEMA otherschema;
|
||||
SET search_path TO otherschema, public;
|
||||
|
||||
SELECT ddl_in_plpgsql();
|
||||
DROP INDEX prepared_index;
|
||||
|
||||
-- this creates the same table it 'otherschema'. If there is a leak the index will not be
|
||||
-- created on this table, but instead on the table in the public schema
|
||||
CREATE TABLE prepare_ddl (x int, y int);
|
||||
SELECT create_distributed_table('prepare_ddl', 'x');
|
||||
|
||||
SELECT ddl_in_plpgsql();
|
||||
-- verify the index is created in the correct schema
|
||||
SELECT schemaname, indexrelname FROM pg_stat_all_indexes WHERE indexrelname = 'prepared_index';
|
||||
|
||||
-- cleanup
|
||||
DROP TABLE prepare_ddl;
|
||||
DROP SCHEMA otherschema;
|
||||
|
||||
RESET search_path;
|
||||
|
||||
-- test prepared COPY
|
||||
CREATE OR REPLACE FUNCTION copy_in_plpgsql()
|
||||
RETURNS VOID AS
|
||||
|
|
Loading…
Reference in New Issue