On Tue, Jan 23, 2024 at 5:03 PM David Rowley <dgrowle...@gmail.com> wrote:
> I've not caught up on the specifics of 0452b461b, but I just wanted to > highlight that there was some work done in [1] in this area. It seems > Ankit didn't ever add that to a CF, so that might explain why it's > been lost. > > Anyway, just pointing it out as there may be useful code or discussion > in the corresponding threads. Thanks for pointing it out. I looked at the patch there and noticed several problems with it. * That patch is incomplete and does not work as expected. It at least needs to modify truncate_useless_pathkeys() to account for DISTINCT clause (I think this has been mentioned in that thread). * That patch would not consider the origin DISTINCT pathkeys if it could do some reordering, which is not great and can generate inefficient plans. For instance (after fixing the first problem) create table t (a int, b int); create index on t(a); set enable_hashagg to off; set enable_incremental_sort to off; set enable_seqscan to off; explain (costs off) select distinct b, a from t order by b, a; QUERY PLAN ------------------------------------------------- Sort Sort Key: b, a -> Unique -> Sort Sort Key: a, b -> Index Scan using t_a_idx on t (6 rows) Using DISTINCT pathkeys {b, a} is more efficient for this plan, because only one Sort would be required. But that patch is not able to do that, because it does not consider the origin DISTINCT pathkeys after reordering. Thanks Richard