MIME-Version: 1.0
Content-Type: multipart/mixed;
 boundary="=_3ea21e89d721c68f41576baeec11ff72"
Content-Transfer-Encoding: 7bit
Date: Tue, 07 Apr 2020 15:40:18 +0300
From: Alexey Kondratov <a.kondratov@postgrespro.ru>
To: Justin Pryzby <pryzby@telsasoft.com>
Cc: Michael Paquier <michael@paquier.xyz>, Masahiko Sawada
 <masahiko.sawada@2ndquadrant.com>, Steve Singer <steve@ssinger.info>,
 pgsql-hackers@lists.postgresql.org, Alvaro Herrera
 <alvherre@2ndquadrant.com>, Robert Haas <robertmhaas@gmail.com>, Alexander
 Korotkov <a.korotkov@postgrespro.ru>, Masahiko Sawada
 <sawada.mshk@gmail.com>, Jose Luis Tallon <jltallon@adv-solutions.net>
Subject: Re: Allow CLUSTER, VACUUM FULL and REINDEX to change tablespace on
 the fly
In-Reply-To: <20200406184406.GF2228@telsasoft.com>
References: <20200327040106.GC20103@telsasoft.com>
 <20200328001112.GX20103@telsasoft.com>
 <f85b48beddc77c953f444cef06e1c16f@postgrespro.ru>
 <20200330183439.GQ20103@telsasoft.com>
 <624921b1ebca9897173d85c424c00646@postgrespro.ru>
 <20200401060334.GB142683@paquier.xyz> <20200401115718.GQ14618@telsasoft.com>
 <20200401130836.GT14618@telsasoft.com>
 <20200403182712.GR14618@telsasoft.com>
 <a0fd6af4bdf3cd209a69cea112ab10ea@postgrespro.ru>
 <20200406184406.GF2228@telsasoft.com>
User-Agent: Roundcube Webmail/1.4.0
Message-ID: <9b6a6cec134451550b74aa2ffd1446d0@postgrespro.ru>
Precedence: bulk

--=_3ea21e89d721c68f41576baeec11ff72
Content-Transfer-Encoding: 7bit
Content-Type: text/plain; charset=US-ASCII;
 format=flowed

On 2020-04-06 21:44, Justin Pryzby wrote:
> On Mon, Apr 06, 2020 at 08:43:46PM +0300, Alexey Kondratov wrote:
>> 
>> +/* XXX: reusing reindex_option_list */
>> +			| CLUSTER opt_verbose '(' reindex_option_list ')' qualified_name
>> cluster_index_specification
>> 
>> Could we actually simply reuse vac_analyze_option_list? From the first 
>> sight
>> it does just the right thing, excepting the special handling of 
>> spelling
>> ANALYZE/ANALYSE, but it does not seem to be a problem.
> 
> Hm, do you mean to let cluster.c reject the other options like 
> "analyze" ?
> I'm not sure why that would be better than reusing reindex?
> I think the suggestion will probably be to just copy+paste the reindex 
> option
> list and rename it to cluster (possibly with the explanation that 
> they're
> separate and independant and so their behavior shouldn't be tied 
> together).
> 

I mean to literally use vac_analyze_option_list for reindex and cluster 
as well. Please, check attached 0007. Now, vacuum, reindex and cluster 
filter options list and reject everything that is not supported, so it 
seems completely fine to just reuse vac_analyze_option_list, doesn't it?

>> 
>> ReindexRelationConcurrently is used for all cases, but it hits 
>> different
>> code paths in the case of database, table and index. I have not 
>> checked yet,
>> but are you sure it is safe removing these validations in the case of
>> REINDEX CONCURRENTLY?
> 
> You're right about the pg_global case, fixed.  System catalogs can't be
> reindexed CONCURRENTLY, so they're already caught by that check.
> 
>> > XXX: for cluster/vacuum, it might be more friendly to check before
>> > clustering
>> > the table, rather than after clustering and re-indexing.
>> 
>> Yes, I think it would be much more user-friendly.
> 
> I realized it's not needed or useful to check indexes in advance of 
> clustering,
> since 1) a mapped index will be on a mapped relation, which is already 
> checked;
> 2) a system index will be on a system relation.  Right ?
> 

Yes, it seems that you are right. I have tried to create user index on 
system relation with allow_system_table_mods=1, but this new index 
appeared to become system as well. That way, we do not have to check 
indexes in advance.


-- 
Alexey Kondratov

Postgres Professional https://www.postgrespro.com
Russian Postgres Company
--=_3ea21e89d721c68f41576baeec11ff72
Content-Transfer-Encoding: base64
Content-Type: text/x-diff;
 name=v18-0007-Reuse-vac_analyze_option_list-for-cluster-and-re.patch
