Content-Type: multipart/mixed; boundary="------------laxpcDUvuhPFPYcD0PYR5HQ9"
Message-ID: <24c11397-d9a1-4aac-a999-a79352a6a35b@gmail.com>
Date: Wed, 17 Jul 2024 10:28:58 +0700
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: Expand applicability of aggregate's sortop optimization
To: Matthias van de Meent <boekewurm+postgres@gmail.com>,
 PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
References: <CAEze2Wg-+EV4HdbQiut7X3KQd39xwmrpV4CeCmoJFFjH8cGdhw@mail.gmail.com>
Content-Language: en-AU
Cc: David Rowley <dgrowleyml@gmail.com>, ilmari@ilmari.org
From: Andrei Lepikhov <lepihov@gmail.com>
In-Reply-To: <CAEze2Wg-+EV4HdbQiut7X3KQd39xwmrpV4CeCmoJFFjH8cGdhw@mail.gmail.com>
Archived-At: <https://www.postgresql.org/message-id/24c11397-d9a1-4aac-a999-a79352a6a35b%40gmail.com>
Precedence: bulk

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

On 5/8/24 17:13, Matthias van de Meent wrote:
> As you may know, aggregates like SELECT MIN(unique1) FROM tenk1; are
> rewritten as SELECT unique1 FROM tenk1 ORDER BY unique1 USING < LIMIT
> 1; by using the optional sortop field in the aggregator.
> However, this optimization is disabled for clauses that in itself have
> an ORDER BY clause such as `MIN(unique1 ORDER BY <anything>), because
> <anything> can cause reordering of distinguisable values like 1.0 and
> 1.00, which then causes measurable differences in the output. In the
> general case, that's a good reason to not apply this optimization, but
> in some cases, we could still apply the index optimization.
Thanks for the job! I guess you could be more brave and push down also 
FILTER statement.
> 
> One of those cases is fixed in the attached patch: if we order by the
> same column that we're aggregating, using the same order class as the
> aggregate's sort operator (i.e. the aggregate's sortop is in the same
> btree opclass as the ORDER BY's sort operator), then we can still use
> the index operation: The sort behaviour isn't changed, thus we can
> apply the optimization.
> 
> PFA the small patch that implements this.
> 
> Note that we can't blindly accept just any ordering by the same
> column: If we had an opclass that sorted numeric values by the length
> of the significant/mantissa, then that'd provide a different (and
> distinct) ordering from that which is expected by the normal
> min()/max() aggregates for numeric, which could cause us to return
> arguably incorrect results for the aggregate expression.
As I see, the code:
aggsortop = fetch_agg_sort_op(aggref->aggfnoid);
if (!OidIsValid(aggsortop))
   return false;		/* not a MIN/MAX aggregate */

used twice and can be evaluated earlier to avoid duplicated code.

Also, I'm unsure about the necessity of looking through the btree 
classes. Maybe just to check the commutator to the sortop, like in the 
diff attached? Or could you provide an example to support your approach?

-- 
regards, Andrei Lepikhov

--------------laxpcDUvuhPFPYcD0PYR5HQ9
Content-Type: text/x-patch; charset=UTF-8; name="rewritten_patch.diff"
Content-Disposition: attachment; filename="rewritten_patch.diff"
Content-Transfer-Encoding: base64

ZGlmZiAtLWdpdCBhL3NyYy9iYWNrZW5kL29wdGltaXplci9wbGFuL3BsYW5hZ2cuYyBiL3Ny
Yy9iYWNrZW5kL29wdGltaXplci9wbGFuL3BsYW5hZ2cuYwppbmRleCBhZmI1NDQ1Yjc3Li5l
OWU5NWUyYjJhIDEwMDY0NAotLS0gYS9zcmMvYmFja2VuZC9vcHRpbWl6ZXIvcGxhbi9wbGFu
YWdnLmMKKysrIGIvc3JjL2JhY2tlbmQvb3B0aW1pemVyL3BsYW4vcGxhbmFnZy5jCkBAIC0y
NTMsNiArMjUzLDIwIEBAIGNhbl9taW5tYXhfYWdncyhQbGFubmVySW5mbyAqcm9vdCwgTGlz
dCAqKmNvbnRleHQpCiAJCWlmIChsaXN0X2xlbmd0aChhZ2dyZWYtPmFyZ3MpICE9IDEpCiAJ
CQlyZXR1cm4gZmFsc2U7CQkvKiBpdCBjb3VsZG4ndCBiZSBNSU4vTUFYICovCiAKKwkJLyoK
KwkJICogV2UgbWlnaHQgaW1wbGVtZW50IHRoZSBvcHRpbWl6YXRpb24gd2hlbiBhIEZJTFRF
UiBjbGF1c2UgaXMgcHJlc2VudAorCQkgKiBieSBhZGRpbmcgdGhlIGZpbHRlciB0byB0aGUg
cXVhbHMgb2YgdGhlIGdlbmVyYXRlZCBzdWJxdWVyeS4gIEZvcgorCQkgKiBub3csIGp1c3Qg
cHVudC4KKwkJICovCisJCWlmIChhZ2dyZWYtPmFnZ2ZpbHRlciAhPSBOVUxMKQorCQkJcmV0
dXJuIGZhbHNlOworCisJCWN1clRhcmdldCA9IChUYXJnZXRFbnRyeSAqKSBsaW5pdGlhbChh
Z2dyZWYtPmFyZ3MpOworCisJCWFnZ3NvcnRvcCA9IGZldGNoX2FnZ19zb3J0X29wKGFnZ3Jl
Zi0+YWdnZm5vaWQpOworCQlpZiAoIU9pZElzVmFsaWQoYWdnc29ydG9wKSkKKwkJCXJldHVy
biBmYWxzZTsJCS8qIG5vdCBhIE1JTi9NQVggYWdncmVnYXRlICovCisKIAkJLyoKIAkJICog
T1JERVIgQlkgaXMgdXN1YWxseSBpcnJlbGV2YW50IGZvciBNSU4vTUFYLCBidXQgaXQgY2Fu
IGNoYW5nZSB0aGUKIAkJICogb3V0Y29tZSBpZiB0aGUgYWdnc29ydG9wJ3Mgb3BlcmF0b3Ig
Y2xhc3MgcmVjb2duaXplcyBub24taWRlbnRpY2FsCkBAIC0yNjcsMjIgKzI4MSwzNyBAQCBj
YW5fbWlubWF4X2FnZ3MoUGxhbm5lckluZm8gKnJvb3QsIExpc3QgKipjb250ZXh0KQogCQkg
KiBxdWlja2x5LgogCQkgKi8KIAkJaWYgKGFnZ3JlZi0+YWdnb3JkZXIgIT0gTklMKQotCQkJ
cmV0dXJuIGZhbHNlOwotCQkvKiBub3RlOiB3ZSBkbyBub3QgY2FyZSBpZiBESVNUSU5DVCBp
cyBtZW50aW9uZWQgLi4uICovCi0KLQkJLyoKLQkJICogV2UgbWlnaHQgaW1wbGVtZW50IHRo
ZSBvcHRpbWl6YXRpb24gd2hlbiBhIEZJTFRFUiBjbGF1c2UgaXMgcHJlc2VudAotCQkgKiBi
eSBhZGRpbmcgdGhlIGZpbHRlciB0byB0aGUgcXVhbHMgb2YgdGhlIGdlbmVyYXRlZCBzdWJx
dWVyeS4gIEZvcgotCQkgKiBub3csIGp1c3QgcHVudC4KLQkJICovCi0JCWlmIChhZ2dyZWYt
PmFnZ2ZpbHRlciAhPSBOVUxMKQotCQkJcmV0dXJuIGZhbHNlOworCQl7CisJCQlTb3J0R3Jv
dXBDbGF1c2UgKm9yZGVyQ2xhdXNlOworCisJCQkvKgorCQkJICogSWYgdGhlIG9yZGVyIGNs
YXVzZSBpcyB0aGUgc2FtZSBjb2x1bW4gYXMgdGhlIG9uZSB3ZSdyZQorCQkJICogYWdncmVn
YXRpbmcsIHdlIGNhbiBzdGlsbCB1c2UgdGhlIGluZGV4OiBJdCBpcyB1bmRlZmluZWQgd2hp
Y2gKKwkJCSAqIHZhbHVlIGlzIE1JTigpIG9yIE1BWCgpLCBhcyB3ZWxsIGFzIHdoaWNoIHZh
bHVlIGlzIGZpcnN0IG9yCisJCQkgKiBsYXN0IHdoZW4gc29ydGVkLiBTbywgd2UgY2FuIHN0
aWxsIHVzZSB0aGUgaW5kZXggSUZGIHRoZQorCQkJICogYWdncmVnYXRlZCBleHByZXNzaW9u
IGVxdWFscyB0aGUgZXhwcmVzc2lvbiB1c2VkIGluIHRoZQorCQkJICogb3JkZXJpbmcgb3Bl
cmF0aW9uLgorCQkJICovCisKKwkJCS8qCisJCQkgKiBXZSBvbmx5IGFjY2VwdCBhIHNpbmds
ZSBhcmd1bWVudCB0byBtaW4vbWF4IGFnZ3JlZ2F0ZXMsCisJCQkgKiBvcmRlcmluZ3MgdGhh
dCBoYXZlIG1vcmUgY2xhdXNlcyB3b24ndCBwcm92aWRlIGNvcnJlY3QgcmVzdWx0cy4KKwkJ
CSAqLworCQkJaWYgKGxpc3RfbGVuZ3RoKGFnZ3JlZi0+YWdnb3JkZXIpID4gMSkKKwkJCQly
ZXR1cm4gZmFsc2U7CisKKwkJCW9yZGVyQ2xhdXNlID0gY2FzdE5vZGUoU29ydEdyb3VwQ2xh
dXNlLCBsaW5pdGlhbChhZ2dyZWYtPmFnZ29yZGVyKSk7CisKKwkJCWlmIChvcmRlckNsYXVz
ZS0+dGxlU29ydEdyb3VwUmVmICE9IGN1clRhcmdldC0+cmVzc29ydGdyb3VwcmVmKQorCQkJ
CXJldHVybiBmYWxzZTsKKworCQkJaWYgKG9yZGVyQ2xhdXNlLT5zb3J0b3AgIT0gYWdnc29y
dG9wICYmCisJCQkJb3JkZXJDbGF1c2UtPnNvcnRvcCAhPSBnZXRfY29tbXV0YXRvcihhZ2dz
b3J0b3ApKQorCQkJCXJldHVybiBmYWxzZTsKKwkJfQogCi0JCWFnZ3NvcnRvcCA9IGZldGNo
X2FnZ19zb3J0X29wKGFnZ3JlZi0+YWdnZm5vaWQpOwotCQlpZiAoIU9pZElzVmFsaWQoYWdn
c29ydG9wKSkKLQkJCXJldHVybiBmYWxzZTsJCS8qIG5vdCBhIE1JTi9NQVggYWdncmVnYXRl
ICovCisJCS8qIG5vdGU6IHdlIGRvIG5vdCBjYXJlIGlmIERJU1RJTkNUIGlzIG1lbnRpb25l
ZCAuLi4gKi8KIAotCQljdXJUYXJnZXQgPSAoVGFyZ2V0RW50cnkgKikgbGluaXRpYWwoYWdn
cmVmLT5hcmdzKTsKIAogCQlpZiAoY29udGFpbl9tdXRhYmxlX2Z1bmN0aW9ucygoTm9kZSAq
KSBjdXJUYXJnZXQtPmV4cHIpKQogCQkJcmV0dXJuIGZhbHNlOwkJLyogbm90IHBvdGVudGlh
bGx5IGluZGV4YWJsZSAqLwpkaWZmIC0tZ2l0IGEvc3JjL3Rlc3QvcmVncmVzcy9leHBlY3Rl
ZC9hZ2dyZWdhdGVzLm91dCBiL3NyYy90ZXN0L3JlZ3Jlc3MvZXhwZWN0ZWQvYWdncmVnYXRl
cy5vdXQKaW5kZXggYTU1OTZhYjIxMC4uNWQzNzUxMGQ2NCAxMDA2NDQKLS0tIGEvc3JjL3Rl
c3QvcmVncmVzcy9leHBlY3RlZC9hZ2dyZWdhdGVzLm91dAorKysgYi9zcmMvdGVzdC9yZWdy
ZXNzL2V4cGVjdGVkL2FnZ3JlZ2F0ZXMub3V0CkBAIC0xMDAzLDYgKzEwMDMsNzEgQEAgc2Vs
ZWN0IG1heCh1bmlxdWUxKSBmcm9tIHRlbmsxIHdoZXJlIHVuaXF1ZTEgPiA0MjsKICA5OTk5
CiAoMSByb3cpCiAKKy0tIFdoZW4gc29ydGluZyBvbiB0aGUgY29sdW1uIHRoYXQncyBiZWlu
ZyBhZ2dyZWdhdGVkLCBpbmRleGVzIGNhbiBhbHNvIGJlCistLSB1c2VkLCBidXQgb25seSB3
aGVuIHRoZSBhZ2dyZWdhdGUncyBvcGVyYXRvciBoYXMgdGhlIHNhbWUgb3JkZXJpbmcgYmVo
YXZpb3IKKy0tIGFzIHRoZSBPUkRFUiBCWS1jbGF1c2UsIGkuZS4gaWYgaXQgaXMgaW4gdGhl
IHNhbWUgYnRyZWUgb3BjbGFzcyBhcyB0aGUgb25lCistLSBjaG9zZW4gZm9yIHRoZSBPUkRF
UiBCWSBjbGF1c2UuCitleHBsYWluIChjb3N0cyBvZmYpCisgIHNlbGVjdCBtaW4odW5pcXVl
MSBPUkRFUiBCWSB1bmlxdWUxIEFTQyBOVUxMUyBMQVNUKSBmcm9tIHRlbmsxOworICAgICAg
ICAgICAgICAgICAgICAgICAgIFFVRVJZIFBMQU4gICAgICAgICAgICAgICAgICAgICAgICAg
CistLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0t
LS0tLS0tLS0KKyBSZXN1bHQKKyAgIEluaXRQbGFuIDEKKyAgICAgLT4gIExpbWl0CisgICAg
ICAgICAgIC0+ICBJbmRleCBPbmx5IFNjYW4gdXNpbmcgdGVuazFfdW5pcXVlMSBvbiB0ZW5r
MQorICAgICAgICAgICAgICAgICBJbmRleCBDb25kOiAodW5pcXVlMSBJUyBOT1QgTlVMTCkK
Kyg1IHJvd3MpCisKK3NlbGVjdCBtaW4odW5pcXVlMSBPUkRFUiBCWSB1bmlxdWUxIEFTQyBO
VUxMUyBMQVNUKSBmcm9tIHRlbmsxOworIG1pbiAKKy0tLS0tCisgICAwCisoMSByb3cpCisK
K2V4cGxhaW4gKGNvc3RzIG9mZikKKyAgc2VsZWN0IG1heCh1bmlxdWUxIE9SREVSIEJZIHVu
aXF1ZTEgVVNJTkcgPCkgZnJvbSB0ZW5rMTsKKyAgICAgICAgICAgICAgICAgICAgICAgICAg
ICAgUVVFUlkgUExBTiAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIAorLS0tLS0tLS0t
LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0t
LS0tLS0tCisgUmVzdWx0CisgICBJbml0UGxhbiAxCisgICAgIC0+ICBMaW1pdAorICAgICAg
ICAgICAtPiAgSW5kZXggT25seSBTY2FuIEJhY2t3YXJkIHVzaW5nIHRlbmsxX3VuaXF1ZTEg
b24gdGVuazEKKyAgICAgICAgICAgICAgICAgSW5kZXggQ29uZDogKHVuaXF1ZTEgSVMgTk9U
IE5VTEwpCisoNSByb3dzKQorCitzZWxlY3QgbWF4KHVuaXF1ZTEgT1JERVIgQlkgdW5pcXVl
MSBVU0lORyA8KSBmcm9tIHRlbmsxOworIG1heCAgCistLS0tLS0KKyA5OTk5CisoMSByb3cp
CisKK2V4cGxhaW4gKGNvc3RzIG9mZikKKyAgc2VsZWN0IG1heCh1bmlxdWUxIE9SREVSIEJZ
IHRlbnRob3VzKSBmcm9tIHRlbmsxOworICAgICAgICAgIFFVRVJZIFBMQU4gICAgICAgICAg
IAorLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLQorIEFnZ3JlZ2F0ZQorICAgLT4g
IFNvcnQKKyAgICAgICAgIFNvcnQgS2V5OiB0ZW50aG91cworICAgICAgICAgLT4gIFNlcSBT
Y2FuIG9uIHRlbmsxCisoNCByb3dzKQorCitzZWxlY3QgbWF4KHVuaXF1ZTEgT1JERVIgQlkg
dGVudGhvdXMpIGZyb20gdGVuazE7CisgbWF4ICAKKy0tLS0tLQorIDk5OTkKKygxIHJvdykK
KworLS0gIEJ1dCBldmVuIHRoZW4sIHRoZSBpbmRleCBjYW4ndCBiZSB1c2VkIGlmIHdlIG9y
ZGVyIGJ5IG11bHRpcGxlIGNvbHVtbnMuCitleHBsYWluIChjb3N0cyBvZmYpCisgIHNlbGVj
dCBtYXgodW5pcXVlMSBPUkRFUiBCWSB1bmlxdWUxLCB0ZW50aG91cykgZnJvbSB0ZW5rMTsK
KyAgICAgICAgICAgICBRVUVSWSBQTEFOICAgICAgICAgICAgICAKKy0tLS0tLS0tLS0tLS0t
LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0KKyBBZ2dyZWdhdGUKKyAgIC0+ICBTb3J0CisgICAg
ICAgICBTb3J0IEtleTogdW5pcXVlMSwgdGVudGhvdXMKKyAgICAgICAgIC0+ICBTZXEgU2Nh
biBvbiB0ZW5rMQorKDQgcm93cykKKwogLS0gdGhlIHBsYW5uZXIgbWF5IGNob29zZSBhIGdl
bmVyaWMgYWdncmVnYXRlIGhlcmUgaWYgcGFyYWxsZWwgcXVlcnkgaXMKIC0tIGVuYWJsZWQs
IHNpbmNlIHRoYXQgcGxhbiB3aWxsIGJlIHBhcmFsbGVsIHNhZmUgYW5kIHRoZSAib3B0aW1p
emVkIgogLS0gcGxhbiwgd2hpY2ggaGFzIGFsbW9zdCBpZGVudGljYWwgY29zdCwgd2lsbCBu
b3QgYmUuICB3ZSB3YW50IHRvIHRlc3QKZGlmZiAtLWdpdCBhL3NyYy90ZXN0L3JlZ3Jlc3Mv
c3FsL2FnZ3JlZ2F0ZXMuc3FsIGIvc3JjL3Rlc3QvcmVncmVzcy9zcWwvYWdncmVnYXRlcy5z
cWwKaW5kZXggY2E2ZDFiY2ZiNy4uNWNkZjMzNmZjMCAxMDA2NDQKLS0tIGEvc3JjL3Rlc3Qv
cmVncmVzcy9zcWwvYWdncmVnYXRlcy5zcWwKKysrIGIvc3JjL3Rlc3QvcmVncmVzcy9zcWwv
YWdncmVnYXRlcy5zcWwKQEAgLTM2OCw2ICszNjgsMjQgQEAgZXhwbGFpbiAoY29zdHMgb2Zm
KQogICBzZWxlY3QgbWF4KHVuaXF1ZTEpIGZyb20gdGVuazEgd2hlcmUgdW5pcXVlMSA+IDQy
Owogc2VsZWN0IG1heCh1bmlxdWUxKSBmcm9tIHRlbmsxIHdoZXJlIHVuaXF1ZTEgPiA0MjsK
IAorLS0gV2hlbiBzb3J0aW5nIG9uIHRoZSBjb2x1bW4gdGhhdCdzIGJlaW5nIGFnZ3JlZ2F0
ZWQsIGluZGV4ZXMgY2FuIGFsc28gYmUKKy0tIHVzZWQsIGJ1dCBvbmx5IHdoZW4gdGhlIGFn
Z3JlZ2F0ZSdzIG9wZXJhdG9yIGhhcyB0aGUgc2FtZSBvcmRlcmluZyBiZWhhdmlvcgorLS0g
YXMgdGhlIE9SREVSIEJZLWNsYXVzZSwgaS5lLiBpZiBpdCBpcyBpbiB0aGUgc2FtZSBidHJl
ZSBvcGNsYXNzIGFzIHRoZSBvbmUKKy0tIGNob3NlbiBmb3IgdGhlIE9SREVSIEJZIGNsYXVz
ZS4KK2V4cGxhaW4gKGNvc3RzIG9mZikKKyAgc2VsZWN0IG1pbih1bmlxdWUxIE9SREVSIEJZ
IHVuaXF1ZTEgQVNDIE5VTExTIExBU1QpIGZyb20gdGVuazE7CitzZWxlY3QgbWluKHVuaXF1
ZTEgT1JERVIgQlkgdW5pcXVlMSBBU0MgTlVMTFMgTEFTVCkgZnJvbSB0ZW5rMTsKK2V4cGxh
aW4gKGNvc3RzIG9mZikKKyAgc2VsZWN0IG1heCh1bmlxdWUxIE9SREVSIEJZIHVuaXF1ZTEg
VVNJTkcgPCkgZnJvbSB0ZW5rMTsKK3NlbGVjdCBtYXgodW5pcXVlMSBPUkRFUiBCWSB1bmlx
dWUxIFVTSU5HIDwpIGZyb20gdGVuazE7CitleHBsYWluIChjb3N0cyBvZmYpCisgIHNlbGVj
dCBtYXgodW5pcXVlMSBPUkRFUiBCWSB0ZW50aG91cykgZnJvbSB0ZW5rMTsKK3NlbGVjdCBt
YXgodW5pcXVlMSBPUkRFUiBCWSB0ZW50aG91cykgZnJvbSB0ZW5rMTsKKworLS0gIEJ1dCBl
dmVuIHRoZW4sIHRoZSBpbmRleCBjYW4ndCBiZSB1c2VkIGlmIHdlIG9yZGVyIGJ5IG11bHRp
cGxlIGNvbHVtbnMuCitleHBsYWluIChjb3N0cyBvZmYpCisgIHNlbGVjdCBtYXgodW5pcXVl
MSBPUkRFUiBCWSB1bmlxdWUxLCB0ZW50aG91cykgZnJvbSB0ZW5rMTsKKwogLS0gdGhlIHBs
YW5uZXIgbWF5IGNob29zZSBhIGdlbmVyaWMgYWdncmVnYXRlIGhlcmUgaWYgcGFyYWxsZWwg
cXVlcnkgaXMKIC0tIGVuYWJsZWQsIHNpbmNlIHRoYXQgcGxhbiB3aWxsIGJlIHBhcmFsbGVs
IHNhZmUgYW5kIHRoZSAib3B0aW1pemVkIgogLS0gcGxhbiwgd2hpY2ggaGFzIGFsbW9zdCBp
ZGVudGljYWwgY29zdCwgd2lsbCBub3QgYmUuICB3ZSB3YW50IHRvIHRlc3QK

--------------laxpcDUvuhPFPYcD0PYR5HQ9--