mirror of https://github.com/citusdata/citus.git
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 actionspull/6367/head
Onur Tirtir
GitHub
parent
1ede0b9db3
commit
de24a3eda5
|
|
@ -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);
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
|
|
|
|||
|
|
@ -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?
|
||||
---------------------------------------------------------------------
|
||||
|
|
|
|||
|
|
@ -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');
|
||||
|
|
|
|||
Loading…
Reference in New Issue