Content-Disposition: attachment;
 filename=v18-0007-Reuse-vac_analyze_option_list-for-cluster-and-re.patch;
 size=2876

RnJvbSBhZmExMzI2NzRkMmI0OTMyOGE1ZTk1ZWNkMmZmMzBkODM4ZjdlYzZjIE1vbiBTZXAgMTcg
MDA6MDA6MDAgMjAwMQpGcm9tOiBBbGV4ZXkgS29uZHJhdG92IDxrb25kcmF0b3YuYWxla3NleUBn
bWFpbC5jb20+CkRhdGU6IFR1ZSwgNyBBcHIgMjAyMCAxMzo0Mjo0MiArMDMwMApTdWJqZWN0OiBb
UEFUQ0ggdjE4IDcvN10gUmV1c2UgdmFjX2FuYWx5emVfb3B0aW9uX2xpc3QgZm9yIGNsdXN0ZXIg
YW5kIHJlaW5kZXgKCi0tLQogc3JjL2JhY2tlbmQvcGFyc2VyL2dyYW0ueSB8IDM4ICsrKy0tLS0t
LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tCiAxIGZpbGUgY2hhbmdlZCwgMyBpbnNlcnRp
b25zKCspLCAzNSBkZWxldGlvbnMoLSkKCmRpZmYgLS1naXQgYS9zcmMvYmFja2VuZC9wYXJzZXIv
Z3JhbS55IGIvc3JjL2JhY2tlbmQvcGFyc2VyL2dyYW0ueQppbmRleCAzNjY1ZWU4NzAwLi43ODIx
ZjA3YzQwIDEwMDY0NAotLS0gYS9zcmMvYmFja2VuZC9wYXJzZXIvZ3JhbS55CisrKyBiL3NyYy9i
YWNrZW5kL3BhcnNlci9ncmFtLnkKQEAgLTUxMiwxMCArNTEyLDYgQEAgc3RhdGljIE5vZGUgKm1h
a2VSZWN1cnNpdmVWaWV3U2VsZWN0KGNoYXIgKnJlbG5hbWUsIExpc3QgKmFsaWFzZXMsIE5vZGUg
KnF1ZXJ5KTsKICV0eXBlIDxsaXN0PglleHBsYWluX29wdGlvbl9saXN0CiAKICV0eXBlIDxpdmFs
PglyZWluZGV4X3RhcmdldF90eXBlIHJlaW5kZXhfdGFyZ2V0X211bHRpdGFibGUKLSV0eXBlIDxz
dHI+CQlyZWluZGV4X29wdGlvbl9uYW1lCi0ldHlwZSA8bm9kZT4JcmVpbmRleF9vcHRpb25fYXJn
Ci0ldHlwZSA8bGlzdD4JcmVpbmRleF9vcHRpb25fbGlzdAotJXR5cGUgPGRlZmVsdD4JcmVpbmRl
eF9vcHRpb25fZWxlbQogCiAldHlwZSA8bm9kZT4JY29weV9nZW5lcmljX29wdF9hcmcgY29weV9n
ZW5lcmljX29wdF9hcmdfbGlzdF9pdGVtCiAldHlwZSA8ZGVmZWx0Pgljb3B5X2dlbmVyaWNfb3B0
X2VsZW0KQEAgLTg0MjgsNyArODQyNCw3IEBAIFJlaW5kZXhTdG10OgogCQkJCQluLT5wYXJhbXMg
PSBOSUw7CiAJCQkJCSQkID0gKE5vZGUgKiluOwogCQkJCX0KLQkJCXwgUkVJTkRFWCAnKCcgcmVp
bmRleF9vcHRpb25fbGlzdCAnKScgcmVpbmRleF90YXJnZXRfdHlwZSBvcHRfY29uY3VycmVudGx5
IHF1YWxpZmllZF9uYW1lCisJCQl8IFJFSU5ERVggJygnIHZhY19hbmFseXplX29wdGlvbl9saXN0
ICcpJyByZWluZGV4X3RhcmdldF90eXBlIG9wdF9jb25jdXJyZW50bHkgcXVhbGlmaWVkX25hbWUK
IAkJCQl7CiAJCQkJCVJlaW5kZXhTdG10ICpuID0gbWFrZU5vZGUoUmVpbmRleFN0bXQpOwogCQkJ
CQluLT5raW5kID0gJDU7CkBAIC04NDM4LDcgKzg0MzQsNyBAQCBSZWluZGV4U3RtdDoKIAkJCQkJ
bi0+cGFyYW1zID0gJDM7CiAJCQkJCSQkID0gKE5vZGUgKiluOwogCQkJCX0KLQkJCXwgUkVJTkRF
WCAnKCcgcmVpbmRleF9vcHRpb25fbGlzdCAnKScgcmVpbmRleF90YXJnZXRfbXVsdGl0YWJsZSBv
cHRfY29uY3VycmVudGx5IG5hbWUKKwkJCXwgUkVJTkRFWCAnKCcgdmFjX2FuYWx5emVfb3B0aW9u
X2xpc3QgJyknIHJlaW5kZXhfdGFyZ2V0X211bHRpdGFibGUgb3B0X2NvbmN1cnJlbnRseSBuYW1l
CiAJCQkJewogCQkJCQlSZWluZGV4U3RtdCAqbiA9IG1ha2VOb2RlKFJlaW5kZXhTdG10KTsKIAkJ
CQkJbi0+a2luZCA9ICQ1OwpAQCAtODQ1OCwzMyArODQ1NCw2IEBAIHJlaW5kZXhfdGFyZ2V0X211
bHRpdGFibGU6CiAJCQl8IFNZU1RFTV9QCQkJCXsgJCQgPSBSRUlOREVYX09CSkVDVF9TWVNURU07
IH0KIAkJCXwgREFUQUJBU0UJCQkJeyAkJCA9IFJFSU5ERVhfT0JKRUNUX0RBVEFCQVNFOyB9CiAJ
CTsKLXJlaW5kZXhfb3B0aW9uX2xpc3Q6Ci0JCQlyZWluZGV4X29wdGlvbl9lbGVtCi0JCQkJewot
CQkJCQkkJCA9IGxpc3RfbWFrZTEoJDEpOwotCQkJCX0KLQkJCXwgcmVpbmRleF9vcHRpb25fbGlz
dCAnLCcgcmVpbmRleF9vcHRpb25fZWxlbQotCQkJCXsKLQkJCQkJJCQgPSBsYXBwZW5kKCQxLCAk
Myk7Ci0JCQkJfQotCQk7Ci0KLXJlaW5kZXhfb3B0aW9uX2VsZW06Ci0JCQlyZWluZGV4X29wdGlv
bl9uYW1lIHJlaW5kZXhfb3B0aW9uX2FyZwotCQkJCXsKLQkJCQkJJCQgPSBtYWtlRGVmRWxlbSgk
MSwgJDIsIEAxKTsKLQkJCQl9Ci0JCTsKLQotcmVpbmRleF9vcHRpb25fbmFtZToKLQkJCU5vblJl
c2VydmVkV29yZAkJCQkJCQl7ICQkID0gJDE7IH0KLQkJOwotCi1yZWluZGV4X29wdGlvbl9hcmc6
Ci0JCQlvcHRfYm9vbGVhbl9vcl9zdHJpbmcJCQkJCXsgJCQgPSAoTm9kZSAqKSBtYWtlU3RyaW5n
KCQxKTsgfQotCQkJfCBOdW1lcmljT25seQkJCXsgJCQgPSAoTm9kZSAqKSAkMTsgfQotCQkJfCAv
KiBFTVBUWSAqLwkJIAkJCQkJeyAkJCA9IE5VTEw7IH0KLQkJOwogCiAvKioqKioqKioqKioqKioq
KioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioq
KioqKioKICAqCkBAIC0xMDY0Myw4ICsxMDYxMiw3IEBAIENsdXN0ZXJTdG10OgogCQkJCQluLT5w
YXJhbXMgPSBOSUw7CiAJCQkJCSQkID0gKE5vZGUqKW47CiAJCQkJfQotLyogWFhYOiByZXVzaW5n
IHJlaW5kZXhfb3B0aW9uX2xpc3QgKi8KLQkJCXwgQ0xVU1RFUiBvcHRfdmVyYm9zZSAnKCcgcmVp
bmRleF9vcHRpb25fbGlzdCAnKScgcXVhbGlmaWVkX25hbWUgY2x1c3Rlcl9pbmRleF9zcGVjaWZp
Y2F0aW9uCisJCQl8IENMVVNURVIgb3B0X3ZlcmJvc2UgJygnIHZhY19hbmFseXplX29wdGlvbl9s
aXN0ICcpJyBxdWFsaWZpZWRfbmFtZSBjbHVzdGVyX2luZGV4X3NwZWNpZmljYXRpb24KIAkJCQl7
CiAJCQkJCUNsdXN0ZXJTdG10ICpuID0gbWFrZU5vZGUoQ2x1c3RlclN0bXQpOwogCQkJCQluLT5y
ZWxhdGlvbiA9ICQ2OwotLSAKMi4xOS4xCgo=
--=_3ea21e89d721c68f41576baeec11ff72--