8000 Simplify partitioned table creation vs. relcache · postgres/postgres@2fbdf1b · GitHub
[go: up one dir, main page]
Skip to content
{"payload":{"commit":{"oid":"2fbdf1b38bc54b297e0f885ca97e0c8f5c922e72","url":"/postgres/postgres/commit/2fbdf1b38bc54b297e0f885ca97e0c8f5c922e72","authoredDate":"2018-09-05T14:36:13.000-03:00","committedDate":"2018-09-05T14:36:13.000-03:00","shortMessage":null,"shortMessageMarkdown":"\u003cdiv\u003eSimplify partitioned table creation vs. relcache\u003c/div\u003e","shortMessageMarkdownLink":null,"bodyMessageHtml":"In the original code, we were storing the pg_inherits row for a\npartitioned table too early: enough that we had a hack for relcache to\navoid falling flat on its face while reading such a partial entry. If\nwe finish the pg_class creation first and *then* store the pg_inherits\nentry, we don't need that hack.\n\nAlso recognize that pg_class.relpartbound is not marked NOT NULL and\ntherefore it's entirely possible to read null values, so having only\nAssert() protection isn't enough. Change those to if/elog tests\ninstead. This qualifies as a robustness fix, so backpatch to pg11.\n\nIn passing, remove one access that wasn't actually needed, and reword\none message to be like all the others that check for the same thing.\n\nReviewed-by: Amit Langote\nDiscussion: \u003ca href=\"https://postgr.es/m/20180903213916.hh6wasnrdg6xv2ud@alvherre.pgsql\" rel=\"nofollow\"\u003ehttps://postgr.es/m/20180903213916.hh6wasnrdg6xv2ud@alvherre.pgsql\u003c/a\u003e","authors":[{"login":"alvherre","displayName":"Alvaro Herrera","avatarUrl":"https://avatars.githubusercontent.com/u/340005?v=4","path":"/alvherre","isGitHub":false}],"committerAttribution":false,"committer":{"login":"alvherre","displayName":"Alvaro Herrera","avatarUrl":"https://avatars.githubusercontent.com/u/340005?v=4","path":"/alvherre","isGitHub":false},"parents":["f5a6509bb1ec5222a707205941a40f280f3e6e15"],"globalRelayId":"MDY6Q29tbWl0OTI3NDQyOjJmYmRmMWIzOGJjNTRiMjk3ZTBmODg1Y2E5N2UwYzhmNWM5MjJlNzI=","sha1":"f5a6509bb1ec5222a707205941a40f280f3e6e15","sha2":"2fbdf1b38bc54b297e0f885ca97e0c8f5c922e72"},"currentUser":null,"repo":{"id":927442,"defaultBranch":"master","name":"postgres","ownerLogin":"postgres","currentUserCanPush":false,"isFork":false,"isEmpty":false,"createdAt":"2010-09-21T11:35:45.000Z","ownerAvatar":"https://avatars.githubusercontent.com/u/177543?v=4","public":true,"private":false,"isOrgOwned":true},"diffEntryData":[{"diffLines":[{"stylingDirective":null,"type":"HUNK","blobLineNumber":772,"text":"@@ -773,9 +773,6 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,","html":"@@ -773,9 +773,6 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,","displayNoNewLineWarning":false,"position":0,"left":772,"right":772},{"stylingDirective":null,"type":"CONTEXT","blobLineNumber":773,"text":" \t\t\t\t\t\t\t\t\t\t InvalidOid,","html":" \t\t\t\t\t\t\t\t\t\t \u003cspan class=pl-s1\u003eInvalidOid\u003c/span\u003e,","displayNoNewLineWarning":false,"position":1,"left":773,"right":773},{"stylingDirective":null,"type":"CONTEXT","blobLineNumber":774,"text":" \t\t\t\t\t\t\t\t\t\t typaddress);","html":" \t\t\t\t\t\t\t\t\t\t \u003cspan class=pl-s1\u003etypaddress\u003c/span\u003e);","displayNoNewLineWarning":false,"position":2,"left":774,"right":774},{"stylingDirective":null,"type":"CONTEXT","blobLineNumber":775,"text":" ","html":"\u003cbr\u003e","displayNoNewLineWarning":false,"position":3,"left":775,"right":775},{"stylingDirective":null,"type":"DELETION","blobLineNumber":776,"text":"-\t/* Store inheritance information for new rel. */","html":"-\t\u003cspan class=pl-c\u003e/* Store inheritance information for new rel. */\u003c/span\u003e","displayNoNewLineWarning":false,"position":4,"left":776,"right":775},{"stylingDirective":null,"type":"DELETION","blobLineNumber":777,"text":"-\tStoreCatalogInheritance(relationId, inheritOids, stmt-\u003epartbound != NULL);","html":"-\t\u003cspan class=pl-en\u003eStoreCatalogInheritance\u003c/span\u003e(\u003cspan class=pl-s1\u003erelationId\u003c/span\u003e, \u003cspan class=pl-s1\u003einheritOids\u003c/span\u003e, \u003cspan class=pl-s1\u003estmt\u003c/span\u003e\u003cspan class=pl-c1\u003e-\u0026gt;\u003c/span\u003e\u003cspan class=pl-c1\u003epartbound\u003c/span\u003e \u003cspan class=pl-c1\u003e!=\u003c/span\u003e \u003cspan class=pl-c1\u003eNULL\u003c/span\u003e);","displayNoNewLineWarning":false,"position":5,"left":777,"right":775},{"stylingDirective":null,"type":"DELETION","blobLineNumber":778,"text":"-","html":"-","displayNoNewLineWarning":false,"position":6,"left":778,"right":775},{"stylingDirective":null,"type":"CONTEXT","blobLineNumber":776,"text":" \t/*","html":" \t\u003cspan class=pl-c\u003e/*\u003c/span\u003e","displayNoNewLineWarning":false,"position":7,"left":779,"right":776},{"stylingDirective":null,"type":"CONTEXT","blobLineNumber":777,"text":" \t * We must bump the command counter to make the newly-created relation","html":" \u003cspan class=pl-c\u003e\t * We must bump the command counter to make the newly-created relation\u003c/span\u003e","displayNoNewLineWarning":false,"position":8,"left":780,"right":777},{"stylingDirective":null,"type":"CONTEXT","blobLineNumber":778,"text":" \t * tuple visible for opening.","html":" \u003cspan class=pl-c\u003e\t * tuple visible for opening.\u003c/span\u003e","displayNoNewLineWarning":false,"position":9,"left":781,"right":778},{"stylingDirective":null,"type":"HUNK","blobLineNumber":865,"text":"@@ -869,6 +866,9 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,","html":"@@ -869,6 +866,9 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,","displayNoNewLineWarning":false,"position":10,"left":868,"right":865},{"stylingDirective":null,"type":"CONTEXT","blobLineNumber":866,"text":" \t\theap_close(parent, NoLock);","html":" \t\t\u003cspan class=pl-en\u003eheap_close\u003c/span\u003e(\u003cspan class=pl-s1\u003eparent\u003c/span\u003e, \u003cspan class=pl-s1\u003eNoLock\u003c/span\u003e);","displayNoNewLineWarning":false,"position":11,"left":869,"right":866},{"stylingDirective":null,"type":"CONTEXT","blobLineNumber":867,"text":" \t}","html":" \t}","displayNoNewLineWarning":false,"position":12,"left":870,"right":867},{"stylingDirective":null,"type":"CONTEXT","blobLineNumber":868,"text":" ","html":"\u003cbr\u003e","displayNoNewLineWarning":false,"position":13,"left":871,"right":868},{"stylingDirective":null,"type":"ADDITION","blobLineNumber":869,"text":"+\t/* Store inheritance information for new rel. */","html":"+\t\u003cspan class=pl-c\u003e/* Store inheritance information for new rel. */\u003c/span\u003e","displayNoNewLineWarning":false,"position":14,"left":871,"right":869},{"stylingDirective":null,"type":"ADDITION","blobLineNumber":870,"text":"+\tStoreCatalogInheritance(relationId, inheritOids, stmt-\u003epartbound != NULL);","html":"+\t\u003cspan class=pl-en\u003eStoreCatalogInheritance\u003c/span\u003e(\u003cspan class=pl-s1\u003erelationId\u003c/span\u003e, \u003cspan class=pl-s1\u003einheritOids\u003c/span\u003e, \u003cspan class=pl-s1\u003estmt\u003c/span\u003e\u003cspan class=pl-c1\u003e-\u0026gt;\u003c/span\u003e\u003cspan class=pl-c1\u003epartbound\u003c/span\u003e \u003cspan class=pl-c1\u003e!=\u003c/span\u003e \u003cspan class=pl-c1\u003eNULL\u003c/span\u003e);","displayNoNewLineWarning":false,"position":15,"left":871,"right":870},{"stylingDirective":null,"type":"ADDITION","blobLineNumber":871,"text":"+","html":"+","displayNoNewLineWarning":false,"position":16,"left":871,"right":871},{"stylingDirective":null,"type":"CONTEXT","blobLineNumber":872,"text":" \t/*","html":" \t\u003cspan class=pl-c\u003e/*\u003c/span\u003e","displayNoNewLineWarning":false,"position":17,"left":872,"right":872},{"stylingDirective":null,"type":"CONTEXT","blobLineNumber":873,"text":" \t * Process the partitioning specification (if any) and store the partition","html":" \u003cspan class=pl-c\u003e\t * Process the partitioning specification (if any) and store the partition\u003c/span\u003e","displayNoNewLineWarning":false,"position":18,"left":873,"right":873},{"stylingDirective":null,"type":"CONTEXT","blobLineNumber":874,"text":" \t * key information into the catalog.","html":" \u003cspan class=pl-c\u003e\t * key information into the catalog.\u003c/span\u003e","displayNoNewLineWarning":false,"position":19,"left":874,"right":874},{"stylingDirective":null,"type":"HUNK","blobLineNumber":14702,"text":"@@ -14703,10 +14703,6 @@ ATExecDetachPartition(Relation rel, RangeVar *name)","html":"@@ -14703,10 +14703,6 @@ ATExecDetachPartition(Relation rel, RangeVar *name)","displayNoNewLineWarning":false,"position":20,"left":14702,"right":14702},{"stylingDirective":null,"type":"CONTEXT","blobLineNumber":14703,"text":" \t\t\t RelationGetRelid(partRel));","html":" \t\t\t \u003cspan class=pl-en\u003eRelationGetRelid\u003c/span\u003e(\u003cspan class=pl-s1\u003epartRel\u003c/span\u003e));","displayNoNewLineWarning":false,"position":21,"left":14703,"right":14703},{"stylingDirective":null,"type":"CONTEXT","blobLineNumber":14704,"text":" \tAssert(((Form_pg_class) GETSTRUCT(tuple))-\u003erelispartition);","html":" \t\u003cspan class=pl-en\u003eAssert\u003c/span\u003e(((\u003cspan class=pl-smi\u003eForm_pg_class\u003c/span\u003e) \u003cspan class=pl-en\u003eGETSTRUCT\u003c/span\u003e(\u003cspan class=pl-s1\u003etuple\u003c/span\u003e))\u003cspan class=pl-c1\u003e-\u0026gt;\u003c/span\u003e\u003cspan class=pl-c1\u003erelispartition\u003c/span\u003e);","displayNoNewLineWarning":false,"position":22,"left":14704,"right":14704},{"stylingDirective":null,"type":"CONTEXT","blobLineNumber":14705,"text":" ","html":"\u003cbr\u003e","displayNoNewLineWarning":false,"position":23,"left":14705,"right":14705},{"stylingDirective":null,"type":"DELETION","blobLineNumber":14706,"text":"-\t(void) SysCacheGetAttr(RELOID, tuple, Anum_pg_class_relpartbound,","html":"-\t(\u003cspan class=pl-smi\u003evoid\u003c/span\u003e) \u003cspan class=pl-en\u003eSysCacheGetAttr\u003c/span\u003e(\u003cspan class=pl-c1\u003eRELOID\u003c/span\u003e, \u003cspan class=pl-s1\u003etuple\u003c/span\u003e, \u003cspan class=pl-s1\u003eAnum_pg_class_relpartbound\u003c/span\u003e,","displayNoNewLineWarning":false,"position":24,"left":14706,"right":14705},{"stylingDirective":null,"type":"DELETION","blobLineNumber":14707,"text":"-\t\t\t\t\t\t \u0026isnull);","html":"-\t\t\t\t\t\t \u003cspan class=pl-c1\u003e\u0026amp;\u003c/span\u003e\u003cspan class=pl-s1\u003eisnull\u003c/span\u003e);","displayNoNewLineWarning":false,"position":25,"left":14707,"right":14705},{"stylingDirective":null,"type":"DELETION","blobLineNumber":14708,"text":"-\tAssert(!isnull);","html":"-\t\u003cspan class=pl-en\u003eAssert\u003c/span\u003e(!\u003cspan class=pl-s1\u003eisnull\u003c/span\u003e);","displayNoNewLineWarning":false,"position":26,"left":14708,"right":14705},{"stylingDirective":null,"type":"DELETION","blobLineNumber":14709,"text":"-","html":"-","displayNoNewLineWarning":false,"position":27,"left":14709,"right":14705},{"stylingDirective":null,"type":"CONTEXT","blobLineNumber":14706,"text":" \t/* Clear relpartbound and reset relispartition */","html":" \t\u003cspan class=pl-c\u003e/* Clear relpartbound and reset relispartition */\u003c/span\u003e","displayNoNewLineWarning":false,"position":28,"left":14710,"right":14706},{"stylingDirective":null,"type":"CONTEXT","blobLineNumber":14707,"text":" \tmemset(new_val, 0, sizeof(new_val));","html":" \t\u003cspan class=pl-en\u003ememset\u003c/span\u003e(\u003cspan class=pl-s1\u003enew_val\u003c/span\u003e, \u003cspan class=pl-c1\u003e0\u003c/span\u003e, \u003cspan class=pl-k\u003esizeof\u003c/span\u003e(\u003cspan class=pl-s1\u003enew_val\u003c/span\u003e));","displayNoNewLineWarning":false,"position":29,"left":14711,"right":14707},{"stylingDirective":null,"type":"CONTEXT","blobLineNumber":14708,"text":" \tmemset(new_null, false, sizeof(new_null));","html":" \t\u003cspan class=pl-en\u003ememset\u003c/span\u003e(\u003cspan class=pl-s1\u003enew_null\u003c/span\u003e, false, \u003cspan class=pl-k\u003esizeof\u003c/span\u003e(\u003cspan class=pl-s1\u003enew_null\u003c/span\u003e));","displayNoNewLineWarning":false,"position":30,"left":14712,"right":14708}],"diffNumber":0,"diffSize":"0 Bytes","isBinary":false,"isTooBig":false,"collapsed":false,"isSubmodule":false,"lineCount":15152,"linesChanged":10,"newTreeEntry":{"lineCount":15152,"path":"src/backend/commands/tablecmds.c","mode":100644,"isGenerated":false},"oldTreeEntry":{"lineCount":0,"path":"src/backend/commands/tablecmds.c","mode":100644},"linesAdded":3,"linesDeleted":7,"path":"src/backend/commands/tablecmds.c","pathDigest":"d373fb790605aded212c2e3a0fbe8c015d110077ff46c07d0a6d6647a2d7e928","status":"MODIFIED","truncatedReason":null,"oldOid":"f5a6509bb1ec5222a707205941a40f280f3e6e15","newOid":"2fbdf1b38bc54b297e0f885ca97e0c8f5c922e72","copilotChatReference":null,"deletedSha":"f5a6509bb1ec5222a707205941a40f280f3e6e15","canToggleRichDiff":false,"defaultToRichDiff":false,"proseDifffHtml":null,"renderInfo":null,"dependencyDiffPath":null,"submodule":null},{"diffLines":[{"stylingDirective":null,"type":"HUNK","blobLineNumber":1640,"text":"@@ -1641,8 +1641,9 @@ get_qual_for_range(Relation parent, PartitionBoundSpec *spec,","html":"@@ -1641,8 +1641,9 @@ get_qual_for_range(Relation parent, PartitionBoundSpec *spec,","displayNoNewLineWarning":false,"position":0,"left":1640,"right":1640},{"stylingDirective":null,"type":"CONTEXT","blobLineNumber":1641,"text":" \t\t\tdatum = SysCacheGetAttr(RELOID, tuple,","html":" \t\t\t\u003cspan class=pl-s1\u003edatum\u003c/span\u003e \u003cspan class=pl-c1\u003e=\u003c/span\u003e \u003cspan class=pl-en\u003eSysCacheGetAttr\u003c/span\u003e(\u003cspan class=pl-c1\u003eRELOID\u003c/span\u003e, \u003cspan class=pl-s1\u003etuple\u003c/span\u003e,","displayNoNewLineWarning":false,"position":1,"left":1641,"right":1641},{"stylingDirective":null,"type":"CONTEXT","blobLineNumber":1642,"text":" \t\t\t\t\t\t\t\t\tAnum_pg_class_relpartbound,","html":" \t\t\t\t\t\t\t\t\t\u003cspan class=pl-s1\u003eAnum_pg_class_relpartbound\u003c/span\u003e,","displayNoNewLineWarning":false,"position":2,"left":1642,"right":1642},{"stylingDirective":null,"type":"CONTEXT","blobLineNumber":1643,"text":" \t\t\t\t\t\t\t\t\t\u0026isnull);","html":" \t\t\t\t\t\t\t\t\t\u003cspan class=pl-c1\u003e\u0026amp;\u003c/span\u003e\u003cspan class=pl-s1\u003eisnull\u003c/span\u003e);","displayNoNewLineWarning":false,"position":3,"left":1643,"right":1643},{"stylingDirective":null,"type":"ADDITION","blobLineNumber":1644,"text":"+\t\t\tif (isnull)","html":"+\t\t\t\u003cspan class=pl-k\u003eif\u003c/span\u003e (\u003cspan class=pl-s1\u003eisnull\u003c/span\u003e)","displayNoNewLineWarning":false,"position":4,"left":1643,"right":1644},{"stylingDirective":null,"type":"ADDITION","blobLineNumber":1645,"text":"+\t\t\t\telog(ERROR, \"null relpartbound for relation %u\", inhrelid);","html":"+\t\t\t\t\u003cspan class=pl-en\u003eelog\u003c/span\u003e(\u003cspan class=pl-c1\u003eERROR\u003c/span\u003e, \u003cspan class=pl-s\u003e\u0026quot;null relpartbound for relation %u\u0026quot;\u003c/span\u003e, \u003cspan class=pl-s1\u003einhrelid\u003c/span\u003e);","displayNoNewLineWarning":false,"position":5,"left":1643,"right":1645},{"stylingDirective":null,"type":"CONTEXT","blobLineNumber":1646,"text":" ","html":"\u003cbr\u003e","displayNoNewLineWarning":false,"position":6,"left":1644,"right":1646},{"stylingDirective":null,"type":"DELETION","blobLineNumber":1645,"text":"-\t\t\tAssert(!isnull);","html":"-\t\t\t\u003cspan class=pl-en\u003eAssert\u003c/span\u003e(!\u003cspan class=pl-s1\u003eisnull\u003c/span\u003e);","displayNoNewLineWarning":false,"position":7,"left":1645,"right":1646},{"stylingDirective":null,"type":"CONTEXT","blobLineNumber":1647,"text":" \t\t\tbspec = (PartitionBoundSpec *)","html":" \t\t\t\u003cspan class=pl-s1\u003ebspec\u003c/span\u003e \u003cspan class=pl-c1\u003e=\u003c/span\u003e (\u003cspan class=pl-smi\u003ePartitionBoundSpec\u003c/span\u003e \u003cspan class=pl-c1\u003e*\u003c/span\u003e)","displayNoNewLineWarning":false,"position":8,"left":1646,"right":1647},{"stylingDirective":null,"type":"CONTEXT","blobLineNumber":1648,"text":" \t\t\t\tstringToNode(TextDatumGetCString(datum));","html":" \t\t\t\t\u003cspan class=pl-en\u003estringToNode\u003c/span\u003e(\u003cspan class=pl-en\u003eTextDatumGetCString\u003c/span\u003e(\u003cspan class=pl-s1\u003edatum\u003c/span\u003e));","displayNoNewLineWarning":false,"position":9,"left":1647,"right":1648},{"stylingDirective":null,"type":"CONTEXT","blobLineNumber":1649,"text":" \t\t\tif (!IsA(bspec, PartitionBoundSpec))","html":" \t\t\t\u003cspan class=pl-k\u003eif\u003c/span\u003e (!\u003cspan class=pl-en\u003eIsA\u003c/span\u003e(\u003cspan class=pl-s1\u003ebspec\u003c/span\u003e, \u003cspan class=pl-s1\u003ePartitionBoundSpec\u003c/span\u003e))","displayNoNewLineWarning":false,"position":10,"left":1648,"right":1649}],"diffNumber":1,"diffSize":"0 Bytes","isBinary":false,"isTooBig":false,"collapsed":false,"isSubmodule":false,"lineCount":2307,"linesChanged":3,"newTreeEntry":{"lineCount":2307,"path":"src/backend/partitioning/partbounds.c","mode":100644,"isGenerated":false},"oldTreeEntry":{"lineCount":0,"path":"src/backend/partitioning/partbounds.c","mode":100644},"linesAdded":2,"linesDeleted":1,"path":"src/backend/partitioning/partbounds.c","pathDigest":"ef9f716dcef24bb41566340fa9e0f6262389d570c53f0a15d1f2d54888339401","status":"MODIFIED","truncatedReason":null,"oldOid":"f5a6509bb1ec5222a707205941a40f280f3e6e15","newOid":"2fbdf1b38bc54b297e0f885ca97e0c8f5c922e72","copilotChatReference":null,"deletedSha":"f5a6509bb1ec5222a707205941a40f280f3e6e15","canToggleRichDiff":false,"defaultToRichDiff":false,"proseDifffHtml":null,"renderInfo":null,"dependencyDiffPath":null,"submodule":null},{"diffLines":[{"stylingDirective":null,"type":"HUNK","blobLineNumber":301,"text":"@@ -302,23 +302,11 @@ RelationBuildPartitionDesc(Relation rel)","html":"@@ -302,23 +302,11 @@ RelationBuildPartitionDesc(Relation rel)","displayNoNewLineWarning":false,"position":0,"left":301,"right":301},{"stylingDirective":null,"type":"CONTEXT","blobLineNumber":302,"text":" \t\tif (!HeapTupleIsValid(tuple))","html":" \t\t\u003cspan class=pl-k\u003eif\u003c/span\u003e (!\u003cspan class=pl-en\u003eHeapTupleIsValid\u003c/span\u003e(\u003cspan class=pl-s1\u003etuple\u003c/span\u003e))","displayNoNewLineWarning":false,"position":1,"left":302,"right":302},{"stylingDirective":null,"type":"CONTEXT","blobLineNumber":303,"text":" \t\t\telog(ERROR, \"cache lookup failed for relation %u\", inhrelid);","html":" \t\t\t\u003cspan class=pl-en\u003eelog\u003c/span\u003e(\u003cspan class=pl-c1\u003eERROR\u003c/span\u003e, \u003cspan class=pl-s\u003e\u0026quot;cache lookup failed for relation %u\u0026quot;\u003c/span\u003e, \u003cspan class=pl-s1\u003einhrelid\u003c/span\u003e);","displayNoNewLineWarning":false,"position":2,"left":303,"right":303},{"stylingDirective":null,"type":"CONTEXT","blobLineNumber":304,"text":" ","html":"\u003cbr\u003e","displayNoNewLineWarning":false,"position":3,"left":304,"right":304},{"stylingDirective":null,"type":"DELETION","blobLineNumber":305,"text":"-\t\t/*","html":"-\t\t\u003cspan class=pl-c\u003e/*\u003c/span\u003e","displayNoNewLineWarning":false,"position":4,"left":305,"right":304},{"stylingDirective":null,"type":"DELETION","blobLineNumber":306,"text":"-\t\t * It is possible that the pg_class tuple of a partition has not been","html":"-\u003cspan class=pl-c\u003e\t\t * It is possible that the pg_class tuple of a partition has not been\u003c/span\u003e","displayNoNewLineWarning":false,"position":5,"left":306,"right":304},{"stylingDirective":null,"type":"DELETION","blobLineNumber":307,"text":"-\t\t * updated yet to set its relpartbound field. The only case where","html":"-\u003cspan class=pl-c\u003e\t\t * updated yet to set its relpartbound field. The only case where\u003c/span\u003e","displayNoNewLineWarning":false,"position":6,"left":307,"right":304},{"stylingDirective":null,"type":"DELETION","blobLineNumber":308,"text":"-\t\t * this happens is when we open the parent relation to check using its","html":"-\u003cspan class=pl-c\u003e\t\t * this happens is when we open the parent relation to check using its\u003c/span\u003e","displayNoNewLineWarning":false,"position":7,"left":308,"right":304},{"stylingDirective":null,"type":"DELETION","blobLineNumber":309,"text":"-\t\t * partition descriptor that a new partition's bound does not overlap","html":"-\u003cspan class=pl-c\u003e\t\t * partition descriptor that a new partition\u0026#39;s bound does not overlap\u003c/span\u003e","displayNoNewLineWarning":false,"position":8,"left":309,"right":304},{"stylingDirective":null,"type":"DELETION","blobLineNumber":310,"text":"-\t\t * some existing partition.","html":"-\u003cspan class=pl-c\u003e\t\t * some existing partition.\u003c/span\u003e","displayNoNewLineWarning":false,"position":9,"left":310,"right":304},{"stylingDirective":null,"type":"DELETION","blobLineNumber":311,"text":"-\t\t */","html":"-\u003cspan class=pl-c\u003e\t\t */\u003c/span\u003e","displayNoNewLineWarning":false,"position":10,"left":311,"right":304},{"stylingDirective":null,"type":"DELETION","blobLineNumber":312,"text":"-\t\tif (!((Form_pg_class) GETSTRUCT(tuple))-\u003erelispartition)","html":"-\t\t\u003cspan class=pl-k\u003eif\u003c/span\u003e (!((\u003cspan class=pl-smi\u003eForm_pg_class\u003c/span\u003e) \u003cspan class=pl-en\u003eGETSTRUCT\u003c/span\u003e(\u003cspan class=pl-s1\u003etuple\u003c/span\u003e))\u003cspan class=pl-c1\u003e-\u0026gt;\u003c/span\u003e\u003cspan class=pl-c1\u003erelispartition\u003c/span\u003e)","displayNoNewLineWarning":false,"position":11,"left":312,"right":304},{"stylingDirective":null,"type":"DELETION","blobLineNumber":313,"text":"-\t\t{","html":"-\t\t{","displayNoNewLineWarning":false,"position":12,"left":313,"right":304},{"stylingDirective":null,"type":"DELETION","blobLineNumber":314,"text":"-\t\t\tReleaseSysCache(tuple);","html":"-\t\t\t\u003cspan class=pl-en\u003eReleaseSysCache\u003c/span\u003e(\u003cspan class=pl-s1\u003etuple\u003c/span\u003e);","displayNoNewLineWarning":false,"position":13,"left":314,"right":304},{"stylingDirective":null,"type":"DELETION","blobLineNumber":315,"text":"-\t\t\tcontinue;","html":"-\t\t\t\u003cspan class=pl-k\u003econtinue\u003c/span\u003e;","displayNoNewLineWarning":false,"position":14,"left":315,"right":304},{"stylingDirective":null,"type":"DELETION","blobLineNumber":316,"text":"-\t\t}","html":"-\t\t}","displayNoNewLineWarning":false,"position":15,"left":316,"right":304},{"stylingDirective":null,"type":"DELETION","blobLineNumber":317,"text":"-","html":"-","displayNoNewLineWarning":false,"position":16,"left":317,"right":304},{"stylingDirective":null,"type":"CONTEXT","blobLineNumber":305,"text":" \t\tdatum = SysCacheGetAttr(RELOID, tuple,","html":" \t\t\u003cspan class=pl-s1\u003edatum\u003c/span\u003e \u003cspan class=pl-c1\u003e=\u003c/span\u003e \u003cspan class=pl-en\u003eSysCacheGetAttr\u003c/span\u003e(\u003cspan class=pl-c1\u003eRELOID\u003c/span\u003e, \u003cspan class=pl-s1\u003etuple\u003c/span\u003e,","displayNoNewLineWarning":false,"position":17,"left":318,"right":305},{"stylingDirective":null,"type":"CONTEXT","blobLineNumber":306,"text":" \t\t\t\t\t\t\t\tAnum_pg_class_relpartbound,","html":" \t\t\t\t\t\t\t\t\u003cspan class=pl-s1\u003eAnum_pg_class_relpartbound\u003c/span\u003e,","displayNoNewLineWarning":false,"position":18,"left":319,"right":306},{"stylingDirective":null,"type":"CONTEXT","blobLineNumber":307,"text":" \t\t\t\t\t\t\t\t\u0026isnull);","html":" \t\t\t\t\t\t\t\t\u003cspan class=pl-c1\u003e\u0026amp;\u003c/span\u003e\u003cspan class=pl-s1\u003eisnull\u003c/span\u003e);","displayNoNewLineWarning":false,"position":19,"left":320,"right":307},{"stylingDirective":null,"type":"DELETION","blobLineNumber":321,"text":"-\t\tAssert(!isnull);","html":"-\t\t\u003cspan class=pl-en\u003eAssert\u003c/span\u003e(!\u003cspan class=pl-s1\u003eisnull\u003c/span\u003e);","displayNoNewLineWarning":false,"position":20,"left":321,"right":307},{"stylingDirective":null,"type":"ADDITION","blobLineNumber":308,"text":"+\t\tif (isnull)","html":"+\t\t\u003cspan class=pl-k\u003eif\u003c/span\u003e (\u003cspan class=pl-s1\u003eisnull\u003c/span\u003e)","displayNoNewLineWarning":false,"position":21,"left":321,"right":308},{"stylingDirective":null,"type":"ADDITION","blobLineNumber":309,"text":"+\t\t\telog(ERROR, \"null relpartbound for relation %u\", inhrelid);","html":"+\t\t\t\u003cspan class=pl-en\u003eelog\u003c/span\u003e(\u003cspan class=pl-c1\u003eERROR\u003c/span\u003e, \u003cspan class=pl-s\u003e\u0026quot;null relpartbound for relation %u\u0026quot;\u003c/span\u003e, \u003cspan class=pl-s1\u003einhrelid\u003c/span\u003e);","displayNoNewLineWarning":false,"position":22,"left":321,"right":309},{"stylingDirective":null,"type":"CONTEXT","blobLineNumber":310,"text":" \t\tboundspec = (Node *) stringToNode(TextDatumGetCString(datum));","html":" \t\t\u003cspan class=pl-s1\u003eboundspec\u003c/span\u003e \u003cspan class=pl-c1\u003e=\u003c/span\u003e (\u003cspan class=pl-smi\u003eNode\u003c/span\u003e \u003cspan class=pl-c1\u003e*\u003c/span\u003e) \u003cspan class=pl-en\u003estringToNode\u003c/span\u003e(\u003cspan class=pl-en\u003eTextDatumGetCString\u003c/span\u003e(\u003cspan class=pl-s1\u003edatum\u003c/span\u003e));","displayNoNewLineWarning":false,"position":23,"left":322,"right":310},{"stylingDirective":null,"type":"CONTEXT","blobLineNumber":311,"text":" ","html":"\u003cbr\u003e","displayNoNewLineWarning":false,"position":24,"left":323,"right":311},{"stylingDirective":null,"type":"CONTEXT","blobLineNumber":312,"text":" \t\t/*","html":" \t\t\u003cspan class=pl-c\u003e/*\u003c/span\u003e","displayNoNewLineWarning":false,"position":25,"left":324,"right":312},{"stylingDirective":null,"type":"HUNK","blobLineNumber":870,"text":"@@ -883,9 +871,8 @@ generate_partition_qual(Relation rel)","html":"@@ -883,9 +871,8 @@ generate_partition_qual(Relation rel)","displayNoNewLineWarning":false,"position":26,"left":882,"right":870},{"stylingDirective":null,"type":"CONTEXT","blobLineNumber":871,"text":" \tboundDatum = SysCacheGetAttr(RELOID, tuple,","html":" \t\u003cspan class=pl-s1\u003eboundDatum\u003c/span\u003e \u003cspan class=pl-c1\u003e=\u003c/span\u003e \u003cspan class=pl-en\u003eSysCacheGetAttr\u003c/span\u003e(\u003cspan class=pl-c1\u003eRELOID\u003c/span\u003e, \u003cspan class=pl-s1\u003etuple\u003c/span\u003e,","displayNoNewLineWarning":false,"position":27,"left":883,"right":871},{"stylingDirective":null,"type":"CONTEXT","blobLineNumber":872,"text":" \t\t\t\t\t\t\t\t Anum_pg_class_relpartbound,","html":" \t\t\t\t\t\t\t\t \u003cspan class=pl-s1\u003eAnum_pg_class_relpartbound\u003c/span\u003e,","displayNoNewLineWarning":false,"position":28,"left":884,"right":872},{"stylingDirective":null,"type":"CONTEXT","blobLineNumber":873,"text":" \t\t\t\t\t\t\t\t \u0026isnull);","html":" \t\t\t\t\t\t\t\t \u003cspan class=pl-c1\u003e\u0026amp;\u003c/span\u003e\u003cspan class=pl-s1\u003eisnull\u003c/span\u003e);","displayNoNewLineWarning":false,"position":29,"left":885,"right":873},{"stylingDirective":null,"type":"DELETION","blobLineNumber":886,"text":"-\tif (isnull)\t\t\t\t\t/* should not happen */","html":"-\t\u003cspan class=pl-k\u003eif\u003c/span\u003e (\u003cspan class=pl-s1\u003eisnull\u003c/span\u003e)\t\t\t\t\t\u003cspan class=pl-c\u003e/* should not happen */\u003c/span\u003e","displayNoNewLineWarning":false,"position":30,"left":886,"right":873},{"stylingDirective":null,"type":"DELETION","blobLineNumber":887,"text":"-\t\telog(ERROR, \"relation \\\"%s\\\" has relpartbound = null\",","html":"-\t\t\u003cspan class=pl-en\u003eelog\u003c/span\u003e(\u003cspan class=pl-c1\u003eERROR\u003c/span\u003e, \u003cspan class=pl-s\u003e\u0026quot;relation \\\u0026quot;%s\\\u0026quot; has relpartbound = null\u0026quot;\u003c/span\u003e,","displayNoNewLineWarning":false,"position":31,"left":887,"right":873},{"stylingDirective":null,"type":"DELETION","blobLineNumber":888,"text":"-\t\t\t RelationGetRelationName(rel));","html":"-\t\t\t \u003cspan class=pl-en\u003eRelationGetRelationName\u003c/span\u003e(\u003cspan class=pl-s1\u003erel\u003c/span\u003e));","displayNoNewLineWarning":false,"position":32,"left":888,"right":873},{"stylingDirective":null,"type":"ADDITION","blobLineNumber":874,"text":"+\tif (isnull)","html":"+\t\u003cspan class=pl-k\u003eif\u003c/span\u003e (\u003cspan class=pl-s1\u003eisnull\u003c/span\u003e)","displayNoNewLineWarning":false,"position":33,"left":888,"right":874},{"stylingDirective":null,"type":"ADDITION","blobLineNumber":875,"text":"+\t\telog(ERROR, \"null relpartbound for relation %u\", RelationGetRelid(rel));","html":"+\t\t\u003cspan class=pl-en\u003eelog\u003c/span\u003e(\u003cspan class=pl-c1\u003eERROR\u003c/span\u003e, \u003cspan class=pl-s\u003e\u0026quot;null relpartbound for relation %u\u0026quot;\u003c/span\u003e, \u003cspan class=pl-en\u003eRelationGetRelid\u003c/span\u003e(\u003cspan class=pl-s1\u003erel\u003c/span\u003e));","displayNoNewLineWarning":false,"position":34,"left":888,"right":875},{"stylingDirective":null,"type":"CONTEXT","blobLineNumber":876,"text":" \tbound = castNode(PartitionBoundSpec,","html":" \t\u003cspan class=pl-s1\u003ebound\u003c/span\u003e \u003cspan class=pl-c1\u003e=\u003c/span\u003e \u003cspan class=pl-en\u003ecastNode\u003c/span\u003e(\u003cspan class=pl-s1\u003ePartitionBoundSpec\u003c/span\u003e,","displayNoNewLineWarning":false,"position":35,"left":889,"right":876},{"stylingDirective":null,"type":"CONTEXT","blobLineNumber":877,"text":" \t\t\t\t\t stringToNode(TextDatumGetCString(boundDatum)));","html":" \t\t\t\t\t \u003cspan class=pl-en\u003estringToNode\u003c/span\u003e(\u003cspan class=pl-en\u003eTextDatumGetCString\u003c/span\u003e(\u003cspan class=pl-s1\u003eboundDatum\u003c/span\u003e)));","displayNoNewLineWarning":false,"position":36,"left":890,"right":877},{"stylingDirective":null,"type":"CONTEXT","blobLineNumber":878,"text":" \tReleaseSysCache(tuple);","html":" \t\u003cspan class=pl-en\u003eReleaseSysCache\u003c/span\u003e(\u003cspan class=pl-s1\u003etuple\u003c/span\u003e);","displayNoNewLineWarning":false,"position":37,"left":891,"right":878}],"diffNumber":2,"diffSize":"0 Bytes","isBinary":false,"isTooBig":false,"collapsed":false,"isSubmodule":false,"lineCount":954,"linesChanged":21,"newTreeEntry":{"lineCount":954,"path":"src/backend/utils/cache/partcache.c","mode":100644,"isGenerated":false},"oldTreeEntry":{"lineCount":0,"path":"src/backend/utils/cache/partcache.c","mode":100644},"linesAdded":4,"linesDeleted":17,"path":"src/backend/utils/cache/partcache.c","pathDigest":"4a0bf3023a4741569313bd9d3622942661fcb2a10b0c2d4b93af11197b9b6756","status":"MODIFIED","truncatedReason":null,"oldOid":"f5a6509bb1ec5222a707205941a40f280f3e6e15","newOid":"2fbdf1b38bc54b297e0f885ca97e0c8f5c922e72","copilotChatReference":null,"deletedSha":"f5a6509bb1ec5222a707205941a40f280f3e6e15","canToggleRichDiff":false,"defaultToRichDiff":false,"proseDifffHtml":null,"renderInfo":null,"dependencyDiffPath":null,"submodule":null}],"splitViewPreference":"unified","ignoreWhitespace":false,"repoOwnerGlobalRelayId":"MDEyOk9yZ2FuaXphdGlvbjE3NzU0Mw==","commentsPreference":"visible","diffLineSpacingPreference":"relaxed","useMonospaceFont":false,"pasteUrlLinkAsPlainText":false,"userNotices":[],"path":"/postgres/postgres/commit/2fbdf1b38bc54b297e0f885ca97e0c8f5c922e72","fileTreeExpanded":true,"headerInfo":{"additions":9,"deletions":25,"filesChanged":3,"filesChangedString":"3"},"moreDiffsToLoad":false,"asyncDiffLoadInfo":{"startIndex":3,"truncated":false,"byteCount":2960,"lineShownCount":80},"commentInfo":{"canComment":false,"locked":false,"canLock":false,"repoArchived":false},"csrf_tokens":{"/users/diffview?diff=split":{"post":"J9NZfArGkdQMOjzQUmCZ8XtXEA2TASK4cSpUs0Wza_C-sslSlmFFZYUdCpnJwQmtP4aR-7hHAXj6oJwLm-QiCQ"},"/users/diffview?diff=unified":{"post":"NiCeMcVzqqwtzKED6ml7VglW1Fxoj9hNFu2Uw1aaT-2vQQ4fWdR-HaTrl0pxyOsKTYdVqkPJ-42dZ1x7iM0GFA"},"/notifications/thread":{"post":"eLeqQ7I2Vtlhln83TdjxxHWZ4PS-91ePiXG5LLXdqPBIqfKfSKLJjCmL47idzL8dxpFfMQbyuRtfopLeNM1HjA"}}},"title":"Simplify partitioned table creation vs. relcache · postgres/postgres@2fbdf1b","appPayload":{"helpUrl":"https://docs.github.com","findInDiffWorkerPath":"/assets-cdn/worker/find-in-diff-worker-2bfe39677d14.js","enabled_features":{"diff_ux_refresh_beta":false,"diff_inline_comments":true,"diff_ux_refresh_ssr_five":false,"diff_ux_refresh_ssr_ten":false,"react_diff_line_type_character_correction":true}}}

Commit 2fbdf1b

Browse files
alvherrealvherre
committed
Simplify partitioned table creation vs. relcache
In the original code, we were storing the pg_inherits row for a partitioned table too early: enough that we had a hack for relcache to avoid falling flat on its face while reading such a partial entry. If we finish the pg_class creation first and *then* store the pg_inherits entry, we don't need that hack. Also recognize that pg_class.relpartbound is not marked NOT NULL and therefore it's entirely possible to read null values, so having only Assert() protection isn't enough. Change those to if/elog tests instead. This qualifies as a robustness fix, so backpatch to pg11. In passing, remove one access that wasn't actually needed, and reword one message to be like all the others that check for the same thing. Reviewed-by: Amit Langote Discussion: https://postgr.es/m/20180903213916.hh6wasnrdg6xv2ud@alvherre.pgsql
1 parent f5a6509 commit 2fbdf1b

File tree

3 files changed

+9
-25
lines changed

3 files changed

+9
-25
lines changed

‎src/backend/commands/tablecmds.c

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -773,9 +773,6 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
773773
InvalidOid,
774774
typaddress);
775775

776-
/* Store inheritance information for new rel. */
777-
StoreCatalogInheritance(relationId, inheritOids, stmt->partbound != NULL);
778-
779776
/*
780777
* We must bump the command counter to make the newly-created relation
781778
* tuple visible for opening.
@@ -869,6 +866,9 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
869866
heap_close(parent, NoLock);
870867
}
871868

869+
/* Store inheritance information for new rel. */
870+
StoreCatalogInheritance(relationId, inheritOids, stmt->partbound != NULL);
871+
872872
/*
873873
* Process the partitioning specification (if any) and store the partition
874874
* key information into the catalog.
@@ -14703,10 +14703,6 @@ ATExecDetachPartition(Relation rel, RangeVar *name)
1470314703
RelationGetRelid(partRel));
1470414704
Assert(((Form_pg_class) GETSTRUCT(tuple))->relispartition);
1470514705

14706-
(void) SysCacheGetAttr(RELOID, tuple, Anum_pg_class_relpartbound,
14707-
&isnull);
14708-
Assert(!isnull);
14709-
1471014706
/* Clear relpartbound and reset relispartition */
1471114707
memset(new_val, 0, sizeof(new_val));
1471214708
memset(new_null, false, sizeof(new_null));

‎src/backend/partitioning/partbounds.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1641,8 +1641,9 @@ get_qual_for_range(Relation parent, PartitionBoundSpec *spec,
16411641
datum = SysCacheGetAttr(RELOID, tuple,
16421642
Anum_pg_class_relpartbound,
16431643
&isnull);
1644+
if (isnull)
1645+
elog(ERROR, "null relpartbound for relation %u", inhrelid);
16441646

1645-
Assert(!isnull);
16461647
bspec = (PartitionBoundSpec *)
16471648
stringToNode(TextDatumGetCString(datum));
16481649
if (!IsA(bspec, PartitionBoundSpec))

‎src/backend/utils/cache/partcache.c

Lines changed: 4 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -302,23 +302,11 @@ RelationBuildPartitionDesc(Relation rel)
302302
if (!HeapTupleIsValid(tuple))
303303
elog(ERROR, "cache lookup failed for relation %u", inhrelid);
304304

305-
/*
306-
* It is possible that the pg_class tuple of a partition has not been
307-
* updated yet to set its relpartbound field. The only case where
308-
* this happens is when we open the parent relation to check using its
309-
* partition descriptor that a new partition's bound does not overlap
310-
* some existing partition.
311-
*/
312-
if (!((Form_pg_class) GETSTRUCT(tuple))->relispartition)
313-
{
314-
ReleaseSysCache(tuple);
315-
continue;
316-
}
317-
318305
datum = SysCacheGetAttr(RELOID, tuple,
319306
Anum_pg_class_relpartbound,
320307
&isnull);
321-
Assert(!isnull);
308+
if (isnull)
309+
elog(ERROR, "null relpartbound for relation %u", inhrelid);
322310
boundspec = (Node *) stringToNode(TextDatumGetCString(datum));
323311

324312
/*
@@ -883,9 +871,8 @@ generate_partition_qual(Relation rel)
883871
boundDatum = SysCacheGetAttr(RELOID, tuple,
884872
Anum_pg_class_relpartbound,
885873
&isnull);
886-
if (isnull) /* should not happen */
887-
elog(ERROR, "relation \"%s\" has relpartbound = null",
888-
RelationGetRelationName(rel));
874+
if (isnull)
875+
elog(ERROR, "null relpartbound for relation %u", RelationGetRelid(rel));
889876
bound = castNode(PartitionBoundSpec,
890877
stringToNode(TextDatumGetCString(boundDatum)));
891878
ReleaseSysCache(tuple);

0 commit comments

Comments
 (0)
0