From b1882d440063d055c4f0da2c71a4faf3f584434e Mon Sep 17 00:00:00 2001 From: Jeff Davis Date: Fri, 5 Feb 2021 10:00:32 -0800 Subject: [PATCH] Columnar: Call nextval_internal instead of DirectFunctionCall. --- src/backend/columnar/cstore_metadata_tables.c | 49 ++++++------------- src/test/regress/columnar_am_schedule | 1 + .../regress/expected/columnar_permissions.out | 13 +++++ src/test/regress/sql/columnar_permissions.sql | 17 +++++++ 4 files changed, 46 insertions(+), 34 deletions(-) create mode 100644 src/test/regress/expected/columnar_permissions.out create mode 100644 src/test/regress/sql/columnar_permissions.sql diff --git a/src/backend/columnar/cstore_metadata_tables.c b/src/backend/columnar/cstore_metadata_tables.c index 1af26e7aa..c69ce959d 100644 --- a/src/backend/columnar/cstore_metadata_tables.c +++ b/src/backend/columnar/cstore_metadata_tables.c @@ -27,6 +27,7 @@ #include "catalog/pg_type.h" #include "catalog/namespace.h" #include "commands/defrem.h" +#include "commands/sequence.h" #include "commands/trigger.h" #include "distributed/metadata_cache.h" #include "executor/executor.h" @@ -79,6 +80,7 @@ static void GetHighestUsedAddressAndId(uint64 storageId, uint64 *highestUsedAddress, uint64 *highestUsedId); static List * ReadDataFileStripeList(uint64 storageId, Snapshot snapshot); +static Oid ColumnarStorageIdSequenceRelationId(void); static Oid ColumnarStripeRelationId(void); static Oid ColumnarStripeIndexRelationId(void); static Oid ColumnarOptionsRelationId(void); @@ -96,7 +98,6 @@ static bytea * DatumToBytea(Datum value, Form_pg_attribute attrForm); static Datum ByteaToDatum(bytea *bytes, Form_pg_attribute attrForm); static ColumnarMetapage * InitMetapage(Relation relation); static ColumnarMetapage * ReadMetapage(RelFileNode relfilenode, bool missingOk); -static uint64 GetNextStorageId(void); static bool WriteColumnarOptions(Oid regclass, ColumnarOptions *options, bool overwrite); PG_FUNCTION_INFO_V1(columnar_relation_storageid); @@ -1048,6 +1049,17 @@ ByteaToDatum(bytea *bytes, Form_pg_attribute attrForm) } +/* + * ColumnarStorageIdSequenceRelationId returns relation id of columnar.stripe. + * TODO: should we cache this similar to citus? + */ +static Oid +ColumnarStorageIdSequenceRelationId(void) +{ + return get_relname_relid("storageid_seq", ColumnarNamespaceId()); +} + + /* * ColumnarStripeRelationId returns relation id of columnar.stripe. * TODO: should we cache this similar to citus? @@ -1179,9 +1191,9 @@ InitMetapage(Relation relation) * invisible. */ Assert(!IsBinaryUpgrade); - ColumnarMetapage *metapage = palloc0(sizeof(ColumnarMetapage)); - metapage->storageId = GetNextStorageId(); + + metapage->storageId = nextval_internal(ColumnarStorageIdSequenceRelationId(), false); metapage->versionMajor = COLUMNAR_VERSION_MAJOR; metapage->versionMinor = COLUMNAR_VERSION_MINOR; @@ -1196,37 +1208,6 @@ InitMetapage(Relation relation) } -/* - * GetNextStorageId returns the next value from the storage id sequence. - */ -static uint64 -GetNextStorageId(void) -{ - Oid savedUserId = InvalidOid; - int savedSecurityContext = 0; - Oid sequenceId = get_relname_relid("storageid_seq", ColumnarNamespaceId()); - Datum sequenceIdDatum = ObjectIdGetDatum(sequenceId); - - /* - * Not all users have update access to the sequence, so switch - * security context. - */ - GetUserIdAndSecContext(&savedUserId, &savedSecurityContext); - SetUserIdAndSecContext(CitusExtensionOwner(), SECURITY_LOCAL_USERID_CHANGE); - - /* - * Generate new and unique storage id from sequence. - */ - Datum storageIdDatum = DirectFunctionCall1(nextval_oid, sequenceIdDatum); - - SetUserIdAndSecContext(savedUserId, savedSecurityContext); - - uint64 storageId = DatumGetInt64(storageIdDatum); - - return storageId; -} - - /* * columnar_relation_storageid returns storage id associated with the * given relation id, or -1 if there is no associated storage id yet. diff --git a/src/test/regress/columnar_am_schedule b/src/test/regress/columnar_am_schedule index f1a2498e1..430b7aaa8 100644 --- a/src/test/regress/columnar_am_schedule +++ b/src/test/regress/columnar_am_schedule @@ -11,6 +11,7 @@ test: am_drop test: am_indexes test: columnar_fallback_scan test: columnar_partitioning +test: columnar_permissions test: am_empty test: am_insert test: am_update_delete diff --git a/src/test/regress/expected/columnar_permissions.out b/src/test/regress/expected/columnar_permissions.out new file mode 100644 index 000000000..eb058c951 --- /dev/null +++ b/src/test/regress/expected/columnar_permissions.out @@ -0,0 +1,13 @@ +select current_user \gset +create user columnar_user; +NOTICE: not propagating CREATE ROLE/USER commands to worker nodes +HINT: Connect to worker nodes directly to manually create all necessary users and roles. +\c - columnar_user +create table columnar_permissions(i int) using columnar; +insert into columnar_permissions values(1); +alter table columnar_permissions add column j int; +insert into columnar_permissions values(2,20); +vacuum columnar_permissions; +truncate columnar_permissions; +drop table columnar_permissions; +\c - :current_user diff --git a/src/test/regress/sql/columnar_permissions.sql b/src/test/regress/sql/columnar_permissions.sql new file mode 100644 index 000000000..e131de55c --- /dev/null +++ b/src/test/regress/sql/columnar_permissions.sql @@ -0,0 +1,17 @@ + +select current_user \gset + +create user columnar_user; + +\c - columnar_user + +create table columnar_permissions(i int) using columnar; +insert into columnar_permissions values(1); +alter table columnar_permissions add column j int; +insert into columnar_permissions values(2,20); +vacuum columnar_permissions; +truncate columnar_permissions; +drop table columnar_permissions; + +\c - :current_user +