Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement "citus local table" creation logic and a udf #3852

Merged
merged 36 commits into from
Jun 25, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
f9d0b0e
extern GetForeignKeyOids
onurctirtir May 20, 2020
b058af1
extern EnsureTableNotDistributed
onurctirtir May 31, 2020
8e223fd
implement table creation
onurctirtir Jun 1, 2020
deb1a85
add tests for create_citus_local_table
onurctirtir Apr 13, 2020
48480d3
update normalize rule
onurctirtir May 15, 2020
2fee5b8
fixup
onurctirtir Jun 4, 2020
080b27a
fixup: lock pg_dist_node in CoordinatorNode()
onurctirtir Jun 6, 2020
d635536
fixup: test if we escape object names properly
onurctirtir Jun 6, 2020
4695d5a
fixup: fix wrong variable "referenced"
onurctirtir Jun 8, 2020
266b150
fixup: use generate_qualified_relation_name
onurctirtir Jun 8, 2020
23e5280
fixup: move sequences to shell table
onurctirtir Jun 8, 2020
31f15f5
fixup: (please see detailed commit message)
onurctirtir Jun 9, 2020
195f146
fixup: test initial foreign keys
onurctirtir Jun 9, 2020
27d1e85
fixup: do not release lock until the end of xact
onurctirtir Jun 9, 2020
4a4aba5
fixup: error out if relation does not exist
onurctirtir Jun 9, 2020
c823d20
fixup: add some isolation tests
onurctirtir Jun 9, 2020
5d9b35e
fixup: add another normalization rule for isolation tests
onurctirtir Jun 10, 2020
7f7a612
fixup: simple tests for foreign tables & triggers
onurctirtir Jun 10, 2020
16cb979
fixup: improve error message
onurctirtir Jun 10, 2020
92b487c
fixup: don't support policies in community edition
onurctirtir Jun 11, 2020
7807f5f
fixup: tests with inheritance & unlogged tables
onurctirtir Jun 11, 2020
d55dac1
remove unused function for fkeys
onurctirtir Jun 22, 2020
7d91ad8
sequences: address review & fix another bug
onurctirtir Jun 22, 2020
37341cd
log commands that are executed
onurctirtir Jun 22, 2020
736877a
rename triggers on shard relation
onurctirtir Jun 22, 2020
12fcc53
address some other reviews
onurctirtir Jun 22, 2020
39df007
improve tests for triggeers
onurctirtir Jun 22, 2020
da74ce7
add test for sequences
onurctirtir Jun 23, 2020
6fa943e
fix access to pg_trigger for pg11
onurctirtir Jun 23, 2020
e40826d
Revert "update normalize rule"
onurctirtir Jun 23, 2020
025a428
improve comment for getOwnedSequences
onurctirtir Jun 23, 2020
66954e3
remove redundant check in IsCitusLocalTable
onurctirtir Jun 23, 2020
10c5a47
address reviews
onurctirtir Jun 25, 2020
1753a5f
address
onurctirtir Jun 25, 2020
faf6ee6
fix normalization rule
onurctirtir Jun 25, 2020
1e2995a
fix test output
onurctirtir Jun 25, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
81 changes: 1 addition & 80 deletions src/backend/distributed/commands/create_citus_local_table.c
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,6 @@

static void ErrorIfUnsupportedCreateCitusLocalTable(Relation relation);
static void ErrorIfUnsupportedCitusLocalTableKind(Oid relationId);
static void ErrorIfRelationIsAKnownShard(Oid relationId);
static void ErrorIfTableHasExternalForeignKeys(Oid relationId);
static void ErrorIfCoordinatorNotAddedAsWorkerNode(Oid relationId);
static uint64 ConvertLocalTableToShard(Oid relationId);
static void RenameRelationToShardRelation(Oid shellRelationId, uint64 shardId);
static void RenameShardRelationConstraints(Oid shardRelationId, uint64 shardId);
Expand Down Expand Up @@ -104,7 +101,7 @@ ErrorIfUnsupportedCreateCitusLocalTable(Relation relation)

