allow .distinct() to specify a column for DISTINCT ON

[williamflaherty]

As the title says, would be great if you could specify a column in TableName.select().distinct() to handle distinct on or something similar?

I started poking around to add it myself but got stopped up by order_by needing to be in the same order issues from asyncpg. (I'm far from a PostgreSQL expert).

[dantownsend]

This would be a nice feature.

Looking at the docs it seems like the column specified in the DISTINCT ON must also be the first column specified in the ORDER BY.

So in this example, location:

SELECT DISTINCT ON (location) location, time, report
    FROM weather_reports
    ORDER BY location, time DESC;

This might be why asyncpg is complaining.

Feel free to open a PR if you want - doesn't have to be perfect.

[sinisaos]

@dantownsend Quite the opposite, I wanted to make as big a distinction as possible between the distinct and distinct_on methods and that's why I made two methods because distinct and distinct_on are quite different clauses. In distinct_on, the first column in self.columns_delegate.selected_columns is always an expression in DISTINCT_ON clause.

If we have both distinct and distinct_on we can only allow one of them.

We can have only one clause, not both

select = (
    "SELECT DISTINCT" # distinct
    if self.distinct_delegate._distinct
    else "SELECT DISTINCT ON" # distinct_on
    if self.distinct_on_delegate._distinct_on
    else "SELECT" # normal select
)

It's somehow more understandable to me with two different methods, but if you thought that the implementation should be different, I'll delete the branch.

[dantownsend]

@sinisaos I see why you did it that way, and it's valid solution.

I like to try and mirror the Postgres docs as much as possible, and even though DISTINCT ON and DISTINCT do quite different things, in the docs ON is more of an option for DISTINCT, which is why I like the approach of adding an argument to the distinct method.

In your branch you've made it very easy to use by not having to specify the column, but with DISTINCT ON you can specify multiple columns, so we need to let the user specify it otherwise we're not exposing all of the functionality.

[sinisaos]

In your branch you've made it very easy to use by not having to specify the column, but with DISTINCT ON you can specify multiple columns, so we need to let the user specify it otherwise we're not exposing all of the functionality.

Now you can also specify multiple columns like this

Piccolo:
Band.select(Band.name, Band.popularity, #or any other column#)
.distinct_on().order_by(Band.name) 

SQL:
SELECT DISTINCT ON (name  # this is self.columns_delegate.selected_columns #) 
name, popularity FROM band ORDER BY name;

Now you can also specify multiple order_by or no order_by at all.

@dantownsend No problem. Thanks for explaining what you think about the implementation, so I'll leave it up to you or someone else. Thank you.

[dantownsend]

@sinisaos It's just a matter of preference - what you've done it great. But I think making the user explicitly specify everything makes it work for more edge cases, but has the downside of being less convenient. Don't delete what you've done - I'll have a play around with it, and will create a PR based off it with this alternative approach so you can see what I'm suggesting.

[dantownsend]

Here's a draft PR: #813

[dantownsend]

@williamflaherty We've made some progress on this: #813

I know you already did some thinking about it, so would appreciate your thoughts on the current solution.

I've sent you an invite with the triage role, otherwise I can't mention you in the PR. It's a strange limitation of GitHub 🤦