Content-Type: multipart/mixed; boundary="------------B4vtcdY0tnfN1dF8yJkkveYK"
Message-ID: <e7a4d391-bfc5-4226-88fa-9f35c1d2fbdc@gmail.com>
Date: Thu, 14 Nov 2024 19:01:49 +0700
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: Reordering DISTINCT keys to match input path's pathkeys
To: Richard Guo <guofenglinux@gmail.com>
Cc: PostgreSQL-development <pgsql-hackers@postgresql.org>,
 David Rowley <dgrowleyml@gmail.com>
References: 
 <CAMbWs48dR26cCcX0f=8bja2JKQPcU64136kHk=xekHT9xschiQ@mail.gmail.com>
 <CAApHDvq39jp0EXbtu0Kwx7um_x54MiFcKx2sNj9kdTc9TEYu7g@mail.gmail.com>
 <CAMbWs4-+gBN4sQ7wyEptrsKO1AxJHOocxWQ72=bX1g1=9cOrSA@mail.gmail.com>
 <CAMbWs48j2xnk3WpO55dJU-4yoSMRb56=qf=zKPfb5driTwH-Yg@mail.gmail.com>
 <CAMbWs49gL-c=xBuQTQcVSMjsAnUhDt1-x6BZHkNskvcVdZ94gw@mail.gmail.com>
 <0fc11a92-f6e4-4d8c-b882-91283ef54bce@gmail.com>
 <CAMbWs4-SHEBahuQCo7-4yK4=CmEP6jd5RwKbGUB2akFB79mCCQ@mail.gmail.com>
 <e56a0dbd-c969-4a69-b5c3-b640bc8ebc8a@gmail.com>
 <CAMbWs4_mC81kpuHTABY2BQup+6ZFSYDPMD9RWykFuNA1WXBajw@mail.gmail.com>
Content-Language: en-AU
From: Andrei Lepikhov <lepihov@gmail.com>
In-Reply-To: 
 <CAMbWs4_mC81kpuHTABY2BQup+6ZFSYDPMD9RWykFuNA1WXBajw@mail.gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/e7a4d391-bfc5-4226-88fa-9f35c1d2fbdc%40gmail.com>
Precedence: bulk

This is a multi-part message in MIME format.
--------------B4vtcdY0tnfN1dF8yJkkveYK
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit

On 11/14/24 08:09, Richard Guo wrote:
> On Wed, Nov 13, 2024 at 7:36 PM Andrei Lepikhov <lepihov@gmail.com> wrote:
>> On 11/13/24 13:49, Richard Guo wrote:
>> In thread [1], I try to add one more strategy that minimises the number
>> of comparison operator calls. It seems that it would work the same way
>> with the DISTINCT statement. Do you think it make sense in general and
>> can be a possible direction of improvement for the current patch?
> 
> I haven’t had a chance to follow that thread.  From a quick look at
> that patch, it seems to improve the general costing logic for sorting.
> If that’s the case, I think it would be beneficial in the areas where
> we use cost_sort(), including in this patch.
Yes, the core of the discussion is cost calculation. It is a base for 
the final patch that adds a grouping strategy, according to which it may 
be profitable to put the column with max distinct value at the first 
position to make sorting less expensive. I think it makes sense to do 
the same in this DISTINCT feature, too (as a further improvement, of 
course).

>> disable features during severe slowdowns or bugs. It might make sense to
>> group them into a single 'Clause Reordering' parameter.
> code.  If these GUCs are mainly for debugging, I think it's better to
> keep them separate so that we can debug each optimization individually.
Ok.
See minor changes I propose in the attachment.

-- 
regards, Andrei Lepikhov
--------------B4vtcdY0tnfN1dF8yJkkveYK
Content-Type: text/x-patch; charset=UTF-8; name="minor-fix.diff"
Content-Disposition: attachment; filename="minor-fix.diff"
Content-Transfer-Encoding: base64