Oid relationId = relation->rd_id;

ErrorIfCoordinatorNotAddedAsWorkerNode(relationId);
ErrorIfCoordinatorNotAddedAsWorkerNode();
ErrorIfUnsupportedCitusLocalTableKind(relationId);
EnsureTableNotDistributed(relationId);
onderkalaci marked this conversation as resolved.
Show resolved Hide resolved

Expand Down Expand Up @@ -166,82 +163,6 @@ ErrorIfUnsupportedCitusLocalTableKind(Oid relationId)
}


/*
* ErrorIfRelationIsAKnownShard errors out if the relation with relationId is
* a shard relation relation.
*/
static void
ErrorIfRelationIsAKnownShard(Oid relationId)
{
/* search the relation in all schemas */
bool onlySearchPath = false;
if (!RelationIsAKnownShard(relationId, onlySearchPath))
{
return;
}

const char *relationName = get_rel_name(relationId);

ereport(ERROR, (errmsg("cannot create citus local table as \"%s\" is a shard "
"relation ", relationName)));
}


/*
* ErrorIfTableHasExternalForeignKeys errors out if the relation with relationId
* is involved in a foreign key relationship other than the self-referencing ones.
*/
static void
ErrorIfTableHasExternalForeignKeys(Oid relationId)
{
int flags = (INCLUDE_REFERENCING_CONSTRAINTS | EXCLUDE_SELF_REFERENCES);
List *foreignKeyIdsTableReferencing = GetForeignKeyOids(relationId, flags);

flags = (INCLUDE_REFERENCED_CONSTRAINTS | EXCLUDE_SELF_REFERENCES);
List *foreignKeyIdsTableReferenced = GetForeignKeyOids(relationId, flags);

List *foreignKeysWithOtherTables = list_concat(foreignKeyIdsTableReferencing,
foreignKeyIdsTableReferenced);

if (list_length(foreignKeysWithOtherTables) == 0)
{
return;
}

const char *relationName = get_rel_name(relationId);

ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("cannot create citus local table \"%s\", citus local "
"tables cannot be involved in foreign key relationships "
"with other tables initially", relationName),
errhint("Drop foreign keys with other tables and re-define them "
"with ALTER TABLE commands after creating the table.")));
}


/*
* ErrorIfCoordinatorNotAddedAsWorkerNode error out if coordinator is not added
* to metadata.
*/
static void
ErrorIfCoordinatorNotAddedAsWorkerNode(Oid relationId)
{
if (CoordinatorAddedAsWorkerNode())
{
return;
}

const char *relationName = get_rel_name(relationId);

ereport(ERROR, (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
errmsg("cannot create citus local table \"%s\", citus local "
"tables can only be created from coordinator node if "
"it is added as a worker node", relationName),
errhint("First, add the coordinator with master_add_node "
"command")));
}


/*
* CreateCitusLocalTable is the internal method that creates a citus table
* from the table with relationId. The created table would have the following
Expand Down
31 changes: 31 additions & 0 deletions src/backend/distributed/commands/foreign_constraint.c
Original file line number Diff line number Diff line change
Expand Up @@ -649,6 +649,37 @@ FindForeignKeyOidWithName(List *foreignKeyOids, const char *inputConstraintName)
}


/*
* ErrorIfTableHasExternalForeignKeys errors out if the relation with relationId
* is involved in a foreign key relationship other than the self-referencing ones.
*/
void
ErrorIfTableHasExternalForeignKeys(Oid relationId)
{
int flags = (INCLUDE_REFERENCING_CONSTRAINTS | EXCLUDE_SELF_REFERENCES);
List *foreignKeyIdsTableReferencing = GetForeignKeyOids(relationId, flags);

flags = (INCLUDE_REFERENCED_CONSTRAINTS | EXCLUDE_SELF_REFERENCES);
List *foreignKeyIdsTableReferenced = GetForeignKeyOids(relationId, flags);

List *foreignKeysWithOtherTables = list_concat(foreignKeyIdsTableReferencing,
foreignKeyIdsTableReferenced);

if (list_length(foreignKeysWithOtherTables) == 0)
{
return;
}

const char *relationName = get_rel_name(relationId);
ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("relation \"%s\" is involved in a foreign key relationship "
"with another table", relationName),
errhint("Drop foreign keys with other tables and re-define them "
"with ALTER TABLE commands after the current operation "
"is done.")));
}


