Bug: Empty queries do not trigger common filters when using multitenancy

[niphlod]

see web2py/web2py#1181 .
I'd love to trash db().select(db.table.ALL) but it it stays there at least there should be a note on the book.

@mdipierro , @gi0baro , @michele-comitini, @abastardi : thoughts ?

[gi0baro]

@niphlod I'm for drop this feature and update the book. We should also warn users for backward compatibility breaks.

If other developers agree with this, I can include this in the major refactoring I'm working on, will publish a PR in the next week probably.

[niphlod]

drop = discontinue plus a note on the book about multi-tenancy working with Query but not Set IMHO is the way to go. It's not the first one coming in reporting the issue

[abastardi]

What feature are you talking about dropping/discontinuing?

[niphlod]

quoting myself

"""
I'd love to trash db().select(db.table.ALL) but if it stays there at least there should be a note on the book
"""

I really don't get why supporting db().select(fields) when it clearly creates side-problems like this one. And I can't see the usecase.
Everyone should just use everywhere db(something_i_want).select(fields).
I'm quite convinced that db().select(fields) crept in on apps just to type a few less characters than the - frankly not that heavy - requirement of issuing a select statement without a where condition.
A much cleaner approach (I know, in retrospect it's far too easy) would have been to support a select_naive() for those so adamant in supporting query-less select()s.
offtopic: same goes for db(blabla).select().first() instead of the really less performance-hungry db(blabla).select(limitby=(0,1)) ....
a db(blabla).select_first() would have been surely more helpful in restraining typed characters while issuing sane queries to the backend.

[abastardi]

If we don't want to fix it, maybe db().select(...) should raise an exception when there are common_filters in place, so at least it won't create a security problem. In other words, we can declare that no-query operations are incompatible with common_filters and simply not allow the combination.

[gi0baro]

@niphlod can I assign this to you?

[gi0baro]

@niphlod I mean the drop of db().select(db.table.ALL) logic from the codebase.

[niphlod]

huh ? did I win ? already ? without discussions ? if yes, count me in, I definitely WANT to be blamed for the removal. Don't think though that I won: @mdipierro , @abastardi , @michele-comitini final vote ?

[mdipierro]

We should not break backward compatibility. This is easy to fix.
On Apr 20, 2016 4:36 AM, "niphlod" [email protected] wrote:

huh ? did I win ? already ? without discussions ? if yes, count me in, I
definitely WANT to be blamed for the removal. Don't think though that I
won: @mdipierro https://github.com/mdipierro , @abastardi
https://github.com/abastardi , @michele-comitini
https://github.com/michele-comitini final vote ?

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#327 (comment)

[gi0baro]

@mdipierro the point here is not how it would be easy to fix, the point is to voluntarily keep a syntax that is "semantically" different from everything else and that produce side problems.

I can't see the point of keeping it while you can just tell people that they should move to the correct syntax.

[mdipierro]

It is not semantically different. db(db.table).select(db.select.field) would be a redundant syntax. There should be no need to specify a table in the query if we specify it in the select(....).

[gi0baro]

Still, can't see the point of writing db().select(db.table.ALL) instead of writing db(db.table).select(), especially since I have to write everything like db(some_query).select().
And again, the first causes problems.