ZGlmZiAtLWdpdCBhL3NyYy9iYWNrZW5kL29wdGltaXplci9wYXRoL3BhdGhrZXlzLmMgYi9z
cmMvYmFja2VuZC9vcHRpbWl6ZXIvcGF0aC9wYXRoa2V5cy5jCmluZGV4IDk5MDYzZDA1MDYu
LjkwMDZhMTRhYmMgMTAwNjQ0Ci0tLSBhL3NyYy9iYWNrZW5kL29wdGltaXplci9wYXRoL3Bh
dGhrZXlzLmMKKysrIGIvc3JjL2JhY2tlbmQvb3B0aW1pemVyL3BhdGgvcGF0aGtleXMuYwpA
QCAtMjI4Nyw3ICsyMjg3LDYgQEAgc3RhdGljIGludAogcGF0aGtleXNfdXNlZnVsX2Zvcl9k
aXN0aW5jdChQbGFubmVySW5mbyAqcm9vdCwgTGlzdCAqcGF0aGtleXMpCiB7CiAJaW50CQkJ
bl9jb21tb25fcGF0aGtleXM7Ci0JTGlzdENlbGwgICAqbGM7CiAKIAkvKgogCSAqIGRpc3Rp
bmN0X3BhdGhrZXlzIG1heSBoYXZlIGJlY29tZSBlbXB0eSBpZiBhbGwgb2YgdGhlIHBhdGhr
ZXlzIHdlcmUKQEAgLTIyOTgsMTAgKzIyOTcsOCBAQCBwYXRoa2V5c191c2VmdWxfZm9yX2Rp
c3RpbmN0KFBsYW5uZXJJbmZvICpyb290LCBMaXN0ICpwYXRoa2V5cykKIAogCS8qIHdhbGsg
dGhlIHBhdGhrZXlzIGFuZCBzZWFyY2ggZm9yIG1hdGNoaW5nIERJU1RJTkNUIGtleSAqLwog
CW5fY29tbW9uX3BhdGhrZXlzID0gMDsKLQlmb3JlYWNoKGxjLCBwYXRoa2V5cykKKwlmb3Jl
YWNoX25vZGUoUGF0aEtleSwgcGF0aGtleSwgcGF0aGtleXMpCiAJewotCQlQYXRoS2V5ICAg
ICpwYXRoa2V5ID0gKFBhdGhLZXkgKikgbGZpcnN0KGxjKTsKLQogCQkvKiBubyBtYXRjaGlu
ZyBESVNUSU5DVCBrZXksIHdlJ3JlIGRvbmUgKi8KIAkJaWYgKCFsaXN0X21lbWJlcl9wdHIo
cm9vdC0+ZGlzdGluY3RfcGF0aGtleXMsIHBhdGhrZXkpKQogCQkJYnJlYWs7CmRpZmYgLS1n
aXQgYS9zcmMvYmFja2VuZC9vcHRpbWl6ZXIvcGxhbi9wbGFubmVyLmMgYi9zcmMvYmFja2Vu
ZC9vcHRpbWl6ZXIvcGxhbi9wbGFubmVyLmMKaW5kZXggMjg3MGVmNThmMC4uYzIyZTQxZTgz
YiAxMDA2NDQKLS0tIGEvc3JjL2JhY2tlbmQvb3B0aW1pemVyL3BsYW4vcGxhbm5lci5jCisr
KyBiL3NyYy9iYWNrZW5kL29wdGltaXplci9wbGFuL3BsYW5uZXIuYwpAQCAtNTE1OCw3ICs1
MTU4LDYgQEAgY3JlYXRlX2ZpbmFsX2Rpc3RpbmN0X3BhdGhzKFBsYW5uZXJJbmZvICpyb290
LCBSZWxPcHRJbmZvICppbnB1dF9yZWwsCiAJCQlQYXRoCSAgICppbnB1dF9wYXRoID0gKFBh
dGggKikgbGZpcnN0KGxjKTsKIAkJCVBhdGgJICAgKnNvcnRlZF9wYXRoOwogCQkJTGlzdAkg
ICAqdXNlZnVsX3BhdGhrZXlzX2xpc3QgPSBOSUw7Ci0JCQlMaXN0Q2VsbCAgICpsYzI7CiAK
IAkJCXVzZWZ1bF9wYXRoa2V5c19saXN0ID0KIAkJCQlnZXRfdXNlZnVsX3BhdGhrZXlzX2Zv
cl9kaXN0aW5jdChyb290LApAQCAtNTE2NiwxMCArNTE2NSw4IEBAIGNyZWF0ZV9maW5hbF9k
aXN0aW5jdF9wYXRocyhQbGFubmVySW5mbyAqcm9vdCwgUmVsT3B0SW5mbyAqaW5wdXRfcmVs
LAogCQkJCQkJCQkJCQkJIGlucHV0X3BhdGgtPnBhdGhrZXlzKTsKIAkJCUFzc2VydChsaXN0
X2xlbmd0aCh1c2VmdWxfcGF0aGtleXNfbGlzdCkgPiAwKTsKIAotCQkJZm9yZWFjaChsYzIs
IHVzZWZ1bF9wYXRoa2V5c19saXN0KQorCQkJZm9yZWFjaF9ub2RlKExpc3QsIHVzZWZ1bF9w
YXRoa2V5cywgdXNlZnVsX3BhdGhrZXlzX2xpc3QpCiAJCQl7Ci0JCQkJTGlzdAkgICAqdXNl
ZnVsX3BhdGhrZXlzID0gKExpc3QgKikgbGZpcnN0KGxjMik7Ci0KIAkJCQlzb3J0ZWRfcGF0
aCA9IG1ha2Vfb3JkZXJlZF9wYXRoKHJvb3QsCiAJCQkJCQkJCQkJCQlkaXN0aW5jdF9yZWws
CiAJCQkJCQkJCQkJCQlpbnB1dF9wYXRoLApAQCAtNTI3NiwxMSArNTI3Myw5IEBAIGdldF91
c2VmdWxfcGF0aGtleXNfZm9yX2Rpc3RpbmN0KFBsYW5uZXJJbmZvICpyb290LCBMaXN0ICpu
ZWVkZWRfcGF0aGtleXMsCiB7CiAJTGlzdAkgICAqdXNlZnVsX3BhdGhrZXlzX2xpc3QgPSBO
SUw7CiAJTGlzdAkgICAqdXNlZnVsX3BhdGhrZXlzID0gTklMOwotCUxpc3RDZWxsICAgKmxj
OwogCiAJLyogYWx3YXlzIGluY2x1ZGUgdGhlIGdpdmVuICduZWVkZWRfcGF0aGtleXMnICov
Ci0JdXNlZnVsX3BhdGhrZXlzX2xpc3QgPSBsYXBwZW5kKHVzZWZ1bF9wYXRoa2V5c19saXN0
LAotCQkJCQkJCQkgICBuZWVkZWRfcGF0aGtleXMpOworCXVzZWZ1bF9wYXRoa2V5c19saXN0
ID0gbGFwcGVuZCh1c2VmdWxfcGF0aGtleXNfbGlzdCwgbmVlZGVkX3BhdGhrZXlzKTsKIAog
CWlmICghZW5hYmxlX2Rpc3RpbmN0X3Jlb3JkZXJpbmcpCiAJCXJldHVybiB1c2VmdWxfcGF0
aGtleXNfbGlzdDsKQEAgLTUyOTQsMTAgKzUyODksOCBAQCBnZXRfdXNlZnVsX3BhdGhrZXlz
X2Zvcl9kaXN0aW5jdChQbGFubmVySW5mbyAqcm9vdCwgTGlzdCAqbmVlZGVkX3BhdGhrZXlz
LAogCSAqIGxpc3QgbWF0Y2hlcyBpbml0aWFsIGRpc3RpbmN0Q2xhdXNlIHBhdGhrZXlzOyBv
dGhlcndpc2UsIGl0IHdvbid0IGhhdmUKIAkgKiB0aGUgZGVzaXJlZCBiZWhhdmlvci4KIAkg
Ki8KLQlmb3JlYWNoKGxjLCBwYXRoX3BhdGhrZXlzKQorCWZvcmVhY2hfbm9kZShQYXRoS2V5
LCBwYXRoa2V5LCBwYXRoX3BhdGhrZXlzKQogCXsKLQkJUGF0aEtleSAgICAqcGF0aGtleSA9
IChQYXRoS2V5ICopIGxmaXJzdChsYyk7Ci0KIAkJLyoKIAkJICogVGhlIFBhdGhLZXkgbm9k
ZXMgYXJlIGNhbm9uaWNhbCwgc28gdGhleSBjYW4gYmUgY2hlY2tlZCBmb3IKIAkJICogZXF1
YWxpdHkgYnkgc2ltcGxlIHBvaW50ZXIgY29tcGFyaXNvbi4KQEAgLTUzMjQsMTkgKzUzMTcs
MTYgQEAgZ2V0X3VzZWZ1bF9wYXRoa2V5c19mb3JfZGlzdGluY3QoUGxhbm5lckluZm8gKnJv
b3QsIExpc3QgKm5lZWRlZF9wYXRoa2V5cywKIAkJcmV0dXJuIHVzZWZ1bF9wYXRoa2V5c19s
aXN0OwogCiAJLyogQXBwZW5kIHRoZSByZW1haW5pbmcgUGF0aEtleSBub2RlcyBpbiBuZWVk
ZWRfcGF0aGtleXMgKi8KLQl1c2VmdWxfcGF0aGtleXMgPSBsaXN0X2NvbmNhdF91bmlxdWVf
cHRyKHVzZWZ1bF9wYXRoa2V5cywKLQkJCQkJCQkJCQkJIG5lZWRlZF9wYXRoa2V5cyk7CisJ
dXNlZnVsX3BhdGhrZXlzID0gbGlzdF9jb25jYXRfdW5pcXVlX3B0cih1c2VmdWxfcGF0aGtl
eXMsIG5lZWRlZF9wYXRoa2V5cyk7CiAKIAkvKgogCSAqIElmIHRoZSByZXN1bHRpbmcgcGF0
aGtleSBsaXN0IGlzIHRoZSBzYW1lIGFzIHRoZSAnbmVlZGVkX3BhdGhrZXlzJywKIAkgKiBq
dXN0IGRyb3AgaXQuCiAJICovCi0JaWYgKGNvbXBhcmVfcGF0aGtleXMobmVlZGVkX3BhdGhr
ZXlzLAotCQkJCQkJIHVzZWZ1bF9wYXRoa2V5cykgPT0gUEFUSEtFWVNfRVFVQUwpCisJaWYg
KGNvbXBhcmVfcGF0aGtleXMobmVlZGVkX3BhdGhrZXlzLCB1c2VmdWxfcGF0aGtleXMpID09
IFBBVEhLRVlTX0VRVUFMKQogCQlyZXR1cm4gdXNlZnVsX3BhdGhrZXlzX2xpc3Q7CiAKLQl1
c2VmdWxfcGF0aGtleXNfbGlzdCA9IGxhcHBlbmQodXNlZnVsX3BhdGhrZXlzX2xpc3QsCi0J
CQkJCQkJCSAgIHVzZWZ1bF9wYXRoa2V5cyk7CisJdXNlZnVsX3BhdGhrZXlzX2xpc3QgPSBs
YXBwZW5kKHVzZWZ1bF9wYXRoa2V5c19saXN0LCB1c2VmdWxfcGF0aGtleXMpOwogCiAJcmV0
dXJuIHVzZWZ1bF9wYXRoa2V5c19saXN0OwogfQo=

--------------B4vtcdY0tnfN1dF8yJkkveYK--