/*
* GetForeignKeyOids takes in a relationId, and returns a list of OIDs for
* foreign constraints that the relation with relationId is involved according
Expand Down
23 changes: 18 additions & 5 deletions src/backend/distributed/operations/worker_node_manager.c
Original file line number Diff line number Diff line change
Expand Up @@ -445,11 +445,7 @@ ReferenceTablePlacementNodeList(LOCKMODE lockMode)
WorkerNode *
CoordinatorNode()
{
if (!CoordinatorAddedAsWorkerNode())
{
ereport(ERROR, (errmsg("couldn't find the coordinator node in the metadata "
"as it is not added as a worker")));
}
ErrorIfCoordinatorNotAddedAsWorkerNode();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For future reference, this line doesn't error out with the current tests.


WorkerNode *coordinatorNode = LookupNodeForGroup(COORDINATOR_GROUP_ID);

Expand All @@ -460,6 +456,23 @@ CoordinatorNode()
}


/*
* ErrorIfCoordinatorNotAddedAsWorkerNode errors out if coordinator is not added
* to metadata.
*/
void
ErrorIfCoordinatorNotAddedAsWorkerNode()
{
if (CoordinatorAddedAsWorkerNode())
{
return;
}

ereport(ERROR, (errmsg("could not find the coordinator node in "
"metadata as it is not added as a worker")));
}


/*
* DistributedTablePlacementNodeList returns a list of all active, primary
* worker nodes that can store new data, i.e shouldstoreshards is 'true'
Expand Down
19 changes: 19 additions & 0 deletions src/backend/distributed/worker/worker_shard_visibility.c
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,25 @@ citus_table_is_visible(PG_FUNCTION_ARGS)
}


/*
* ErrorIfRelationIsAKnownShard errors out if the relation with relationId is
* a shard relation.
*/
void
ErrorIfRelationIsAKnownShard(Oid relationId)
{
/* search the relation in all schemas */
bool onlySearchPath = false;
if (!RelationIsAKnownShard(relationId, onlySearchPath))
{
return;
}

const char *relationName = get_rel_name(relationId);
ereport(ERROR, (errmsg("relation \"%s\" is a shard relation ", relationName)));
}


