New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unify `FilteredQuerySource` and `SelectStatement` #760

Closed
sgrif opened this Issue Feb 24, 2017 · 0 comments

Comments

Projects
None yet
1 participant
@sgrif
Member

sgrif commented Feb 24, 2017

The reason FilteredQuerySource exists is basically to say "select was never called explicitly". This is because we want update(users.filter(id.eq(1))) to work, but not update(users.select(id)), since that's nonsense.

Right now, SelectStatement first comes into play at the AsQuery level. A table will implement AsQuery by doing SelectStatement::simple(all_columns, self). Rather than passing all_columns there, I'd like to introduce some sort of "default select clause" marker, and then only implement UpdateTarget for SelectStatement when the select clause is that marker.

sgrif added a commit that referenced this issue Feb 25, 2017

Unify `FilteredQuerySource` and `SelectStatement`
`FilteredQuerySource` only existed to make sure you couldn't pass a
query with a select clause or any other clause besides `WHERE` as a
target for `update` or `delete`. Instead of introducing a parallel
query type, we can instead just add the notion of a "default" select
clause.

I intentionally wanted this to be a voldemort type, so I needed to
rearrange the parameters on `SelectStatement` which caused a bit more
churn than I would have liked.

Close #760.

@sgrif sgrif closed this in 636ccd3 Mar 3, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment