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 1nSFTd-0004Fw-SA for pgsql-hackers@arkaria.postgresql.org; Thu, 10 Mar 2022 09:52:58 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.92) (envelope-from ) id 1nSFTc-0000wY-P3 for pgsql-hackers@arkaria.postgresql.org; Thu, 10 Mar 2022 09:52:56 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1nSFTc-0000wO-04 for pgsql-hackers@lists.postgresql.org; Thu, 10 Mar 2022 09:52:56 +0000 Received: from out4-smtp.messagingengine.com ([66.111.4.28]) by magus.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1nSFTW-0000Ek-Bq for pgsql-hackers@postgresql.org; Thu, 10 Mar 2022 09:52:55 +0000 Received: from compute3.internal (compute3.nyi.internal [10.202.2.43]) by mailout.nyi.internal (Postfix) with ESMTP id EDC405C02B8; Thu, 10 Mar 2022 04:52:45 -0500 (EST) Received: from mailfrontend2 ([10.202.2.163]) by compute3.internal (MEProxy); Thu, 10 Mar 2022 04:52:45 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:date:date:from:from :in-reply-to:in-reply-to:message-id:mime-version:references :reply-to:sender:subject:subject:to:to:x-me-proxy:x-me-proxy :x-me-sender:x-me-sender:x-sasl-enc; s=fm2; bh=wUaZ0JyVIlMiLpw9G 4FQcQxJU22Bgnk9CDQwBA2DYec=; b=YVn26ROcn6CD3njitTs+uzE5+ybjzp0xZ /Zeb1lFQJ5wxY66XKFLutmWPrE2myfLR1V9bytNJx+m2sB/JO31bV0G/9GJZdK94 gBkAcHUtVouD3U9MS/p4ya2vwx332G8ck3w67bEup3EauQl8B7dnFG8EFRp3Umk/ 4bCaC0ButDDpNK+eDrmbjdOiuaMQqoL8Z5LKLmjbZ2Xei/F6AS53C9KtTr/PjMmy QFRmLjr8+7jXdj9o0Zil8vg5vmdq4MYeJOsK4ZNQWz540+c1wALZPdZ6+01PhHrs qgPtMXCov9HRaE232SC90DJHcOTsdHDyeyiM6BHek6Ul6fjOQzmBg== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvvddruddvtddgtdelucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurheptgfkffggfgfuvfhfhfgjsehmtderredtfeejnecuhfhrohhmpefrvghtvghr ucfgihhsvghnthhrrghuthcuoehpvghtvghrrdgvihhsvghnthhrrghuthesvghnthgvrh hprhhishgvuggsrdgtohhmqeenucggtffrrghtthgvrhhnpedutdevledtvedvteduvdef vdfhieetkeelvdfhhfejfeejvdeujeeggeefudegtdenucevlhhushhtvghrufhiiigvpe dtnecurfgrrhgrmhepmhgrihhlfhhrohhmpehpvghtvghrrdgvihhsvghnthhrrghuthes vghnthgvrhhprhhishgvuggsrdgtohhm X-ME-Proxy: Received: by mail.messagingengine.com (Postfix) with ESMTPA; Thu, 10 Mar 2022 04:52:44 -0500 (EST) Content-Type: multipart/mixed; boundary="------------85Z0Z4MxhgBeTgHwtuFN1Zp9" Message-ID: <29ae752f-80e9-8d31-601c-62cf01cc93d8@enterprisedb.com> Date: Thu, 10 Mar 2022 10:52:41 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0) Gecko/20100101 Thunderbird/91.6.2 Subject: Re: ICU for global collation Content-Language: en-US To: Julien Rouhaud Cc: pgsql-hackers , Daniel Verite References: <20220118040246.dewl5pain2gkv2iv@jrouhaud> <171c5f7c-43ee-dbe9-31f9-5e3eb06a54cf@enterprisedb.com> <20220121135119.ywszwl4353v5dzh5@jrouhaud> <20220121161331.xn7fhtfg5j2ucilb@jrouhaud> <87e93704-b7f7-71f6-161d-8713e6334ed3@enterprisedb.com> <704405ed-f1fa-d2a7-b9f9-48a07fb45648@enterprisedb.com> <96a50803-72ba-8eec-948a-d87608ac15a0@enterprisedb.com> <20220305083830.lpz3k3yku5lmm5xs@jrouhaud> From: Peter Eisentraut In-Reply-To: <20220305083830.lpz3k3yku5lmm5xs@jrouhaud> List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk This is a multi-part message in MIME format. --------------85Z0Z4MxhgBeTgHwtuFN1Zp9 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit On 05.03.22 09:38, Julien Rouhaud wrote: > @@ -168,18 +175,6 @@ DefineCollation(ParseState *pstate, List *names, List *parameters, bool if_not_e > errmsg("collation \"default\" cannot be copied"))); > } > > - if (localeEl) > - { > - collcollate = defGetString(localeEl); > - collctype = defGetString(localeEl); > - } > [...] > > I tried to read the function and quickly got confused about whether possible > problematic conditions could be reached or not and had protection or not. I > think that DefineCollation is becoming more and more unreadable, with a mix of > fromEl, !fromEl and either. Maybe it would be a good time to refactor the code > a bit and have have something like > > if (fromEl) > { > // initialize all specific things > } > else > { > // initialize all specific things > } > > // handle defaults and sanity checks > > What do you think? How about the attached? --------------85Z0Z4MxhgBeTgHwtuFN1Zp9 Content-Type: text/plain; charset=UTF-8; name="0001-DefineCollation-code-cleanup.patch" Content-Disposition: attachment; filename="0001-DefineCollation-code-cleanup.patch" Content-Transfer-Encoding: base64 RnJvbSBjN2I0MDhiYTY0NmM1ZWU1ZjhjNmFlODRiZWMwNGNhNDA1OWVkMDhmIE1vbiBTZXAg MTcgMDA6MDA6MDAgMjAwMQpGcm9tOiBQZXRlciBFaXNlbnRyYXV0IDxwZXRlckBlaXNlbnRy YXV0Lm9yZz4KRGF0ZTogVGh1LCAxMCBNYXIgMjAyMiAxMDo0OTo0OSArMDEwMApTdWJqZWN0 OiBbUEFUQ0hdIERlZmluZUNvbGxhdGlvbigpIGNvZGUgY2xlYW51cAoKUmVvcmdhbml6ZSB0 aGUgY29kZSBpbiBEZWZpbmVDb2xsYXRpb24oKSBzbyB0aGF0IHRoZSBwYXJ0cyB1c2luZyB0 aGUKRlJPTSBjbGF1c2UgYW5kIHRoZSBwYXJ0cyBub3QgZG9pbmcgc28gYXJlIG1vcmUgY2xl YW5seSBzZXBhcmF0ZWQuICBObwpmdW5jdGlvbmFsaXR5IGNoYW5nZSBpbnRlbmRlZC4KClJl cG9ydGVkLWJ5OiBKdWxpZW4gUm91aGF1ZCA8cmp1anUxMjNAZ21haWwuY29tPgotLS0KIHNy Yy9iYWNrZW5kL2NvbW1hbmRzL2NvbGxhdGlvbmNtZHMuYyB8IDEwOSArKysrKysrKysrKysr Ky0tLS0tLS0tLS0tLS0KIDEgZmlsZSBjaGFuZ2VkLCA1NyBpbnNlcnRpb25zKCspLCA1MiBk ZWxldGlvbnMoLSkKCmRpZmYgLS1naXQgYS9zcmMvYmFja2VuZC9jb21tYW5kcy9jb2xsYXRp b25jbWRzLmMgYi9zcmMvYmFja2VuZC9jb21tYW5kcy9jb2xsYXRpb25jbWRzLmMKaW5kZXgg MTJmYzIzMTZmOS4uOTNkZjFkMzY2YyAxMDA2NDQKLS0tIGEvc3JjL2JhY2tlbmQvY29tbWFu ZHMvY29sbGF0aW9uY21kcy5jCisrKyBiL3NyYy9iYWNrZW5kL2NvbW1hbmRzL2NvbGxhdGlv bmNtZHMuYwpAQCAtNjMsMTIgKzYzLDExIEBAIERlZmluZUNvbGxhdGlvbihQYXJzZVN0YXRl ICpwc3RhdGUsIExpc3QgKm5hbWVzLCBMaXN0ICpwYXJhbWV0ZXJzLCBib29sIGlmX25vdF9l CiAJRGVmRWxlbSAgICAqcHJvdmlkZXJFbCA9IE5VTEw7CiAJRGVmRWxlbSAgICAqZGV0ZXJt aW5pc3RpY0VsID0gTlVMTDsKIAlEZWZFbGVtICAgICp2ZXJzaW9uRWwgPSBOVUxMOwotCWNo YXIJICAgKmNvbGxjb2xsYXRlID0gTlVMTDsKLQljaGFyCSAgICpjb2xsY3R5cGUgPSBOVUxM OwotCWNoYXIJICAgKmNvbGxwcm92aWRlcnN0ciA9IE5VTEw7Ci0JYm9vbAkJY29sbGlzZGV0 ZXJtaW5pc3RpYyA9IHRydWU7Ci0JaW50CQkJY29sbGVuY29kaW5nID0gMDsKLQljaGFyCQlj b2xscHJvdmlkZXIgPSAwOworCWNoYXIJICAgKmNvbGxjb2xsYXRlOworCWNoYXIJICAgKmNv bGxjdHlwZTsKKwlib29sCQljb2xsaXNkZXRlcm1pbmlzdGljOworCWludAkJCWNvbGxlbmNv ZGluZzsKKwljaGFyCQljb2xscHJvdmlkZXI7CiAJY2hhcgkgICAqY29sbHZlcnNpb24gPSBO VUxMOwogCU9pZAkJCW5ld29pZDsKIAlPYmplY3RBZGRyZXNzIGFkZHJlc3M7CkBAIC0xNjcs NjUgKzE2Niw3MSBAQCBEZWZpbmVDb2xsYXRpb24oUGFyc2VTdGF0ZSAqcHN0YXRlLCBMaXN0 ICpuYW1lcywgTGlzdCAqcGFyYW1ldGVycywgYm9vbCBpZl9ub3RfZQogCQkJCQkoZXJyY29k ZShFUlJDT0RFX0lOVkFMSURfT0JKRUNUX0RFRklOSVRJT04pLAogCQkJCQkgZXJybXNnKCJj b2xsYXRpb24gXCJkZWZhdWx0XCIgY2Fubm90IGJlIGNvcGllZCIpKSk7CiAJfQotCi0JaWYg KGxvY2FsZUVsKQorCWVsc2UKIAl7Ci0JCWNvbGxjb2xsYXRlID0gZGVmR2V0U3RyaW5nKGxv Y2FsZUVsKTsKLQkJY29sbGN0eXBlID0gZGVmR2V0U3RyaW5nKGxvY2FsZUVsKTsKLQl9CisJ CWNoYXIJICAgKmNvbGxwcm92aWRlcnN0ciA9IE5VTEw7CiAKLQlpZiAobGNjb2xsYXRlRWwp Ci0JCWNvbGxjb2xsYXRlID0gZGVmR2V0U3RyaW5nKGxjY29sbGF0ZUVsKTsKKwkJY29sbGNv bGxhdGUgPSBOVUxMOworCQljb2xsY3R5cGUgPSBOVUxMOwogCi0JaWYgKGxjY3R5cGVFbCkK LQkJY29sbGN0eXBlID0gZGVmR2V0U3RyaW5nKGxjY3R5cGVFbCk7CisJCWlmIChsb2NhbGVF bCkKKwkJeworCQkJY29sbGNvbGxhdGUgPSBkZWZHZXRTdHJpbmcobG9jYWxlRWwpOworCQkJ Y29sbGN0eXBlID0gZGVmR2V0U3RyaW5nKGxvY2FsZUVsKTsKKwkJfQogCi0JaWYgKHByb3Zp ZGVyRWwpCi0JCWNvbGxwcm92aWRlcnN0ciA9IGRlZkdldFN0cmluZyhwcm92aWRlckVsKTsK KwkJaWYgKGxjY29sbGF0ZUVsKQorCQkJY29sbGNvbGxhdGUgPSBkZWZHZXRTdHJpbmcobGNj b2xsYXRlRWwpOwogCi0JaWYgKGRldGVybWluaXN0aWNFbCkKLQkJY29sbGlzZGV0ZXJtaW5p c3RpYyA9IGRlZkdldEJvb2xlYW4oZGV0ZXJtaW5pc3RpY0VsKTsKKwkJaWYgKGxjY3R5cGVF bCkKKwkJCWNvbGxjdHlwZSA9IGRlZkdldFN0cmluZyhsY2N0eXBlRWwpOwogCi0JaWYgKHZl cnNpb25FbCkKLQkJY29sbHZlcnNpb24gPSBkZWZHZXRTdHJpbmcodmVyc2lvbkVsKTsKKwkJ aWYgKHByb3ZpZGVyRWwpCisJCQljb2xscHJvdmlkZXJzdHIgPSBkZWZHZXRTdHJpbmcocHJv dmlkZXJFbCk7CiAKLQlpZiAoY29sbHByb3ZpZGVyc3RyKQotCXsKLQkJaWYgKHBnX3N0cmNh c2VjbXAoY29sbHByb3ZpZGVyc3RyLCAiaWN1IikgPT0gMCkKLQkJCWNvbGxwcm92aWRlciA9 IENPTExQUk9WSURFUl9JQ1U7Ci0JCWVsc2UgaWYgKHBnX3N0cmNhc2VjbXAoY29sbHByb3Zp ZGVyc3RyLCAibGliYyIpID09IDApCi0JCQljb2xscHJvdmlkZXIgPSBDT0xMUFJPVklERVJf TElCQzsKKwkJaWYgKGRldGVybWluaXN0aWNFbCkKKwkJCWNvbGxpc2RldGVybWluaXN0aWMg PSBkZWZHZXRCb29sZWFuKGRldGVybWluaXN0aWNFbCk7CisJCWVsc2UKKwkJCWNvbGxpc2Rl dGVybWluaXN0aWMgPSB0cnVlOworCisJCWlmICh2ZXJzaW9uRWwpCisJCQljb2xsdmVyc2lv biA9IGRlZkdldFN0cmluZyh2ZXJzaW9uRWwpOworCisJCWlmIChjb2xscHJvdmlkZXJzdHIp CisJCXsKKwkJCWlmIChwZ19zdHJjYXNlY21wKGNvbGxwcm92aWRlcnN0ciwgImljdSIpID09 IDApCisJCQkJY29sbHByb3ZpZGVyID0gQ09MTFBST1ZJREVSX0lDVTsKKwkJCWVsc2UgaWYg KHBnX3N0cmNhc2VjbXAoY29sbHByb3ZpZGVyc3RyLCAibGliYyIpID09IDApCisJCQkJY29s bHByb3ZpZGVyID0gQ09MTFBST1ZJREVSX0xJQkM7CisJCQllbHNlCisJCQkJZXJlcG9ydChF UlJPUiwKKwkJCQkJCShlcnJjb2RlKEVSUkNPREVfSU5WQUxJRF9PQkpFQ1RfREVGSU5JVElP TiksCisJCQkJCQkgZXJybXNnKCJ1bnJlY29nbml6ZWQgY29sbGF0aW9uIHByb3ZpZGVyOiAl cyIsCisJCQkJCQkJCWNvbGxwcm92aWRlcnN0cikpKTsKKwkJfQogCQllbHNlCisJCQljb2xs cHJvdmlkZXIgPSBDT0xMUFJPVklERVJfTElCQzsKKworCQlpZiAoIWNvbGxjb2xsYXRlKQog CQkJZXJlcG9ydChFUlJPUiwKIAkJCQkJKGVycmNvZGUoRVJSQ09ERV9JTlZBTElEX09CSkVD VF9ERUZJTklUSU9OKSwKLQkJCQkJIGVycm1zZygidW5yZWNvZ25pemVkIGNvbGxhdGlvbiBw cm92aWRlcjogJXMiLAotCQkJCQkJCWNvbGxwcm92aWRlcnN0cikpKTsKLQl9Ci0JZWxzZSBp ZiAoIWZyb21FbCkKLQkJY29sbHByb3ZpZGVyID0gQ09MTFBST1ZJREVSX0xJQkM7Ci0KLQlp ZiAoIWNvbGxjb2xsYXRlKQotCQllcmVwb3J0KEVSUk9SLAotCQkJCShlcnJjb2RlKEVSUkNP REVfSU5WQUxJRF9PQkpFQ1RfREVGSU5JVElPTiksCi0JCQkJIGVycm1zZygicGFyYW1ldGVy IFwibGNfY29sbGF0ZVwiIG11c3QgYmUgc3BlY2lmaWVkIikpKTsKKwkJCQkJIGVycm1zZygi cGFyYW1ldGVyIFwibGNfY29sbGF0ZVwiIG11c3QgYmUgc3BlY2lmaWVkIikpKTsKIAotCWlm ICghY29sbGN0eXBlKQotCQllcmVwb3J0KEVSUk9SLAotCQkJCShlcnJjb2RlKEVSUkNPREVf SU5WQUxJRF9PQkpFQ1RfREVGSU5JVElPTiksCi0JCQkJIGVycm1zZygicGFyYW1ldGVyIFwi bGNfY3R5cGVcIiBtdXN0IGJlIHNwZWNpZmllZCIpKSk7CisJCWlmICghY29sbGN0eXBlKQor CQkJZXJlcG9ydChFUlJPUiwKKwkJCQkJKGVycmNvZGUoRVJSQ09ERV9JTlZBTElEX09CSkVD VF9ERUZJTklUSU9OKSwKKwkJCQkJIGVycm1zZygicGFyYW1ldGVyIFwibGNfY3R5cGVcIiBt dXN0IGJlIHNwZWNpZmllZCIpKSk7CiAKLQkvKgotCSAqIE5vbmRldGVybWluaXN0aWMgY29s bGF0aW9ucyBhcmUgY3VycmVudGx5IG9ubHkgc3VwcG9ydGVkIHdpdGggSUNVCi0JICogYmVj YXVzZSB0aGF0J3MgdGhlIG9ubHkgY2FzZSB3aGVyZSBpdCBjYW4gYWN0dWFsbHkgbWFrZSBh IGRpZmZlcmVuY2UuCi0JICogU28gd2UgY2FuIHNhdmUgd3JpdGluZyB0aGUgY29kZSBmb3Ig dGhlIG90aGVyIHByb3ZpZGVycy4KLQkgKi8KLQlpZiAoIWNvbGxpc2RldGVybWluaXN0aWMg JiYgY29sbHByb3ZpZGVyICE9IENPTExQUk9WSURFUl9JQ1UpCi0JCWVyZXBvcnQoRVJST1Is Ci0JCQkJKGVycmNvZGUoRVJSQ09ERV9GRUFUVVJFX05PVF9TVVBQT1JURUQpLAotCQkJCSBl cnJtc2coIm5vbmRldGVybWluaXN0aWMgY29sbGF0aW9ucyBub3Qgc3VwcG9ydGVkIHdpdGgg dGhpcyBwcm92aWRlciIpKSk7CisJCS8qCisJCSAqIE5vbmRldGVybWluaXN0aWMgY29sbGF0 aW9ucyBhcmUgY3VycmVudGx5IG9ubHkgc3VwcG9ydGVkIHdpdGggSUNVCisJCSAqIGJlY2F1 c2UgdGhhdCdzIHRoZSBvbmx5IGNhc2Ugd2hlcmUgaXQgY2FuIGFjdHVhbGx5IG1ha2UgYSBk aWZmZXJlbmNlLgorCQkgKiBTbyB3ZSBjYW4gc2F2ZSB3cml0aW5nIHRoZSBjb2RlIGZvciB0 aGUgb3RoZXIgcHJvdmlkZXJzLgorCQkgKi8KKwkJaWYgKCFjb2xsaXNkZXRlcm1pbmlzdGlj ICYmIGNvbGxwcm92aWRlciAhPSBDT0xMUFJPVklERVJfSUNVKQorCQkJZXJlcG9ydChFUlJP UiwKKwkJCQkJKGVycmNvZGUoRVJSQ09ERV9GRUFUVVJFX05PVF9TVVBQT1JURUQpLAorCQkJ CQkgZXJybXNnKCJub25kZXRlcm1pbmlzdGljIGNvbGxhdGlvbnMgbm90IHN1cHBvcnRlZCB3 aXRoIHRoaXMgcHJvdmlkZXIiKSkpOwogCi0JaWYgKCFmcm9tRWwpCi0JewogCQlpZiAoY29s bHByb3ZpZGVyID09IENPTExQUk9WSURFUl9JQ1UpCiAJCXsKICNpZmRlZiBVU0VfSUNVCi0t IAoyLjM1LjEKCg== --------------85Z0Z4MxhgBeTgHwtuFN1Zp9--