/*
* RelationIsAKnownShard gets a relationId, check whether it's a shard of
* any distributed table. If onlySearchPath is true, then it searches
Expand Down
1 change: 1 addition & 0 deletions src/include/distributed/commands.h
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ extern bool HasForeignKeyToReferenceTable(Oid relationOid);
extern bool TableReferenced(Oid relationOid);
extern bool TableReferencing(Oid relationOid);
extern bool ConstraintIsAForeignKey(char *inputConstaintName, Oid relationOid);
extern void ErrorIfTableHasExternalForeignKeys(Oid relationId);
extern List * GetForeignKeyOids(Oid relationId, int flags);


Expand Down
1 change: 1 addition & 0 deletions src/include/distributed/worker_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ extern List * ActivePrimaryNodeList(LOCKMODE lockMode);
extern bool CoordinatorAddedAsWorkerNode(void);
extern List * ReferenceTablePlacementNodeList(LOCKMODE lockMode);
extern WorkerNode * CoordinatorNode(void);
extern void ErrorIfCoordinatorNotAddedAsWorkerNode(void);
extern List * DistributedTablePlacementNodeList(LOCKMODE lockMode);
extern bool NodeCanHaveDistTablePlacements(WorkerNode *node);
extern uint32 ActiveReadableWorkerNodeCount(void);
Expand Down
1 change: 1 addition & 0 deletions src/include/distributed/worker_shard_visibility.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ extern bool OverrideTableVisibility;


extern void ReplaceTableVisibleFunction(Node *inputNode);
extern void ErrorIfRelationIsAKnownShard(Oid relationId);
extern bool RelationIsAKnownShard(Oid shardRelationId, bool onlySearchPath);


Expand Down
2 changes: 1 addition & 1 deletion src/test/regress/bin/normalize.sed
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ s/ keyval(1|2|ref)_[0-9]+ / keyval\1_xxxxxxx /g
s/ daily_uniques_[0-9]+ / daily_uniques_xxxxxxx /g

# shard table names for isolation_create_citus_local_table
s/ "citus_local_table_[0-9]_[0-9]+" / "citus_local_table_[0-9]_xxxxxxx" /g
s/ "citus_local_table_[0-9]+_[0-9]+" / "citus_local_table_[0-9]+_xxxxxxx" /g

# In foreign_key_restriction_enforcement, normalize shard names
s/"(on_update_fkey_table_|fkey_)[0-9]+"/"\1xxxxxxx"/g
Expand Down
22 changes: 9 additions & 13 deletions src/test/regress/expected/citus_local_tables.out
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ SELECT 1 FROM master_remove_node('localhost', :master_port);
CREATE TABLE citus_local_table_1 (a int primary key);
-- this should fail as coordinator is removed from pg_dist_node
SELECT create_citus_local_table('citus_local_table_1');
ERROR: cannot create citus local table "citus_local_table_1", citus local tables can only be created from coordinator node if it is added as a worker node
ERROR: could not find the coordinator node in metadata as it is not added as a worker
-- let coordinator have citus local tables again for next tests
set client_min_messages to ERROR;
SELECT 1 FROM master_add_node('localhost', :master_port, groupId => 0);
Expand Down Expand Up @@ -96,32 +96,28 @@ SELECT create_citus_local_table('citus_local_table_2');
(1 row)

CREATE TABLE distributed_table (a int);
-- cannot create citus local table from an existing citus table
SELECT create_distributed_table('distributed_table', 'a');
create_distributed_table
---------------------------------------------------------------------

(1 row)

-- this will error out
-- cannot create citus local table from an existing citus table
SELECT create_citus_local_table('distributed_table');
ERROR: table "distributed_table" is already distributed
-- partitiond table tests --
-- partitioned table tests --
CREATE TABLE partitioned_table(a int, b int) PARTITION BY RANGE (a);
CREATE TABLE partitioned_table_1 PARTITION OF partitioned_table FOR VALUES FROM (0) TO (10);
CREATE TABLE partitioned_table_2 PARTITION OF partitioned_table FOR VALUES FROM (10) TO (20);
-- cannot create partitioned citus local tables
SELECT create_citus_local_table('partitioned_table');
ERROR: cannot create citus local table "partitioned_table", only regular tables and foreign tables are supported for citus local table creation
-- cannot create citus local table as a partition of a local table
BEGIN;
CREATE TABLE citus_local_table PARTITION OF partitioned_table FOR VALUES FROM (20) TO (30);
-- this should fail
-- cannot create citus local table as a partition of a local table
SELECT create_citus_local_table('citus_local_table');
ERROR: cannot create citus local table "citus_local_table", citus local tables cannot be partition of other tables
ROLLBACK;
-- cannot create citus local table as a partition of a local table
-- via ALTER TABLE commands as well
BEGIN;
CREATE TABLE citus_local_table (a int, b int);
SELECT create_citus_local_table('citus_local_table');
Expand All @@ -130,11 +126,11 @@ BEGIN;

(1 row)

-- this should fail
-- cannot create citus local table as a partition of a local table
-- via ALTER TABLE commands as well
ALTER TABLE partitioned_table ATTACH PARTITION citus_local_table FOR VALUES FROM (20) TO (30);
ERROR: non-distributed tables cannot have distributed partitions
ROLLBACK;
-- cannot attach citus local table to a partitioned distributed table
BEGIN;
SELECT create_distributed_table('partitioned_table', 'a');
create_distributed_table
Expand All @@ -149,7 +145,7 @@ BEGIN;

(1 row)

-- this should fail
-- cannot attach citus local table to a partitioned distributed table
ALTER TABLE partitioned_table ATTACH PARTITION citus_local_table FOR VALUES FROM (20) TO (30);
ERROR: distributed tables cannot have non-colocated distributed tables as a partition
ROLLBACK;
Expand Down Expand Up @@ -318,9 +314,9 @@ CREATE TABLE local_table_3 (a int primary key, b int references local_table_3(a)
-- below two should fail as we do not allow foreign keys between
-- postgres local tables and citus local tables
SELECT create_citus_local_table('local_table_1');
ERROR: cannot create citus local table "local_table_1", citus local tables cannot be involved in foreign key relationships with other tables initially
ERROR: relation "local_table_1" is involved in a foreign key relationship with another table
SELECT create_citus_local_table('local_table_2');
ERROR: cannot create citus local table "local_table_2", citus local tables cannot be involved in foreign key relationships with other tables initially
ERROR: relation "local_table_2" is involved in a foreign key relationship with another table
-- below should work as we allow initial self references in citus local tables
SELECT create_citus_local_table('local_table_3');
create_citus_local_table
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ create_citus_local_table
step s2-create-citus-local-table-1: SELECT create_citus_local_table('citus_local_table_1'); <waiting ...>
step s1-commit: COMMIT;
step s2-create-citus-local-table-1: <... completed>
error in steps s1-commit s2-create-citus-local-table-1: ERROR: cannot create citus local table as "citus_local_table_[0-9]_xxxxxxx" is a shard relation
error in steps s1-commit s2-create-citus-local-table-1: ERROR: relation "citus_local_table_[0-9]+_xxxxxxx" is a shard relation
step s2-commit: COMMIT;
master_remove_node

Expand All @@ -26,7 +26,7 @@ create_citus_local_table
step s2-create-citus-local-table-3: SELECT create_citus_local_table('another_schema.citus_local_table_3'); <waiting ...>
step s1-commit: COMMIT;
step s2-create-citus-local-table-3: <... completed>
error in steps s1-commit s2-create-citus-local-table-3: ERROR: cannot create citus local table as "citus_local_table_[0-9]_xxxxxxx" is a shard relation
error in steps s1-commit s2-create-citus-local-table-3: ERROR: relation "citus_local_table_[0-9]+_xxxxxxx" is a shard relation
step s2-commit: COMMIT;
master_remove_node

Expand Down Expand Up @@ -187,7 +187,7 @@ master_remove_node
step s2-create-citus-local-table-1: SELECT create_citus_local_table('citus_local_table_1'); <waiting ...>
step s1-commit: COMMIT;
step s2-create-citus-local-table-1: <... completed>
error in steps s1-commit s2-create-citus-local-table-1: ERROR: cannot create citus local table "citus_local_table_1", citus local tables can only be created from coordinator node if it is added as a worker node
error in steps s1-commit s2-create-citus-local-table-1: ERROR: could not find the coordinator node in metadata as it is not added as a worker
step s2-commit: COMMIT;
master_remove_node

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ step "s2-rollback" { ROLLBACK; }

// create_citus_local_table vs command/query //
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: is the // at the end a leftover?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No actually, just to emphasize section comment


// Second session should error out as the table become a citus local table after the first session commits ..
// Second session should error out as the table becomes a citus local table after the first session commits ..
permutation "s1-begin" "s2-begin" "s1-create-citus-local-table-1" "s2-create-citus-local-table-1" "s1-commit" "s2-commit"
// .. and it should error out even if we are in a different schema than the table
permutation "s1-begin" "s2-begin" "s1-create-citus-local-table-3" "s2-create-citus-local-table-3" "s1-commit" "s2-commit"
Expand Down