external
/
citus
mirror of https://github.com/citusdata/citus.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Not drop default col exprs from shard when adding local table to metadata (#6323) 

As we did for GENERATED STORED columns in #4613, we should not drop
column
default expressions that are not based on sequences from shard relation
since
such expressions need to exist e.g. for foreign key actions.

For the column default expressions that are based on sequences we cannot
do much, so we need to disallow having ON DELETE SET DEFAULT actions on
such columns in a separate PR, see #6339.

Fixes #6318.

DESCRIPTION: Fixes a bug that might cause inserting incorrect DEFAULT
values when applying foreign key actions

(cherry picked from commit de24a3eda5)
valgrind-problem
Onur Tirtir 2022-09-23 13:05:08 +03:00
parent ecaa0cda6d
commit 53ec5abb75
5 changed files with 153 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

48
src/backend/distributed/commands/citus_add_local_table_to_metadata.c
View File

@ -87,8 +87,8 @@ static List * ReversedOidList(List *oidList);
static void AppendExplicitIndexIdsToList(Form_pg_index indexForm,
List **explicitIndexIdList,
int flags);
static void DropDefaultExpressionsAndMoveOwnedSequenceOwnerships(Oid sourceRelationId,
Oid targetRelationId);
static void DropNextValExprsAndMoveOwnedSeqOwnerships(Oid sourceRelationId,
Oid targetRelationId);
static void DropDefaultColumnDefinition(Oid relationId, char *columnName);
static void TransferSequenceOwnership(Oid ownedSequenceId, Oid targetRelationId,
char *columnName);
@ -366,11 +366,11 @@ CreateCitusLocalTable(Oid relationId, bool cascadeViaForeignKeys, bool autoConve
/*
* Move sequence ownerships from shard table to shell table and also drop
* DEFAULT expressions from shard relation as we should evaluate such columns
* in shell table when needed.
* DEFAULT expressions based on sequences from shard relation as we should
* evaluate such columns in shell table when needed.
*/
DropDefaultExpressionsAndMoveOwnedSequenceOwnerships(shardRelationId,
shellRelationId);
DropNextValExprsAndMoveOwnedSeqOwnerships(shardRelationId,
shellRelationId);
InsertMetadataForCitusLocalTable(shellRelationId, shardId, autoConverted);
@ -1161,14 +1161,15 @@ GetRenameStatsCommandList(List *statsOidList, uint64 shardId)
/*
* DropDefaultExpressionsAndMoveOwnedSequenceOwnerships drops default column
* definitions for relation with sourceRelationId. Also, for each column that
* defaults to an owned sequence, it grants ownership to the same named column
* of the relation with targetRelationId.
* DropNextValExprsAndMoveOwnedSeqOwnerships drops default column definitions
* that are based on sequences for relation with sourceRelationId.
*
* Also, for each such column that owns a sequence, it grants ownership to the
* same named column of the relation with targetRelationId.
*/
static void
DropDefaultExpressionsAndMoveOwnedSequenceOwnerships(Oid sourceRelationId,
Oid targetRelationId)
DropNextValExprsAndMoveOwnedSeqOwnerships(Oid sourceRelationId,
Oid targetRelationId)
{
List *columnNameList = NIL;
List *ownedSequenceIdList = NIL;
@ -1179,9 +1180,28 @@ DropDefaultExpressionsAndMoveOwnedSequenceOwnerships(Oid sourceRelationId,
Oid ownedSequenceId = InvalidOid;
forboth_ptr_oid(columnName, columnNameList, ownedSequenceId, ownedSequenceIdList)
{
DropDefaultColumnDefinition(sourceRelationId, columnName);
/*
* We drop nextval() expressions because Citus currently evaluates
* nextval() on the shell table, not on the shards. Hence, there is
* no reason for keeping nextval(). Also, distributed/reference table
* shards do not have - so be consistent with those.
*
* Note that we keep other kind of DEFAULT expressions on shards
* because we still want to be able to evaluate DEFAULT expressions
* that are not based on sequences on shards, e.g., for foreign key
* - SET DEFAULT actions.
*/
AttrNumber columnAttrNumber = get_attnum(sourceRelationId, columnName);
if (ColumnDefaultsToNextVal(sourceRelationId, columnAttrNumber))
{
DropDefaultColumnDefinition(sourceRelationId, columnName);
}
/* column might not own a sequence */
/*
* Column might own a sequence without having a nextval() expr on it
* --e.g., due to ALTER SEQUENCE OWNED BY .. --, so check if that is
* the case even if the column doesn't have a DEFAULT.
*/
if (OidIsValid(ownedSequenceId))
{
TransferSequenceOwnership(ownedSequenceId, targetRelationId, columnName);

24
src/backend/distributed/commands/sequence.c
View File

@ -27,6 +27,7 @@
#include "nodes/makefuncs.h"
#include "distributed/worker_create_or_replace.h"
#include "nodes/parsenodes.h"
#include "rewrite/rewriteHandler.h"
#include "utils/builtins.h"
#include "utils/lsyscache.h"
@ -213,6 +214,29 @@ ExtractDefaultColumnsAndOwnedSequences(Oid relationId, List **columnNameList,
}
/*
* ColumnDefaultsToNextVal returns true if the column with attrNumber
* has a default expression that contains nextval().
*/
bool
ColumnDefaultsToNextVal(Oid relationId, AttrNumber attrNumber)
{
AssertArg(AttributeNumberIsValid(attrNumber));
Relation relation = RelationIdGetRelation(relationId);
Node *defExpr = build_column_default(relation, attrNumber);
RelationClose(relation);
if (defExpr == NULL)
{
/* column doesn't have a DEFAULT expression */
return false;
}
return contain_nextval_expression_walker(defExpr, NULL);
}
/*
* PreprocessDropSequenceStmt gets called during the planning phase of a DROP SEQUENCE statement
* and returns a list of DDLJob's that will drop any distributed sequences from the

2
src/include/distributed/commands/sequence.h
View File

@ -10,9 +10,11 @@
#ifndef CITUS_SEQUENCE_H
#define CITUS_SEQUENCE_H
#include "access/attnum.h"
#include "nodes/pg_list.h"
extern bool ColumnDefaultsToNextVal(Oid relationId, AttrNumber attrNumber);
extern void ExtractDefaultColumnsAndOwnedSequences(Oid relationId,
List **columnNameList,
List **ownedSequenceIdList);

52
src/test/regress/expected/pg15.out
View File

@ -349,6 +349,58 @@ NOTICE: renaming the new table to pg15.tbl2
(1 row)
-- Make sure that we allow foreign key columns on local tables added to
-- metadata to have SET NULL/DEFAULT on column basis.
CREATE TABLE PKTABLE_local (tid int, id int, PRIMARY KEY (tid, id));
CREATE TABLE FKTABLE_local (
tid int, id int,
fk_id_del_set_null int,
fk_id_del_set_default int DEFAULT 0,
FOREIGN KEY (tid, fk_id_del_set_null) REFERENCES PKTABLE_local ON DELETE SET NULL (fk_id_del_set_null),
FOREIGN KEY (tid, fk_id_del_set_default) REFERENCES PKTABLE_local ON DELETE SET DEFAULT (fk_id_del_set_default)
);
SELECT citus_add_local_table_to_metadata('FKTABLE_local', cascade_via_foreign_keys=>true);
citus_add_local_table_to_metadata
---------------------------------------------------------------------
(1 row)
-- show that the definition is expected
SELECT pg_get_constraintdef(oid) FROM pg_constraint WHERE conrelid = 'FKTABLE_local'::regclass::oid ORDER BY oid;
pg_get_constraintdef
---------------------------------------------------------------------
FOREIGN KEY (tid, fk_id_del_set_default) REFERENCES pktable_local(tid, id) ON DELETE SET DEFAULT (fk_id_del_set_default)
FOREIGN KEY (tid, fk_id_del_set_null) REFERENCES pktable_local(tid, id) ON DELETE SET NULL (fk_id_del_set_null)
(2 rows)
\c - - - :worker_1_port
SET search_path TO pg15;
-- show that the definition is expected on the worker as well
SELECT pg_get_constraintdef(oid) FROM pg_constraint WHERE conrelid = 'FKTABLE_local'::regclass::oid ORDER BY oid;
pg_get_constraintdef
---------------------------------------------------------------------
FOREIGN KEY (tid, fk_id_del_set_default) REFERENCES pktable_local(tid, id) ON DELETE SET DEFAULT (fk_id_del_set_default)
FOREIGN KEY (tid, fk_id_del_set_null) REFERENCES pktable_local(tid, id) ON DELETE SET NULL (fk_id_del_set_null)
(2 rows)
-- also, make sure that it works as expected
INSERT INTO PKTABLE_local VALUES (1, 0), (1, 1), (1, 2);
INSERT INTO FKTABLE_local VALUES
(1, 1, 1, NULL),
(1, 2, NULL, 2);
DELETE FROM PKTABLE_local WHERE id = 1 OR id = 2;
SELECT * FROM FKTABLE_local ORDER BY id;
tid | id | fk_id_del_set_null | fk_id_del_set_default
---------------------------------------------------------------------
1 | 1 | |
1 | 2 | | 0
(2 rows)
\c - - - :master_port
SET search_path TO pg15;
SET client_min_messages to ERROR;
DROP TABLE FKTABLE_local, PKTABLE_local;
RESET client_min_messages;
SELECT 1 FROM citus_remove_node('localhost', :master_port);
?column?
---------------------------------------------------------------------

41
src/test/regress/sql/pg15.sql
View File

@ -213,6 +213,47 @@ WHEN MATCHED THEN DELETE;
-- now, both distributed, not works
SELECT undistribute_table('tbl1');
SELECT undistribute_table('tbl2');
-- Make sure that we allow foreign key columns on local tables added to
-- metadata to have SET NULL/DEFAULT on column basis.
CREATE TABLE PKTABLE_local (tid int, id int, PRIMARY KEY (tid, id));
CREATE TABLE FKTABLE_local (
tid int, id int,
fk_id_del_set_null int,
fk_id_del_set_default int DEFAULT 0,
FOREIGN KEY (tid, fk_id_del_set_null) REFERENCES PKTABLE_local ON DELETE SET NULL (fk_id_del_set_null),
FOREIGN KEY (tid, fk_id_del_set_default) REFERENCES PKTABLE_local ON DELETE SET DEFAULT (fk_id_del_set_default)
);
SELECT citus_add_local_table_to_metadata('FKTABLE_local', cascade_via_foreign_keys=>true);
-- show that the definition is expected
SELECT pg_get_constraintdef(oid) FROM pg_constraint WHERE conrelid = 'FKTABLE_local'::regclass::oid ORDER BY oid;
\c - - - :worker_1_port
SET search_path TO pg15;
-- show that the definition is expected on the worker as well
SELECT pg_get_constraintdef(oid) FROM pg_constraint WHERE conrelid = 'FKTABLE_local'::regclass::oid ORDER BY oid;
-- also, make sure that it works as expected
INSERT INTO PKTABLE_local VALUES (1, 0), (1, 1), (1, 2);
INSERT INTO FKTABLE_local VALUES
(1, 1, 1, NULL),
(1, 2, NULL, 2);
DELETE FROM PKTABLE_local WHERE id = 1 OR id = 2;
SELECT * FROM FKTABLE_local ORDER BY id;
\c - - - :master_port
SET search_path TO pg15;
SET client_min_messages to ERROR;
DROP TABLE FKTABLE_local, PKTABLE_local;
RESET client_min_messages;
SELECT 1 FROM citus_remove_node('localhost', :master_port);
SELECT create_distributed_table('tbl1', 'x');