From f363a09c4f415616cac624704164fef59caabcb8 Mon Sep 17 00:00:00 2001 From: Alex Abdugafarov Date: Tue, 28 Jun 2022 22:46:38 +0400 Subject: [PATCH] [#12719] YSQL: Remove tablegroup_oid reloption Summary: At the time prior to this diff, `tablegroup_oid` remained the only YB-specifc property that we store in `reloptions` rather than in YB-specific struct. Since master is always aware of the tablegroup, moved this to be exclusive to `YbTableProperties` and to be fetched from master. One side effect of this change is that table's primary key is no longer shown as belonging to the tablegroup (since PK is a dummy thing in YB). Discussed this to be okay (and we can tweak this later if needed). Test Plan: Existing tests Reviewers: myang, tverona, mihnea, dmitry, jason Reviewed By: jason Subscribers: yql, bogdan Differential Revision: https://phabricator.dev.yugabyte.com/D17779 --- .../src/backend/access/common/reloptions.c | 12 -- .../src/backend/bootstrap/bootparse.y | 4 +- src/postgres/src/backend/catalog/aclchk.c | 8 +- src/postgres/src/backend/commands/cluster.c | 2 +- src/postgres/src/backend/commands/indexcmds.c | 28 ++-- src/postgres/src/backend/commands/tablecmds.c | 110 ++++------------ .../src/backend/commands/tablegroup.c | 71 ++--------- src/postgres/src/backend/nodes/copyfuncs.c | 1 - src/postgres/src/backend/nodes/equalfuncs.c | 1 - src/postgres/src/backend/parser/gram.y | 1 - .../src/backend/parser/parse_utilcmd.c | 22 +--- .../src/backend/utils/adt/ruleutils.c | 4 +- .../src/backend/utils/misc/pg_yb_utils.c | 6 +- src/postgres/src/bin/pg_dump/pg_dump.c | 57 +-------- src/postgres/src/bin/psql/describe.c | 120 ++++++++---------- src/postgres/src/fe_utils/string_utils.c | 7 - src/postgres/src/include/catalog/dependency.h | 2 +- .../src/include/commands/tablegroup.h | 1 - src/postgres/src/include/nodes/parsenodes.h | 2 - src/postgres/src/include/utils/rel.h | 9 -- .../test/regress/expected/yb_create_index.out | 2 +- .../test/regress/expected/yb_tablegroup.out | 18 +-- .../expected/yb_tablegroup_colocation_id.out | 4 +- .../src/test/regress/sql/yb_tablegroup.sql | 6 +- src/yb/master/catalog_manager.cc | 7 + src/yb/master/master_ddl.proto | 2 + src/yb/yql/pggate/pg_tabledesc.cc | 4 + src/yb/yql/pggate/pg_tabledesc.h | 3 + src/yb/yql/pggate/ybc_pg_typedefs.h | 2 +- src/yb/yql/pggate/ybc_pggate.cc | 15 ++- src/yb/yql/pggate/ybc_pggate.h | 5 +- 31 files changed, 153 insertions(+), 383 deletions(-) diff --git a/src/postgres/src/backend/access/common/reloptions.c b/src/postgres/src/backend/access/common/reloptions.c index f181f5723fd5..eb5982f32061 100644 --- a/src/postgres/src/backend/access/common/reloptions.c +++ b/src/postgres/src/backend/access/common/reloptions.c @@ -377,17 +377,6 @@ static relopt_int intRelOpts[] = static relopt_oid oidRelOpts[] = { - { - { - "tablegroup_oid", - "Tablegroup oid for this relation.", - RELOPT_KIND_HEAP | RELOPT_KIND_INDEX, - AccessExclusiveLock - }, - InvalidOid, - FirstNormalObjectId, - OID_MAX - }, { { "colocation_id", @@ -1593,7 +1582,6 @@ default_reloptions(Datum reloptions, bool validate, relopt_kind kind) offsetof(StdRdOptions, vacuum_cleanup_index_scale_factor)}, {"colocated", RELOPT_TYPE_BOOL, offsetof(StdRdOptions, colocated)}, - {"tablegroup_oid", RELOPT_TYPE_OID, offsetof(StdRdOptions, tablegroup_oid)}, {"colocation_id", RELOPT_TYPE_OID, offsetof(StdRdOptions, colocation_id)}, {"table_oid", RELOPT_TYPE_OID, offsetof(StdRdOptions, table_oid)}, {"row_type_oid", RELOPT_TYPE_OID, offsetof(StdRdOptions, row_type_oid)}, diff --git a/src/postgres/src/backend/bootstrap/bootparse.y b/src/postgres/src/backend/bootstrap/bootparse.y index 88b420245423..cec749bb63c3 100644 --- a/src/postgres/src/backend/bootstrap/bootparse.y +++ b/src/postgres/src/backend/bootstrap/bootparse.y @@ -262,7 +262,7 @@ Boot_CreateStmt: boot_reldesc = heap_create($2, PG_CATALOG_NAMESPACE, shared_relation ? GLOBALTABLESPACE_OID : 0, - InvalidOid, /* tablegroup */ + InvalidOid, /* reltablegroup */ $3, InvalidOid, tupdesc, @@ -280,7 +280,7 @@ Boot_CreateStmt: id = heap_create_with_catalog($2, PG_CATALOG_NAMESPACE, shared_relation ? GLOBALTABLESPACE_OID : 0, - InvalidOid, /* tablegroup */ + InvalidOid, /* reltablegroup */ $3, $7, InvalidOid, diff --git a/src/postgres/src/backend/catalog/aclchk.c b/src/postgres/src/backend/catalog/aclchk.c index cdf23a1d2278..aa18869c1b77 100644 --- a/src/postgres/src/backend/catalog/aclchk.c +++ b/src/postgres/src/backend/catalog/aclchk.c @@ -4430,7 +4430,7 @@ pg_tablegroup_aclmask(Oid grp_oid, Oid roleid, Acl *acl; Oid ownerId; - // First check that the pg_tablegroup catalog actually exists. + /* First check that the pg_tablegroup catalog actually exists. */ if (!YbTablegroupCatalogExists) { ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), @@ -4442,7 +4442,7 @@ pg_tablegroup_aclmask(Oid grp_oid, Oid roleid, return mask; /* - * Get the tablegroup's ACL from pg_tablegroup + * Get the tablegroup's ACL from pg_yb_tablegroup */ tuple = SearchSysCache1(YBTABLEGROUPOID, ObjectIdGetDatum(grp_oid)); if (!HeapTupleIsValid(tuple)) @@ -5203,7 +5203,7 @@ pg_tablegroup_ownercheck(Oid grp_oid, Oid roleid) HeapTuple grptuple; Oid grpowner; - // Ensure that the pg_tablegroup catalog actually exists. + /* Ensure that the pg_yb_tablegroup catalog actually exists. */ if (!YbTablegroupCatalogExists) { ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), @@ -5214,7 +5214,7 @@ pg_tablegroup_ownercheck(Oid grp_oid, Oid roleid) if (superuser_arg(roleid) || IsYbDbAdminUser(GetUserId())) return true; - /* Search syscache for pg_tablegroup */ + /* Search syscache for the tablegroup */ grptuple = SearchSysCache1(YBTABLEGROUPOID, ObjectIdGetDatum(grp_oid)); if (!HeapTupleIsValid(grptuple)) ereport(ERROR, diff --git a/src/postgres/src/backend/commands/cluster.c b/src/postgres/src/backend/commands/cluster.c index 8b22f0b9a824..ff401b861218 100644 --- a/src/postgres/src/backend/commands/cluster.c +++ b/src/postgres/src/backend/commands/cluster.c @@ -677,7 +677,7 @@ make_new_heap(Oid OIDOldHeap, Oid NewTableSpace, char relpersistence, OIDNewHeap = heap_create_with_catalog(NewHeapName, namespaceid, NewTableSpace, - InvalidOid, /* tablegroup */ + InvalidOid, /* reltablegroup */ InvalidOid, InvalidOid, InvalidOid, diff --git a/src/postgres/src/backend/commands/indexcmds.c b/src/postgres/src/backend/commands/indexcmds.c index e28583884913..755c4ffbe797 100644 --- a/src/postgres/src/backend/commands/indexcmds.c +++ b/src/postgres/src/backend/commands/indexcmds.c @@ -650,8 +650,12 @@ DefineIndex(Oid relationId, } /* Use tablegroup of the indexed table, if any. */ - Oid tablegroupId = YbTablegroupCatalogExists ? - get_tablegroup_oid_by_table_oid(relationId) : InvalidOid; + Oid tablegroupId = InvalidOid; + if (YbTablegroupCatalogExists && IsYBRelation(rel)) + { + YbLoadTablePropertiesIfNeeded(rel, false /* allow_missing */); + tablegroupId = rel->yb_table_properties->tablegroup_oid; + } if (OidIsValid(tablegroupId) && stmt->split_options) { @@ -695,19 +699,6 @@ DefineIndex(Oid relationId, get_tablegroup_name(tablegroupId)); } - /* - * Prepend to stmt->options to be parsed for reloptions if tablegroupId is valid. - * We set this here instead of in parse_utilcmd since we need to do the above - * preprocessing and RBAC checks first. This still happens before transformReloptions - * so this option is included in the reloptions text array. - */ - if (OidIsValid(tablegroupId)) - { - stmt->options = lcons(makeDefElem("tablegroup_oid", - (Node *) makeInteger(tablegroupId), -1), - stmt->options); - } - /* * Force shared indexes into the pg_global tablespace. This is a bit of a * hack but seems simpler than marking them in the BKI commands. On the @@ -1577,11 +1568,8 @@ ComputeIndexAttrs(IndexInfo *indexInfo, { YbLoadTablePropertiesIfNeeded(rel, false /* allow_missing */); - is_colocated = rel->yb_table_properties->is_colocated; - - if (YbTablegroupCatalogExists) - tablegroupId = get_tablegroup_oid_by_table_oid(relId); - + is_colocated = rel->yb_table_properties->is_colocated; + tablegroupId = rel->yb_table_properties->tablegroup_oid; use_yb_ordering = !IsSystemRelation(rel); } RelationClose(rel); diff --git a/src/postgres/src/backend/commands/tablecmds.c b/src/postgres/src/backend/commands/tablecmds.c index 613a8d5bddb2..e7bd1fbb83b6 100644 --- a/src/postgres/src/backend/commands/tablecmds.c +++ b/src/postgres/src/backend/commands/tablecmds.c @@ -326,7 +326,6 @@ struct DropRelationCallbackState #define child_dependency_type(child_is_partition) \ ((child_is_partition) ? DEPENDENCY_AUTO : DEPENDENCY_NORMAL) -static Oid GetTablegroupOidFromCreateStmt(CreateStmt *stmt); static void truncate_check_rel(Relation rel); static List *MergeAttributes(List *schema, List *supers, char relpersistence, bool is_partition, List **supconstr, int *supOidCount); @@ -770,7 +769,25 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId, } } - Oid tablegroupId = GetTablegroupOidFromCreateStmt(stmt); + Oid tablegroupId = stmt->tablegroup + ? get_tablegroup_oid(stmt->tablegroup->tablegroup_name, false) + : InvalidOid; + + /* + * Check permissions for tablegroup. To create a table within a tablegroup, a user must + * either be a superuser, the owner of the tablegroup, or have create perms on it. + */ + if (OidIsValid(tablegroupId) && !pg_tablegroup_ownercheck(tablegroupId, GetUserId())) + { + AclResult aclresult; + + aclresult = pg_tablegroup_aclcheck(tablegroupId, GetUserId(), ACL_CREATE); + if (aclresult != ACLCHECK_OK) + aclcheck_error(aclresult, OBJECT_YBTABLEGROUP, + get_tablegroup_name(tablegroupId)); + } + + Oid colocation_id = YbGetColocationIdFromRelOptions(stmt->options); @@ -1196,53 +1213,6 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId, return address; } -/* - * Select tablegroup to use. If not specified, InvalidOid. - * Will produce an error if the pg_tablegroup system table has not been created. - * Disallows creating a table within a tablegroup without the correct permissions. - */ -static Oid -GetTablegroupOidFromCreateStmt(CreateStmt *stmt) -{ - if (!stmt->tablegroup) - return InvalidOid; - - if (!stmt->tablegroup->has_tablegroup) - ereport(ERROR, - (errcode(ERRCODE_INVALID_TABLE_DEFINITION), - errmsg("Cannot use NO TABLEGROUP in CREATE TABLE statement."))); - - Oid tablegroupId = get_tablegroup_oid(stmt->tablegroup->tablegroup_name, false); - - if (!OidIsValid(tablegroupId)) - return InvalidOid; - - /* - * Check permissions for tablegroup. To create a table within a tablegroup, a user must - * either be a superuser, the owner of the tablegroup, or have create perms on it. - */ - if (!pg_tablegroup_ownercheck(tablegroupId, GetUserId())) - { - AclResult aclresult; - - aclresult = pg_tablegroup_aclcheck(tablegroupId, GetUserId(), ACL_CREATE); - if (aclresult != ACLCHECK_OK) - aclcheck_error(aclresult, OBJECT_YBTABLEGROUP, - get_tablegroup_name(tablegroupId)); - } - - /* - * Prepend to stmt->options to be parsed for reloptions if tablegroupId is valid. We need - * to do the preprocessing and RBAC checks first. This still happens before - * transformReloptions so this option is included in the reloptions text array. - */ - stmt->options = lcons(makeDefElem("tablegroup_oid", - (Node *) makeInteger(tablegroupId), -1), - stmt->options); - - return tablegroupId; -} - /* * Emit the right error or warning message for a "DROP" command issued on a * non-existent relation @@ -7690,39 +7660,6 @@ YBMoveRelDependencies(Relation old_rel, Relation new_rel, } } -/* - * Filter the DefElem list, removing all reloptions that should not be - * carried over when cloning the relation. - */ -static List * -YBExcludeNonCopyableOptions(List *options) -{ - if (options) - { - ListCell *prev = NULL; - ListCell *next; - - for (ListCell *curr = list_head(options); curr != NULL; curr = next) - { - next = lnext(curr); - DefElem *elem = lfirst_node(DefElem, curr); - - /* - * Tablegroup is stored as a reloption but should not be copied - * as such, there's a separate mechanism for it. - */ - if (strcmp(elem->defname, "tablegroup_oid") == 0) - { - options = list_delete_cell(options, curr, prev); - break; /* since we only have one case so far, we can stop early. */ - } - else - prev = curr; - } - } - return options; -} - /* * Primary key is an inherent part of a DocDB table, we can't literally "add" * a primary key to an existing table. @@ -7753,6 +7690,8 @@ YBCloneRelationSetPrimaryKey(Relation* mutable_rel, IndexStmt* stmt) MemoryContext oldcxt, per_tup_cxt; ObjectAddress result = InvalidObjectAddress, address; + Assert(IsYBRelation(*mutable_rel)); + old_relid = RelationGetRelid(*mutable_rel); constr = RelationGetDescr(*mutable_rel)->constr; @@ -7860,13 +7799,10 @@ YBCloneRelationSetPrimaryKey(Relation* mutable_rel, IndexStmt* stmt) create_stmt->options = untransformRelOptions(datum); ReleaseSysCache(tuple); - create_stmt->options = YBExcludeNonCopyableOptions(create_stmt->options); - - const Oid tablegroup_oid = RelationGetTablegroupOid(*mutable_rel); + const Oid tablegroup_oid = (*mutable_rel)->yb_table_properties->tablegroup_oid; if (OidIsValid(tablegroup_oid)) { create_stmt->tablegroup = makeNode(OptTableGroup); - create_stmt->tablegroup->has_tablegroup = true; create_stmt->tablegroup->tablegroup_name = get_tablegroup_name(tablegroup_oid); Assert(create_stmt->tablegroup->tablegroup_name); } @@ -8186,8 +8122,6 @@ YBCloneRelationSetPrimaryKey(Relation* mutable_rel, IndexStmt* stmt) RenameRelation(rename_stmt); CommandCounterIncrement(); - idx_stmt->options = YBExcludeNonCopyableOptions(idx_stmt->options); - /* Create a new index taking up the freed name. */ idx_stmt->idxname = pstrdup(idx_orig_name); idx_addr = DefineIndex(new_relid, diff --git a/src/postgres/src/backend/commands/tablegroup.c b/src/postgres/src/backend/commands/tablegroup.c index d99886792656..e805290d050b 100644 --- a/src/postgres/src/backend/commands/tablegroup.c +++ b/src/postgres/src/backend/commands/tablegroup.c @@ -188,7 +188,7 @@ CreateTableGroup(CreateTableGroupStmt *stmt) tablespaceoid); recordDependencyOnOwner(YbTablegroupRelationId, tablegroupoid, owneroid); - /* We keep the lock on pg_tablegroup until commit */ + /* We keep the lock on pg_yb_tablegroup until commit */ heap_close(rel, NoLock); return tablegroupoid; @@ -217,8 +217,8 @@ get_tablegroup_oid(const char *tablegroupname, bool missing_ok) } /* - * Search pg_tablegroup. We use a heapscan here even though there is an - * index on name, on the theory that pg_tablegroup will usually have just + * Search pg_yb_tablegroup. We use a heapscan here even though there is an + * index on name, on the theory that pg_yb_tablegroup will usually have just * a few entries and so an indexed lookup is a waste of effort. */ rel = heap_open(YbTablegroupRelationId, AccessShareLock); @@ -248,59 +248,6 @@ get_tablegroup_oid(const char *tablegroupname, bool missing_ok) return result; } -/* - * get_tablegroup_oid_by_table_oid - given a table oid, look up its tablegroup oid (if any) - */ -Oid -get_tablegroup_oid_by_table_oid(Oid table_oid) -{ - Oid result = InvalidOid; - HeapTuple tuple; - - // get_tablegroup_oid_by_table_oid will only be called if YbTablegroupCatalogExists so this point - // error should not occur here. Added check just in case. - if (!YbTablegroupCatalogExists) - { - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("Tablegroup system catalog does not exist."))); - } - - /* - * Search pg_class using a cache lookup as pg_class can grow large. - */ - tuple = SearchSysCache1(RELOID, ObjectIdGetDatum(table_oid)); - - /* We assume that there can be at most one matching tuple */ - if (HeapTupleIsValid(tuple)) - { - bool isnull; - Datum datum = SysCacheGetAttr(RELOID, - tuple, - Anum_pg_class_reloptions, - &isnull); - - if (!isnull) { - List *reloptions = untransformRelOptions(datum); - ListCell *cell; - foreach(cell, reloptions) - { - DefElem *defel = (DefElem *) lfirst(cell); - // Every node is a string node when untransformed. Need to type cast. - if (strcmp(defel->defname, "tablegroup_oid") == 0) - { - result = (Oid) pg_atoi(defGetString(defel), sizeof(Oid), 0); - } - } - } - - } - - ReleaseSysCache(tuple); - - return result; -} - /* * get_tablegroup_name - given a tablegroup OID, look up the name * @@ -320,7 +267,7 @@ get_tablegroup_name(Oid grp_oid) } /* - * Search pg_tablegroup using a cache lookup. + * Search pg_yb_tablegroup using a cache lookup. */ tuple = SearchSysCache1(YBTABLEGROUPOID, ObjectIdGetDatum(grp_oid)); @@ -382,7 +329,7 @@ RemoveTablegroupById(Oid grp_oid) InvokeObjectDropHook(YbTablegroupRelationId, grp_oid, 0); /* - * Remove the pg_tablegroup tuple + * Remove the pg_yb_tablegroup tuple */ CatalogTupleDelete(pg_tblgrp_rel, tuple); @@ -393,7 +340,7 @@ RemoveTablegroupById(Oid grp_oid) YBCDropTablegroup(grp_oid); } - /* We keep the lock on pg_tablegroup until commit */ + /* We keep the lock on pg_yb_tablegroup until commit */ heap_close(pg_tblgrp_rel, NoLock); } @@ -467,7 +414,7 @@ RenameTablegroup(const char *oldname, const char *newname) heap_freetuple(newtup); /* - * Close pg_tablegroup, but keep lock till commit. + * Close pg_yb_tablegroup, but keep lock till commit. */ heap_close(rel, NoLock); @@ -525,7 +472,7 @@ AlterTablegroupOwner(const char *grpname, Oid newOwnerId) HeapTuple newtuple; /* Otherwise, must be owner of the existing object or a superuser */ - if (!superuser() && !pg_tablegroup_ownercheck(HeapTupleGetOid(tuple), GetUserId())) + if (!superuser() && !pg_tablegroup_ownercheck(tablegroupoid, GetUserId())) aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_YBTABLEGROUP, grpname); /* Must be able to become new owner */ @@ -568,7 +515,7 @@ AlterTablegroupOwner(const char *grpname, Oid newOwnerId) heap_endscan(scandesc); - /* Close pg_tablegroup, but keep lock till commit */ + /* Close pg_yb_tablegroup, but keep lock till commit */ heap_close(rel, NoLock); return address; diff --git a/src/postgres/src/backend/nodes/copyfuncs.c b/src/postgres/src/backend/nodes/copyfuncs.c index e9e202f5472e..51bfba93d8d8 100644 --- a/src/postgres/src/backend/nodes/copyfuncs.c +++ b/src/postgres/src/backend/nodes/copyfuncs.c @@ -3350,7 +3350,6 @@ _copyOptTableGroup(const OptTableGroup *from) { OptTableGroup *newnode = makeNode(OptTableGroup); - COPY_SCALAR_FIELD(has_tablegroup); COPY_STRING_FIELD(tablegroup_name); return newnode; diff --git a/src/postgres/src/backend/nodes/equalfuncs.c b/src/postgres/src/backend/nodes/equalfuncs.c index f654868c1d9e..893fc5a232fd 100644 --- a/src/postgres/src/backend/nodes/equalfuncs.c +++ b/src/postgres/src/backend/nodes/equalfuncs.c @@ -1800,7 +1800,6 @@ static bool _equalOptTableGroup(const OptTableGroup *a, const OptTableGroup *b) { COMPARE_STRING_FIELD(tablegroup_name); - COMPARE_SCALAR_FIELD(has_tablegroup); return true; } diff --git a/src/postgres/src/backend/parser/gram.y b/src/postgres/src/backend/parser/gram.y index 7ff88c3566b5..3a6368a5fabc 100644 --- a/src/postgres/src/backend/parser/gram.y +++ b/src/postgres/src/backend/parser/gram.y @@ -4304,7 +4304,6 @@ OptTableGroup: { parser_ybc_beta_feature(@1, "tablegroup", true); $$ = makeNode(OptTableGroup); - $$->has_tablegroup = true; $$->tablegroup_name = $2; } | /*EMPTY*/ diff --git a/src/postgres/src/backend/parser/parse_utilcmd.c b/src/postgres/src/backend/parser/parse_utilcmd.c index e9cb8c6bd704..6637ed056384 100644 --- a/src/postgres/src/backend/parser/parse_utilcmd.c +++ b/src/postgres/src/backend/parser/parse_utilcmd.c @@ -387,12 +387,6 @@ transformCreateStmt(CreateStmt *stmt, const char *queryString) (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("users cannot create system catalog tables"))); } - else if (strcmp(def->defname, "tablegroup_oid") == 0) - { - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("cannot supply tablegroup_oid through WITH clause"))); - } else if (strcmp(def->defname, "colocated") == 0) (void) defGetBoolean(def); else if (strcmp(def->defname, "table_oid") == 0) @@ -2855,21 +2849,7 @@ transformIndexStmt(Oid relid, IndexStmt *stmt, const char *queryString) foreach(cell, stmt->options) { DefElem *def = (DefElem*) lfirst(cell); - if (strcmp(def->defname, "tablegroup_oid") == 0) - { - /* - * We must ensure that no tablegroup option was supplied in the - * WITH clause. - * Tablegroups cannot be supplied directly for indexes. We check - * here instead of ybccmds as we supply the reloption for the - * tablegroup of the indexed table in DefineIndex(.) - * if one exists. - */ - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("cannot supply tablegroup_oid through WITH clause"))); - } - else if (strcmp(def->defname, "table_oid") == 0) + if (strcmp(def->defname, "table_oid") == 0) { if (!(yb_enable_create_with_table_oid || IsYsqlUpgrade)) { diff --git a/src/postgres/src/backend/utils/adt/ruleutils.c b/src/postgres/src/backend/utils/adt/ruleutils.c index 4c433f4df1db..221fc3ada4e8 100644 --- a/src/postgres/src/backend/utils/adt/ruleutils.c +++ b/src/postgres/src/backend/utils/adt/ruleutils.c @@ -1541,13 +1541,15 @@ pg_get_indexdef_worker(Oid indexrelid, int colno, } Relation indrel = heap_open(indrelid, AccessShareLock); + YbLoadTablePropertiesIfNeeded(indrel, false /* allow_missing */); /* * If the indexed table's tablegroup mismatches that of an * index table, this is a leftover from beta days of tablegroup * feature. We cannot replicate this via DDL statement anymore. */ - if (indexrel->yb_table_properties->tablegroup_oid != RelationGetTablegroupOid(indrel)) + if (indexrel->yb_table_properties->tablegroup_oid != + indrel->yb_table_properties->tablegroup_oid) { ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), diff --git a/src/postgres/src/backend/utils/misc/pg_yb_utils.c b/src/postgres/src/backend/utils/misc/pg_yb_utils.c index 1c15dea47734..6eac3b785153 100644 --- a/src/postgres/src/backend/utils/misc/pg_yb_utils.c +++ b/src/postgres/src/backend/utils/misc/pg_yb_utils.c @@ -1639,9 +1639,7 @@ YbLoadTablePropertiesIfNeeded(Relation rel, bool allow_missing) rel->yb_table_properties = palloc0(sizeof(YbTablePropertiesData)); HandleYBStatus(YBCPgGetTableDesc(dbid, storage_relid, &desc)); - HandleYBStatus(YBCPgGetSomeTableProperties(desc, rel->yb_table_properties)); - - rel->yb_table_properties->tablegroup_oid = RelationGetTablegroupOid(rel); + HandleYBStatus(YBCPgGetTableProperties(desc, rel->yb_table_properties)); MemoryContextSwitchTo(oldcxt); } @@ -1782,7 +1780,7 @@ yb_table_properties(PG_FUNCTION_ARGS) if (ncols >= 5) { values[3] = - OidIsValid(rel->yb_table_properties->colocation_id) + OidIsValid(rel->yb_table_properties->tablegroup_oid) ? ObjectIdGetDatum(rel->yb_table_properties->tablegroup_oid) : (Datum) 0; values[4] = diff --git a/src/postgres/src/bin/pg_dump/pg_dump.c b/src/postgres/src/bin/pg_dump/pg_dump.c index b267aa8e5c37..47dc3a3028fe 100644 --- a/src/postgres/src/bin/pg_dump/pg_dump.c +++ b/src/postgres/src/bin/pg_dump/pg_dump.c @@ -288,7 +288,6 @@ static void binary_upgrade_extension_member(PQExpBuffer upgrade_buffer, static const char *getAttrName(int attrnum, TableInfo *tblInfo); static const char *fmtCopyColumnList(const TableInfo *ti, PQExpBuffer buffer); static bool nonemptyReloptions(const char *reloptions); -static bool YbHasReloptionsToInclude(const char *reloptions); static void YbAppendReloptions2(PQExpBuffer buffer, bool newline_before, const char *reloptions1, const char *reloptions1_prefix, const char *reloptions2, const char *reloptions2_prefix, @@ -18832,56 +18831,6 @@ nonemptyReloptions(const char *reloptions) return (reloptions != NULL && strlen(reloptions) > 2); } -/* - * Check if a reloptions array is nonempty and has any options - * except for the excluded ones. - */ -static bool -YbHasReloptionsToInclude(const char *reloptions) -{ - if (!nonemptyReloptions(reloptions)) - return false; - - char **options; - int noptions; - - if (!parsePGArray(reloptions, &options, &noptions)) - { - if (options) - free(options); - return false; - } - - bool has_included_values = false; - - for (int i = 0; i < noptions && !has_included_values; i++) - { - char *option = options[i]; - - /* - * Each array element should have the form name=value. If the "=" is - * missing for some reason, treat it like an empty value. - */ - char *name = option; - char *separator = strchr(option, '='); - if (separator) - *separator = '\0'; - - /* - * We ignore the reloption for tablegroup_oid. - * It is appended seperately as a TABLEGROUP clause. - */ - if (strcmp(name, "tablegroup_oid") != 0) - has_included_values = true; - } - - if (options) - free(options); - - return has_included_values; -} - - static void YbAppendReloptions2(PQExpBuffer buffer, bool newline_before, const char *reloptions1, const char *reloptions1_prefix, @@ -18907,14 +18856,14 @@ YbAppendReloptions3(PQExpBuffer buffer, bool newline_before, const char *with = newline_before ? "\nWITH (" : " WITH ("; - if (YbHasReloptionsToInclude(reloptions1)) + if (nonemptyReloptions(reloptions1)) { appendPQExpBufferStr(buffer, with); appendReloptionsArrayAH(buffer, reloptions1, reloptions1_prefix, fout); addwith = false; addcomma = true; } - if (YbHasReloptionsToInclude(reloptions2)) + if (nonemptyReloptions(reloptions2)) { if (addwith) appendPQExpBufferStr(buffer, with); @@ -18924,7 +18873,7 @@ YbAppendReloptions3(PQExpBuffer buffer, bool newline_before, addwith = false; addcomma = true; } - if (YbHasReloptionsToInclude(reloptions3)) + if (nonemptyReloptions(reloptions3)) { if (addwith) appendPQExpBufferStr(buffer, with); diff --git a/src/postgres/src/bin/psql/describe.c b/src/postgres/src/bin/psql/describe.c index fe50cf0f7317..98cbd7317a26 100644 --- a/src/postgres/src/bin/psql/describe.c +++ b/src/postgres/src/bin/psql/describe.c @@ -35,7 +35,7 @@ static bool describeOneTableDetails(const char *schemaname, static void add_tablespace_footer(printTableContent *const cont, char relkind, Oid tablespace, const bool newline); static void add_tablegroup_footer(printTableContent *const cont, char relkind, - Oid tablegroup, const bool newline); + const char* grpname, const bool newline); static void add_role_attribute(PQExpBuffer buf, const char *const str); static bool listTSParsersVerbose(const char *pattern); static bool describeOneTSParser(const char *oid, const char *nspname, @@ -1490,7 +1490,8 @@ describeOneTableDetails(const char *schemaname, char *reloftype; char relpersistence; char relreplident; - Oid tablegroup; + + char *yb_grpname; } tableinfo; bool show_column_details = false; @@ -1653,18 +1654,20 @@ describeOneTableDetails(const char *schemaname, /* Get information about tablegroup (if any) */ printfPQExpBuffer(&tablegroupbuf, - "SELECT SUBSTRING(unnest(reloptions) from '.*tablegroup_oid=(\\d*).*') AS tablegroup\n" - "FROM pg_catalog.pg_class WHERE oid = '%s';", + "SELECT grpname\n" + "FROM pg_catalog.yb_table_properties(%s) p\n" + "LEFT JOIN pg_catalog.pg_yb_tablegroup gr\n" + " ON gr.oid = p.tablegroup_oid;", oid); tgres = PSQLexec(tablegroupbuf.data); - if (tgres && PQntuples(tgres) > 0) + if (tgres && PQntuples(tgres) > 0 && strcmp(PQgetvalue(tgres, 0, 0), "") != 0) { - tableinfo.tablegroup = atooid(PQgetvalue(tgres, 0, 0)); + tableinfo.yb_grpname = pg_strdup(PQgetvalue(tgres, 0, 0)); } else - tableinfo.tablegroup = 0; + tableinfo.yb_grpname = NULL; PQclear(tgres); tgres = NULL; @@ -2231,7 +2234,7 @@ describeOneTableDetails(const char *schemaname, add_tablespace_footer(&cont, tableinfo.relkind, tableinfo.tablespace, true); add_tablegroup_footer(&cont, tableinfo.relkind, - tableinfo.tablegroup, true); + tableinfo.yb_grpname, true); } PQclear(result); @@ -2350,24 +2353,28 @@ describeOneTableDetails(const char *schemaname, /* Get information about tablegroup (if any) */ printfPQExpBuffer(&tablegroupbuf, - "SELECT SUBSTRING(unnest(reloptions) from '.*tablegroup_oid=(\\d*).*') AS tablegroup\n" - "FROM pg_catalog.pg_class WHERE relname = '%s';", + "SELECT tg.grpname\n" + "FROM yb_table_properties('%s.%s'::regclass) props,\n" + " pg_yb_tablegroup tg\n" + "WHERE tg.oid = props.tablegroup_oid;", + schemaname, PQgetvalue(result, i, 0)); tgres = PSQLexec(tablegroupbuf.data); - Oid idx_tablegroup; - if (tgres && PQntuples(tgres) > 0) + char* idx_grpname; + if (tgres && PQntuples(tgres) > 0 && + strcmp(PQgetvalue(tgres, 0, 0), "") != 0) { - idx_tablegroup = atooid(PQgetvalue(tgres, 0, 0)); + idx_grpname = pg_strdup(PQgetvalue(tgres, 0, 0)); } else - idx_tablegroup = 0; + idx_grpname = NULL; PQclear(tgres); tgres = NULL; /* Print tablegroup of the index on the same line */ - add_tablegroup_footer(&cont, RELKIND_INDEX, idx_tablegroup, false); + add_tablegroup_footer(&cont, RELKIND_INDEX, idx_grpname, false); } } PQclear(result); @@ -3167,8 +3174,8 @@ describeOneTableDetails(const char *schemaname, /* Tablespace info */ add_tablespace_footer(&cont, tableinfo.relkind, tableinfo.tablespace, true); - /* Tablespace info */ - add_tablegroup_footer(&cont, tableinfo.relkind, tableinfo.tablegroup, + /* Tablegroup info */ + add_tablegroup_footer(&cont, tableinfo.relkind, tableinfo.yb_grpname, true); } @@ -3276,55 +3283,38 @@ add_tablespace_footer(printTableContent *const cont, char relkind, */ static void add_tablegroup_footer(printTableContent *const cont, char relkind, - Oid tablegroup, const bool newline) + const char* grpname, const bool newline) { /* relkinds for which we support tablegroups */ - if (relkind == RELKIND_RELATION || - relkind == RELKIND_INDEX) + if ((relkind == RELKIND_RELATION || + relkind == RELKIND_INDEX || + relkind == RELKIND_PARTITIONED_TABLE || + relkind == RELKIND_PARTITIONED_INDEX) && + grpname) { - // Ignore InvalidOid - if (tablegroup != 0) + PQExpBufferData buf; + initPQExpBuffer(&buf); + if (newline) { - PGresult *result = NULL; - PQExpBufferData buf; - - initPQExpBuffer(&buf); - printfPQExpBuffer(&buf, - "SELECT grpname FROM pg_catalog.pg_yb_tablegroup\n" - "WHERE oid = '%u';", tablegroup); - result = PSQLexec(buf.data); - if (!result) - { - termPQExpBuffer(&buf); - return; - } - /* Should always be the case, but.... */ - if (PQntuples(result) > 0) - { - if (newline) - { - /* Add the tablegroup as a new footer */ - printfPQExpBuffer(&buf, _("Tablegroup: \"%s\""), - PQgetvalue(result, 0, 0)); - printTableAddFooter(cont, buf.data); - } - else - { - /* Append the tablegroup to the latest footer */ - printfPQExpBuffer(&buf, "%s", cont->footer->data); + /* Add the tablegroup as a new footer */ + printfPQExpBuffer(&buf, _("Tablegroup: \"%s\""), + grpname); + printTableAddFooter(cont, buf.data); + } + else + { + /* Append the tablegroup to the latest footer */ + printfPQExpBuffer(&buf, "%s", cont->footer->data); - /* - translator: before this string there's an index description like - '"foo_pkey" PRIMARY KEY, btree (a)' - */ - appendPQExpBuffer(&buf, _(", tablegroup \"%s\""), - PQgetvalue(result, 0, 0)); - printTableSetFooter(cont, buf.data); - } - } - PQclear(result); - termPQExpBuffer(&buf); + /* + translator: before this string there's an index description like + '"foo_pkey" PRIMARY KEY, btree (a)' + */ + appendPQExpBuffer(&buf, _(", tablegroup \"%s\""), + grpname); + printTableSetFooter(cont, buf.data); } + termPQExpBuffer(&buf); } } @@ -4434,10 +4424,12 @@ listTablegroups(const char *pattern, bool verbose, bool showRelations) if (showRelations) { appendPQExpBufferStr(&buf, - "\nJOIN (SELECT oid, relname, relkind, relowner, unnest(pg_catalog.pg_class.reloptions) " \ - "AS relopt FROM pg_catalog.pg_class) c"); - appendPQExpBufferStr(&buf, - "\nON c.relopt LIKE CONCAT('%tablegroup_oid=%', CAST(g.oid AS TEXT), '%')\n"); + "INNER JOIN (\n" + " SELECT cl.oid, cl.relname, cl.relkind, cl.relowner,\n" + " props.tablegroup_oid\n" + " FROM pg_catalog.pg_class cl, yb_table_properties(cl.oid) props\n" + ") c\n" + "ON c.tablegroup_oid = g.oid\n"); } processSQLNamePattern(pset.db, &buf, pattern, false, false, diff --git a/src/postgres/src/fe_utils/string_utils.c b/src/postgres/src/fe_utils/string_utils.c index 136dac1049f8..d9b54a8b7446 100644 --- a/src/postgres/src/fe_utils/string_utils.c +++ b/src/postgres/src/fe_utils/string_utils.c @@ -779,13 +779,6 @@ appendReloptionsArray(PQExpBuffer buffer, const char *reloptions, else value = ""; - /* - * We ignore the reloption for tablegroup_oid. - * It is appended seperately as a TABLEGROUP clause. - */ - if (strcmp(name, "tablegroup_oid") == 0) - continue; - if (appended) appendPQExpBufferStr(buffer, ", "); appendPQExpBuffer(buffer, "%s%s=", prefix, fmtId(name)); diff --git a/src/postgres/src/include/catalog/dependency.h b/src/postgres/src/include/catalog/dependency.h index 1cc41b19def4..52fcac234176 100644 --- a/src/postgres/src/include/catalog/dependency.h +++ b/src/postgres/src/include/catalog/dependency.h @@ -177,7 +177,7 @@ typedef enum ObjectClass OCLASS_TSCONFIG, /* pg_ts_config */ OCLASS_ROLE, /* pg_authid */ OCLASS_DATABASE, /* pg_database */ - OCLASS_TBLGROUP, /* pg_tablegroup */ + OCLASS_TBLGROUP, /* pg_yb_tablegroup */ OCLASS_TBLSPACE, /* pg_tablespace */ OCLASS_FDW, /* pg_foreign_data_wrapper */ OCLASS_FOREIGN_SERVER, /* pg_foreign_server */ diff --git a/src/postgres/src/include/commands/tablegroup.h b/src/postgres/src/include/commands/tablegroup.h index 4f5d0c89eb54..94f42ebe75e8 100644 --- a/src/postgres/src/include/commands/tablegroup.h +++ b/src/postgres/src/include/commands/tablegroup.h @@ -43,7 +43,6 @@ extern Oid CreateTableGroup(CreateTableGroupStmt *stmt); extern Oid get_tablegroup_oid(const char *tablegroupname, bool missing_ok); -extern Oid get_tablegroup_oid_by_table_oid(Oid table_oid); extern char *get_tablegroup_name(Oid grp_oid); extern void RemoveTablegroupById(Oid grp_oid); diff --git a/src/postgres/src/include/nodes/parsenodes.h b/src/postgres/src/include/nodes/parsenodes.h index e9b5cc57314e..94326c79660c 100644 --- a/src/postgres/src/include/nodes/parsenodes.h +++ b/src/postgres/src/include/nodes/parsenodes.h @@ -2205,8 +2205,6 @@ typedef struct CreateTableGroupStmt typedef struct OptTableGroup { NodeTag type; - - bool has_tablegroup; char *tablegroup_name; } OptTableGroup; diff --git a/src/postgres/src/include/utils/rel.h b/src/postgres/src/include/utils/rel.h index 283a28f8deb6..060c18c26a2a 100644 --- a/src/postgres/src/include/utils/rel.h +++ b/src/postgres/src/include/utils/rel.h @@ -330,15 +330,6 @@ typedef struct StdRdOptions ((relation)->rd_options ? \ ((StdRdOptions *) (relation)->rd_options)->parallel_workers : (defaultpw)) -/* - * RelationGetTablegroupOid - * Returns the relation's tablegroup reloption setting. - * Note multiple eval of argument! - */ -#define RelationGetTablegroupOid(relation) \ - ((relation)->rd_options ? \ - ((StdRdOptions *) (relation)->rd_options)->tablegroup_oid : 0) - /* * RelationGetColocated * Returns the relation's colocated reloption setting. diff --git a/src/postgres/src/test/regress/expected/yb_create_index.out b/src/postgres/src/test/regress/expected/yb_create_index.out index d3be742a4c61..c2bdb0a4d64f 100644 --- a/src/postgres/src/test/regress/expected/yb_create_index.out +++ b/src/postgres/src/test/regress/expected/yb_create_index.out @@ -825,7 +825,7 @@ CREATE INDEX idx2_tbl_group_tbl ON tbl_group_tbl (r1 NULLS LAST); v1 | integer | | | v2 | integer | | | Indexes: - "tbl_group_tbl_pkey" PRIMARY KEY, lsm (r1 ASC, r2 ASC), tablegroup "tbl_group" + "tbl_group_tbl_pkey" PRIMARY KEY, lsm (r1 ASC, r2 ASC) "idx2_tbl_group_tbl" lsm (r1 ASC), tablegroup "tbl_group" "idx_tbl_group_tbl" lsm (r1 ASC NULLS FIRST), tablegroup "tbl_group" Tablegroup: "tbl_group" diff --git a/src/postgres/src/test/regress/expected/yb_tablegroup.out b/src/postgres/src/test/regress/expected/yb_tablegroup.out index dd4706b9447b..693a25ddd9c8 100644 --- a/src/postgres/src/test/regress/expected/yb_tablegroup.out +++ b/src/postgres/src/test/regress/expected/yb_tablegroup.out @@ -167,22 +167,16 @@ SELECT relname FROM pg_class WHERE relname LIKE 'bob%'; (0 rows) -- --- Usage of WITH tablegroup_oid for CREATE TABLE/INDEX. These all fail. +-- Usage of WITH tablegroup_oid reloption, this legacy syntax no longer works. -- CREATE TABLE tgroup_with1 (col1 int, col2 int) WITH (tablegroup_oid=16385); -ERROR: cannot supply tablegroup_oid through WITH clause -CREATE TABLE tgroup_with2 (col1 int, col2 int) WITH (tablegroup_oid=16385, colocated=true); -ERROR: cannot supply tablegroup_oid through WITH clause -CREATE TABLE tgroup_with2 (col1 int, col2 int) WITH (tablegroup_oid=16385, colocated=false); -ERROR: cannot supply tablegroup_oid through WITH clause +WARNING: storage parameter tablegroup_oid is unsupported, ignoring +ERROR: unrecognized parameter "tablegroup_oid" CREATE TABLE tgroup_with3 (col1 int, col2 int) WITH (tablegroup_oid=16385) TABLEGROUP tgroup1; -ERROR: cannot supply tablegroup_oid through WITH clause -CREATE TABLE tgroup_with4 (col1 int, col2 int) WITH (tablegroup_oid=123); -ERROR: cannot supply tablegroup_oid through WITH clause +WARNING: storage parameter tablegroup_oid is unsupported, ignoring +ERROR: unrecognized parameter "tablegroup_oid" CREATE INDEX ON tgroup_test1(col1) WITH (tablegroup_oid=123); -ERROR: cannot supply tablegroup_oid through WITH clause -CREATE INDEX ON tgroup_test1(col1) WITH (tablegroup_oid=123, colocated=true); -ERROR: cannot supply tablegroup_oid through WITH clause +ERROR: unrecognized parameter "tablegroup_oid" -- -- Usage of SPLIT clause with TABLEGROUP should fail -- diff --git a/src/postgres/src/test/regress/expected/yb_tablegroup_colocation_id.out b/src/postgres/src/test/regress/expected/yb_tablegroup_colocation_id.out index b9a3565e8f37..a9fad83273c3 100644 --- a/src/postgres/src/test/regress/expected/yb_tablegroup_colocation_id.out +++ b/src/postgres/src/test/regress/expected/yb_tablegroup_colocation_id.out @@ -31,7 +31,7 @@ INSERT INTO tg2_table2 VALUES ('220t', 120), ('221t', 121), ('222t', v2 | integer | | | v3 | text | | | Indexes: - "tg1_table3_pkey" PRIMARY KEY, lsm (v1 ASC), tablegroup "tg1" + "tg1_table3_pkey" PRIMARY KEY, lsm (v1 ASC) Tablegroup: "tg1" \d tg2_table1 @@ -40,7 +40,7 @@ Tablegroup: "tg1" --------+------+-----------+----------+--------- v1 | text | | not null | Indexes: - "tg2_table1_pkey" PRIMARY KEY, lsm (v1 ASC), tablegroup "tg2" + "tg2_table1_pkey" PRIMARY KEY, lsm (v1 ASC) Tablegroup: "tg2" SELECT * FROM table_props; diff --git a/src/postgres/src/test/regress/sql/yb_tablegroup.sql b/src/postgres/src/test/regress/sql/yb_tablegroup.sql index f7d43841ae38..485f806e1dde 100644 --- a/src/postgres/src/test/regress/sql/yb_tablegroup.sql +++ b/src/postgres/src/test/regress/sql/yb_tablegroup.sql @@ -89,17 +89,13 @@ DROP USER alice; -- we dropped all of alice's entities, so we can r SELECT relname FROM pg_class WHERE relname LIKE 'bob%'; -- --- Usage of WITH tablegroup_oid for CREATE TABLE/INDEX. These all fail. +-- Usage of WITH tablegroup_oid reloption, this legacy syntax no longer works. -- CREATE TABLE tgroup_with1 (col1 int, col2 int) WITH (tablegroup_oid=16385); -CREATE TABLE tgroup_with2 (col1 int, col2 int) WITH (tablegroup_oid=16385, colocated=true); -CREATE TABLE tgroup_with2 (col1 int, col2 int) WITH (tablegroup_oid=16385, colocated=false); CREATE TABLE tgroup_with3 (col1 int, col2 int) WITH (tablegroup_oid=16385) TABLEGROUP tgroup1; -CREATE TABLE tgroup_with4 (col1 int, col2 int) WITH (tablegroup_oid=123); CREATE INDEX ON tgroup_test1(col1) WITH (tablegroup_oid=123); -CREATE INDEX ON tgroup_test1(col1) WITH (tablegroup_oid=123, colocated=true); -- -- Usage of SPLIT clause with TABLEGROUP should fail diff --git a/src/yb/master/catalog_manager.cc b/src/yb/master/catalog_manager.cc index 1c09eb633cdc..58cd1bab1c1d 100644 --- a/src/yb/master/catalog_manager.cc +++ b/src/yb/master/catalog_manager.cc @@ -6181,6 +6181,13 @@ Status CatalogManager::GetTableSchemaInternal(const GetTableSchemaRequestPB* req resp->set_colocated(table->colocated()); + if (table->IsColocatedUserTable()) { + auto* tablegroup = tablegroup_manager_->FindByTable(table->id()); + if (tablegroup) { + resp->set_tablegroup_id(tablegroup->id()); + } + } + VLOG(1) << "Serviced GetTableSchema request for " << req->ShortDebugString() << " with " << yb::ToString(*resp); return Status::OK(); diff --git a/src/yb/master/master_ddl.proto b/src/yb/master/master_ddl.proto index 48e73b022c77..0478e2a7ec0f 100644 --- a/src/yb/master/master_ddl.proto +++ b/src/yb/master/master_ddl.proto @@ -205,6 +205,8 @@ message GetTableSchemaResponsePB { // True if table is colocated (including tablegroups, excluding YSQL system tables). optional bool colocated = 13; + optional bytes tablegroup_id = 16; + optional bool is_compatible_with_previous_version = 14 [default = false]; optional uint32 wal_retention_secs = 15; diff --git a/src/yb/yql/pggate/pg_tabledesc.cc b/src/yb/yql/pggate/pg_tabledesc.cc index 860c135a22cc..d55345feab88 100644 --- a/src/yb/yql/pggate/pg_tabledesc.cc +++ b/src/yb/yql/pggate/pg_tabledesc.cc @@ -86,6 +86,10 @@ YBCPgOid PgTableDesc::GetColocationId() const { return schema().has_colocation_id() ? schema().colocation_id() : kColocationIdNotSet; } +const TablegroupId PgTableDesc::GetTablegroupId() const { + return resp_.has_tablegroup_id() ? resp_.tablegroup_id() : ""; +} + bool PgTableDesc::IsHashPartitioned() const { return schema().num_hash_key_columns() > 0; } diff --git a/src/yb/yql/pggate/pg_tabledesc.h b/src/yb/yql/pggate/pg_tabledesc.h index d91d3cc09a0e..63bfac9f402f 100644 --- a/src/yb/yql/pggate/pg_tabledesc.h +++ b/src/yb/yql/pggate/pg_tabledesc.h @@ -91,6 +91,9 @@ class PgTableDesc : public RefCountedThreadSafe { YBCPgOid GetColocationId() const; + // Empty if no tablegroup. + const TablegroupId GetTablegroupId() const; + uint32_t schema_version() const; private: diff --git a/src/yb/yql/pggate/ybc_pg_typedefs.h b/src/yb/yql/pggate/ybc_pg_typedefs.h index 6c2585a854f2..912b0b969431 100644 --- a/src/yb/yql/pggate/ybc_pg_typedefs.h +++ b/src/yb/yql/pggate/ybc_pg_typedefs.h @@ -352,7 +352,7 @@ typedef struct YbTablePropertiesData { uint64_t num_tablets; uint64_t num_hash_key_columns; bool is_colocated; /* via database or tablegroup, but not for system tables */ - YBCPgOid tablegroup_oid; /* 0 if none */ + YBCPgOid tablegroup_oid; /* InvalidOid if none */ YBCPgOid colocation_id; /* 0 if not colocated */ } YbTablePropertiesData; diff --git a/src/yb/yql/pggate/ybc_pggate.cc b/src/yb/yql/pggate/ybc_pggate.cc index 912310021ceb..6f26c47fc5d2 100644 --- a/src/yb/yql/pggate/ybc_pggate.cc +++ b/src/yb/yql/pggate/ybc_pggate.cc @@ -499,12 +499,21 @@ YBCStatus YBCPgExecTruncateTable(YBCPgStatement handle) { return ToYBCStatus(pgapi->ExecTruncateTable(handle)); } -YBCStatus YBCPgGetSomeTableProperties(YBCPgTableDesc table_desc, - YbTableProperties properties) { +YBCStatus YBCPgGetTableProperties(YBCPgTableDesc table_desc, + YbTableProperties properties) { CHECK_NOTNULL(properties)->num_tablets = table_desc->GetPartitionCount(); + TablegroupId tablegroup_id = table_desc->GetTablegroupId(); + YBCPgOid tablegroup_oid = 0; + if (!tablegroup_id.empty()) { + YBCStatus status = + ExtractValueFromResult(GetPgsqlTablegroupOid(tablegroup_id), &tablegroup_oid); + if (status) { + return status; + } + } properties->num_hash_key_columns = table_desc->num_hash_key_columns(); properties->is_colocated = table_desc->IsColocated(); - properties->tablegroup_oid = 0; /* Isn't set here. */ + properties->tablegroup_oid = tablegroup_oid; properties->colocation_id = table_desc->GetColocationId(); return YBCStatusOK(); } diff --git a/src/yb/yql/pggate/ybc_pggate.h b/src/yb/yql/pggate/ybc_pggate.h index 009b5930d664..cb97c82a0bfa 100644 --- a/src/yb/yql/pggate/ybc_pggate.h +++ b/src/yb/yql/pggate/ybc_pggate.h @@ -230,10 +230,9 @@ YBCStatus YBCPgGetColumnInfo(YBCPgTableDesc table_desc, int16_t attr_number, YBCPgColumnInfo *column_info); -// Does not set tablegroup_oid. // Callers should probably use YbLoadTablePropertiesIfNeeded instead. -YBCStatus YBCPgGetSomeTableProperties(YBCPgTableDesc table_desc, - YbTableProperties properties); +YBCStatus YBCPgGetTableProperties(YBCPgTableDesc table_desc, + YbTableProperties properties); YBCStatus YBCPgDmlModifiesRow(YBCPgStatement handle, bool *modifies_row);