Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1l48DC-0002WF-5a for pgsql-hackers@arkaria.postgresql.org; Mon, 25 Jan 2021 20:11:46 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.92) (envelope-from ) id 1l48DB-0003Ai-3G for pgsql-hackers@arkaria.postgresql.org; Mon, 25 Jan 2021 20:11:45 +0000 Received: from makus.postgresql.org ([2001:4800:3e1:1::229]) by malur.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1l48DA-0003AZ-Kp for pgsql-hackers@lists.postgresql.org; Mon, 25 Jan 2021 20:11:44 +0000 Received: from mail.postgrespro.ru ([93.174.131.139]) by makus.postgresql.org with esmtp (Exim 4.92) (envelope-from ) id 1l48D6-0006CX-I6 for pgsql-hackers@lists.postgresql.org; Mon, 25 Jan 2021 20:11:43 +0000 Received: from mail.postgrespro.ru (cyclops.postgrespro.ru [93.174.131.138]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client did not present a certificate) by mail.postgrespro.ru (Postfix) with ESMTPSA id BE18B21C2388; Mon, 25 Jan 2021 23:11:38 +0300 (MSK) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=postgrespro.ru; s=mail; t=1611605498; bh=AOZQNdHqC1vsPXu+vsgje75uOtB1nZvNb4gZlLRwN84=; h=Date:From:To:Cc:Subject:In-Reply-To:References; b=cIk+JrjS/zGbi5udFnvmc80/qaHS75vO7xVDSx4TWfXY5aRB95IyQp19j9+j1oPf0 A7m113Xo8Gxa5EK6LaRPhqQQJ6gaZK1SoXZ3KlKh7qg1kn9RkmQFriWzR+VO8tbKFj qKePiHzwHmS65TlbvskosKN5if3wG+IOdHk+oBVo= MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="=_0690eeedaab51db287f27f25548bff33" Content-Transfer-Encoding: 7bit Date: Mon, 25 Jan 2021 23:11:38 +0300 From: Alexey Kondratov To: Michael Paquier Cc: Justin Pryzby , Alvaro Herrera , Peter Eisentraut , Masahiko Sawada , Steve Singer , pgsql-hackers@lists.postgresql.org, Robert Haas , Alexander Korotkov , Masahiko Sawada , Jose Luis Tallon Subject: Re: Allow CLUSTER, VACUUM FULL and REINDEX to change tablespace on the fly In-Reply-To: References: <03f88f70618ce73e75837b6125a143f7@postgrespro.ru> <20210120183439.GA21339@alvherre.pgsql> <20210121212651.GY8560@telsasoft.com> <944df7cc452fe0c34cf7820da4588d05@postgrespro.ru> User-Agent: Roundcube Webmail/1.4.0 Message-ID: <690fa051803c071d213b0d07b5aa9f55@postgrespro.ru> X-Sender: a.kondratov@postgrespro.ru List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk --=_0690eeedaab51db287f27f25548bff33 Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset=US-ASCII; format=flowed On 2021-01-25 11:07, Michael Paquier wrote: > On Fri, Jan 22, 2021 at 05:07:02PM +0300, Alexey Kondratov wrote: >> I have updated patches accordingly and also simplified tablespaceOid >> checks >> and assignment in the newly added SetRelTableSpace(). Result is >> attached as >> two separate patches for an ease of review, but no objections to merge >> them >> and apply at once if everything is fine. > > extern void SetRelationHasSubclass(Oid relationId, bool > relhassubclass); > +extern bool SetRelTableSpace(Oid reloid, Oid tablespaceOid); > Seeing SetRelationHasSubclass(), wouldn't it be more consistent to use > SetRelationTableSpace() as routine name? > > I think that we should document that the caller of this routine had > better do a CCI once done to make the tablespace chage visible. > Except for those two nits, the patch needs an indentation run and some > style tweaks but its logic looks fine. So I'll apply that first > piece. > I updated comment with CCI info, did pgindent run and renamed new function to SetRelationTableSpace(). New patch is attached. > +INSERT INTO regress_tblspace_test_tbl (num1, num2, t) > + SELECT round(random()*100), random(), repeat('text', 1000000) > + FROM generate_series(1, 10) s(i); > Repeating 1M times a text value is too costly for such a test. And as > even for empty tables there is one page created for toast indexes, > there is no need for that? > Yes, TOAST relation is created anyway. I just wanted to put some data into a TOAST index, so REINDEX did some meaningful work there, not only a new relfilenode creation. However you are right and this query increases tablespace tests execution for more for more than 2 times on my machine. I think that it is not really required. > > This patch is introducing three new checks for system catalogs: > - don't use tablespace for mapped relations. > - don't use tablespace for system relations, except if > allowSystemTableMods. > - don't move non-shared relation to global tablespace. > For the non-concurrent case, all three checks are in reindex_index(). > For the concurrent case, the two first checks are in > ReindexMultipleTables() and the third one is in > ReindexRelationConcurrently(). That's rather tricky to follow because > CONCURRENTLY is not allowed on system relations. I am wondering if it > would be worth an extra comment effort, or if there is a way to > consolidate that better. > Yeah, all these checks we complicated from the beginning. I will try to find a better place tomorrow or put more info into the comments at least. I am also going to check/fix the remaining points regarding 002 tomorrow. Regards -- Alexey Kondratov Postgres Professional https://www.postgrespro.com Russian Postgres Company --=_0690eeedaab51db287f27f25548bff33 Content-Transfer-Encoding: base64 Content-Type: text/x-diff; name=v5-0001-Extract-common-part-from-ATExecSetTableSpaceNoSto.patch Content-Disposition: attachment; filename=v5-0001-Extract-common-part-from-ATExecSetTableSpaceNoSto.patch; size=4985 RnJvbSAzOTg4MDg0MmQ3YWYzMWRjYmZjZmZlNzIxOTI1MGIzMTEwMjk1NWQ1IE1vbiBTZXAgMTcg MDA6MDA6MDAgMjAwMQpGcm9tOiBBbGV4ZXkgS29uZHJhdG92IDxrb25kcmF0b3YuYWxla3NleUBn bWFpbC5jb20+CkRhdGU6IFdlZCwgMjAgSmFuIDIwMjEgMjA6MjE6MTIgKzAzMDAKU3ViamVjdDog W1BBVENIIHY1IDEvMl0gRXh0cmFjdCBjb21tb24gcGFydCBmcm9tIEFURXhlY1NldFRhYmxlU3Bh Y2VOb1N0b3JhZ2UKIGZvciBhIGZ1dHVyZSB1c2FnZQoKLS0tCiBzcmMvYmFja2VuZC9jb21tYW5k cy90YWJsZWNtZHMuYyB8IDk1ICsrKysrKysrKysrKysrKysrKystLS0tLS0tLS0tLS0tCiBzcmMv aW5jbHVkZS9jb21tYW5kcy90YWJsZWNtZHMuaCB8ICAyICsKIDIgZmlsZXMgY2hhbmdlZCwgNTgg aW5zZXJ0aW9ucygrKSwgMzkgZGVsZXRpb25zKC0pCgpkaWZmIC0tZ2l0IGEvc3JjL2JhY2tlbmQv Y29tbWFuZHMvdGFibGVjbWRzLmMgYi9zcmMvYmFja2VuZC9jb21tYW5kcy90YWJsZWNtZHMuYwpp bmRleCA4Njg3ZTlhOTdjLi5lYzljNDQwZTRlIDEwMDY0NAotLS0gYS9zcmMvYmFja2VuZC9jb21t YW5kcy90YWJsZWNtZHMuYworKysgYi9zcmMvYmFja2VuZC9jb21tYW5kcy90YWJsZWNtZHMuYwpA QCAtMTMyOTEsNiArMTMyOTEsNTkgQEAgQVRFeGVjU2V0VGFibGVTcGFjZShPaWQgdGFibGVPaWQs IE9pZCBuZXdUYWJsZVNwYWNlLCBMT0NLTU9ERSBsb2NrbW9kZSkKIAlsaXN0X2ZyZWUocmVsdG9h c3RpZHhpZHMpOwogfQogCisvKgorICogU2V0UmVsYXRpb25UYWJsZVNwYWNlIC0gbW9kaWZ5IHJl bGF0aW9uIHRhYmxlc3BhY2UgaW4gdGhlIHBnX2NsYXNzIGVudHJ5LgorICoKKyAqICdyZWxvaWQn IGlzIGFuIE9pZCBvZiByZWxhdGlvbiB0byBiZSBtb2RpZmllZC4KKyAqICd0YWJsZXNwYWNlT2lk JyBpcyBhbiBPaWQgb2YgbmV3IHRhYmxlc3BhY2UuCisgKgorICogQ2F0YWxvZyBtb2RpZmljYXRp b24gaXMgZG9uZSBvbmx5IGlmIHRhYmxlc3BhY2VPaWQgaXMgZGlmZmVyZW50IGZyb20KKyAqIHRo ZSBjdXJyZW50bHkgc2V0LiAgUmV0dXJuZWQgYm9vbCB2YWx1ZSBpcyBpbmRpY2F0aW5nIHdoZXRo ZXIgYW55IGNoYW5nZXMKKyAqIHdlcmUgbWFkZSBvciBub3QuICBOb3RlIHRoYXQgY2FsbGVyIGlz IHJlc3BvbnNpYmxlIGZvciBkb2luZworICogQ29tbWFuZENvdW50ZXJJbmNyZW1lbnQoKSB0byBt YWtlIHRhYmxlc3BhY2UgY2hhbmdlcyB2aXNpYmxlLgorICovCitib29sCitTZXRSZWxhdGlvblRh YmxlU3BhY2UoT2lkIHJlbG9pZCwgT2lkIHRhYmxlc3BhY2VPaWQpCit7CisJUmVsYXRpb24JcGdf Y2xhc3M7CisJSGVhcFR1cGxlCXR1cGxlOworCUZvcm1fcGdfY2xhc3MgcmRfcmVsOworCWJvb2wJ CWNoYW5nZWQgPSBmYWxzZTsKKworCS8qIEdldCBhIG1vZGlmaWFibGUgY29weSBvZiB0aGUgcmVs YXRpb24ncyBwZ19jbGFzcyByb3cuICovCisJcGdfY2xhc3MgPSB0YWJsZV9vcGVuKFJlbGF0aW9u UmVsYXRpb25JZCwgUm93RXhjbHVzaXZlTG9jayk7CisKKwl0dXBsZSA9IFNlYXJjaFN5c0NhY2hl Q29weTEoUkVMT0lELCBPYmplY3RJZEdldERhdHVtKHJlbG9pZCkpOworCWlmICghSGVhcFR1cGxl SXNWYWxpZCh0dXBsZSkpCisJCWVsb2coRVJST1IsICJjYWNoZSBsb29rdXAgZmFpbGVkIGZvciBy ZWxhdGlvbiAldSIsIHJlbG9pZCk7CisJcmRfcmVsID0gKEZvcm1fcGdfY2xhc3MpIEdFVFNUUlVD VCh0dXBsZSk7CisKKwkvKiBNeURhdGFiYXNlVGFibGVTcGFjZSBpcyBzdG9yZWQgYXMgSW52YWxp ZE9pZC4gKi8KKwlpZiAodGFibGVzcGFjZU9pZCA9PSBNeURhdGFiYXNlVGFibGVTcGFjZSkKKwkJ dGFibGVzcGFjZU9pZCA9IEludmFsaWRPaWQ7CisKKwkvKiBObyB3b3JrIGlmIG5vIGNoYW5nZSBp biB0YWJsZXNwYWNlLiAqLworCWlmICh0YWJsZXNwYWNlT2lkICE9IHJkX3JlbC0+cmVsdGFibGVz cGFjZSkKKwl7CisJCS8qIFVwZGF0ZSB0aGUgcGdfY2xhc3Mgcm93LiAqLworCQlyZF9yZWwtPnJl bHRhYmxlc3BhY2UgPSB0YWJsZXNwYWNlT2lkOworCQlDYXRhbG9nVHVwbGVVcGRhdGUocGdfY2xh c3MsICZ0dXBsZS0+dF9zZWxmLCB0dXBsZSk7CisKKwkJLyogUmVjb3JkIGRlcGVuZGVuY3kgb24g dGFibGVzcGFjZS4gKi8KKwkJY2hhbmdlRGVwZW5kZW5jeU9uVGFibGVzcGFjZShSZWxhdGlvblJl bGF0aW9uSWQsCisJCQkJCQkJCQkgcmVsb2lkLCByZF9yZWwtPnJlbHRhYmxlc3BhY2UpOworCisJ CWNoYW5nZWQgPSB0cnVlOworCX0KKworCUludm9rZU9iamVjdFBvc3RBbHRlckhvb2soUmVsYXRp b25SZWxhdGlvbklkLCByZWxvaWQsIDApOworCisJaGVhcF9mcmVldHVwbGUodHVwbGUpOworCXRh YmxlX2Nsb3NlKHBnX2NsYXNzLCBSb3dFeGNsdXNpdmVMb2NrKTsKKworCXJldHVybiBjaGFuZ2Vk OworfQorCiAvKgogICogU3BlY2lhbCBoYW5kbGluZyBvZiBBTFRFUiBUQUJMRSBTRVQgVEFCTEVT UEFDRSBmb3IgcmVsYXRpb25zIHdpdGggbm8KICAqIHN0b3JhZ2UgdGhhdCBoYXZlIGFuIGludGVy ZXN0IGluIHByZXNlcnZpbmcgdGFibGVzcGFjZS4KQEAgLTEzMzAxLDEwICsxMzM1NCw2IEBAIEFU RXhlY1NldFRhYmxlU3BhY2UoT2lkIHRhYmxlT2lkLCBPaWQgbmV3VGFibGVTcGFjZSwgTE9DS01P REUgbG9ja21vZGUpCiBzdGF0aWMgdm9pZAogQVRFeGVjU2V0VGFibGVTcGFjZU5vU3RvcmFnZShS ZWxhdGlvbiByZWwsIE9pZCBuZXdUYWJsZVNwYWNlKQogewotCUhlYXBUdXBsZQl0dXBsZTsKLQlP aWQJCQlvbGRUYWJsZVNwYWNlOwotCVJlbGF0aW9uCXBnX2NsYXNzOwotCUZvcm1fcGdfY2xhc3Mg cmRfcmVsOwogCU9pZAkJCXJlbG9pZCA9IFJlbGF0aW9uR2V0UmVsaWQocmVsKTsKIAogCS8qCkBA IC0xMzMxOSw0MSArMTMzNjgsOSBAQCBBVEV4ZWNTZXRUYWJsZVNwYWNlTm9TdG9yYWdlKFJlbGF0 aW9uIHJlbCwgT2lkIG5ld1RhYmxlU3BhY2UpCiAJCQkJKGVycmNvZGUoRVJSQ09ERV9JTlZBTElE X1BBUkFNRVRFUl9WQUxVRSksCiAJCQkJIGVycm1zZygib25seSBzaGFyZWQgcmVsYXRpb25zIGNh biBiZSBwbGFjZWQgaW4gcGdfZ2xvYmFsIHRhYmxlc3BhY2UiKSkpOwogCi0JLyoKLQkgKiBObyB3 b3JrIGlmIG5vIGNoYW5nZSBpbiB0YWJsZXNwYWNlLgotCSAqLwotCW9sZFRhYmxlU3BhY2UgPSBy ZWwtPnJkX3JlbC0+cmVsdGFibGVzcGFjZTsKLQlpZiAobmV3VGFibGVTcGFjZSA9PSBvbGRUYWJs ZVNwYWNlIHx8Ci0JCShuZXdUYWJsZVNwYWNlID09IE15RGF0YWJhc2VUYWJsZVNwYWNlICYmIG9s ZFRhYmxlU3BhY2UgPT0gMCkpCi0JewotCQlJbnZva2VPYmplY3RQb3N0QWx0ZXJIb29rKFJlbGF0 aW9uUmVsYXRpb25JZCwgcmVsb2lkLCAwKTsKLQkJcmV0dXJuOwotCX0KLQotCS8qIEdldCBhIG1v ZGlmaWFibGUgY29weSBvZiB0aGUgcmVsYXRpb24ncyBwZ19jbGFzcyByb3cgKi8KLQlwZ19jbGFz cyA9IHRhYmxlX29wZW4oUmVsYXRpb25SZWxhdGlvbklkLCBSb3dFeGNsdXNpdmVMb2NrKTsKLQot CXR1cGxlID0gU2VhcmNoU3lzQ2FjaGVDb3B5MShSRUxPSUQsIE9iamVjdElkR2V0RGF0dW0ocmVs b2lkKSk7Ci0JaWYgKCFIZWFwVHVwbGVJc1ZhbGlkKHR1cGxlKSkKLQkJZWxvZyhFUlJPUiwgImNh Y2hlIGxvb2t1cCBmYWlsZWQgZm9yIHJlbGF0aW9uICV1IiwgcmVsb2lkKTsKLQlyZF9yZWwgPSAo Rm9ybV9wZ19jbGFzcykgR0VUU1RSVUNUKHR1cGxlKTsKLQotCS8qIHVwZGF0ZSB0aGUgcGdfY2xh c3Mgcm93ICovCi0JcmRfcmVsLT5yZWx0YWJsZXNwYWNlID0gKG5ld1RhYmxlU3BhY2UgPT0gTXlE YXRhYmFzZVRhYmxlU3BhY2UpID8gSW52YWxpZE9pZCA6IG5ld1RhYmxlU3BhY2U7Ci0JQ2F0YWxv Z1R1cGxlVXBkYXRlKHBnX2NsYXNzLCAmdHVwbGUtPnRfc2VsZiwgdHVwbGUpOwotCi0JLyogUmVj b3JkIGRlcGVuZGVuY3kgb24gdGFibGVzcGFjZSAqLwotCWNoYW5nZURlcGVuZGVuY3lPblRhYmxl c3BhY2UoUmVsYXRpb25SZWxhdGlvbklkLAotCQkJCQkJCQkgcmVsb2lkLCByZF9yZWwtPnJlbHRh Ymxlc3BhY2UpOwotCi0JSW52b2tlT2JqZWN0UG9zdEFsdGVySG9vayhSZWxhdGlvblJlbGF0aW9u SWQsIHJlbG9pZCwgMCk7Ci0KLQloZWFwX2ZyZWV0dXBsZSh0dXBsZSk7Ci0KLQl0YWJsZV9jbG9z ZShwZ19jbGFzcywgUm93RXhjbHVzaXZlTG9jayk7Ci0KLQkvKiBNYWtlIHN1cmUgdGhlIHJlbHRh Ymxlc3BhY2UgY2hhbmdlIGlzIHZpc2libGUgKi8KLQlDb21tYW5kQ291bnRlckluY3JlbWVudCgp OworCWlmIChTZXRSZWxhdGlvblRhYmxlU3BhY2UocmVsb2lkLCBuZXdUYWJsZVNwYWNlKSkKKwkJ LyogTWFrZSBzdXJlIHRoZSByZWx0YWJsZXNwYWNlIGNoYW5nZSBpcyB2aXNpYmxlICovCisJCUNv bW1hbmRDb3VudGVySW5jcmVtZW50KCk7CiB9CiAKIC8qCmRpZmYgLS1naXQgYS9zcmMvaW5jbHVk ZS9jb21tYW5kcy90YWJsZWNtZHMuaCBiL3NyYy9pbmNsdWRlL2NvbW1hbmRzL3RhYmxlY21kcy5o CmluZGV4IDA4YzQ2M2QzYzQuLjRlZTc4NTMwMjcgMTAwNjQ0Ci0tLSBhL3NyYy9pbmNsdWRlL2Nv bW1hbmRzL3RhYmxlY21kcy5oCisrKyBiL3NyYy9pbmNsdWRlL2NvbW1hbmRzL3RhYmxlY21kcy5o CkBAIC02MSw2ICs2MSw4IEBAIGV4dGVybiB2b2lkIEV4ZWN1dGVUcnVuY2F0ZUd1dHMoTGlzdCAq ZXhwbGljaXRfcmVscywgTGlzdCAqcmVsaWRzLCBMaXN0ICpyZWxpZHNfCiAKIGV4dGVybiB2b2lk IFNldFJlbGF0aW9uSGFzU3ViY2xhc3MoT2lkIHJlbGF0aW9uSWQsIGJvb2wgcmVsaGFzc3ViY2xh c3MpOwogCitleHRlcm4gYm9vbCBTZXRSZWxhdGlvblRhYmxlU3BhY2UoT2lkIHJlbG9pZCwgT2lk IHRhYmxlc3BhY2VPaWQpOworCiBleHRlcm4gT2JqZWN0QWRkcmVzcyByZW5hbWVhdHQoUmVuYW1l U3RtdCAqc3RtdCk7CiAKIGV4dGVybiBPYmplY3RBZGRyZXNzIFJlbmFtZUNvbnN0cmFpbnQoUmVu YW1lU3RtdCAqc3RtdCk7CgpiYXNlLWNvbW1pdDogODgxOTMzZjE5NDIyMWFiY2NlMDdmYjEzNGVi ZTg2ODVlNWJiNThkZAotLSAKMi4yMC4xCgo= --=_0690eeedaab51db287f27f25548bff33--