Skip to content
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

Filter exception handling #12

Open
moschlar opened this issue May 21, 2013 · 8 comments
Open

Filter exception handling #12

moschlar opened this issue May 21, 2013 · 8 comments

Comments

@moschlar
Copy link
Contributor

I want to put this up for discussion:

Currently, if a specified filter leads to an exception in SQLAlchemy (e.g. on the default TG2 quickstart model with auth, create a CrudRestController for the User model class and search for group=0), the exception is not caught.
(Or another example: Depending on the backend, filtering an Integer column for a character (e.g. user_id=a, works on sqlite, doesn't work on postgres) will throw a DataError).

In my opinion, for an operation like filtering search results, this should only lead to an empty result set and maybe a warning flash to the user that he tried an illegal operation.

What do others think?

@amol-
Copy link
Member

amol- commented May 22, 2013

Search filtering has a bunch of issues currently. The first is the one you are reporting, with type matching. I actually think that in case of invalid filters (like characters for numbers), this can probably be easily detected by catching for ValueError when calling self.table_filler.get_value

The other is that, if I'm not wrong, searching for relations has many limits: Searching for related objects requires to write the id (when you would expected to write what you see, which is usually the view_names).

@moschlar
Copy link
Contributor Author

I'm gonna tackle them next ;)

@amol-
Copy link
Member

amol- commented Jun 14, 2013

I would like to tackle this issue for the upcoming release, do you have an already started patch or can I start from scratch?

@moschlar
Copy link
Contributor Author

Nope, I haven't started yet... :-/

@moschlar
Copy link
Contributor Author

I gave the first part of the problem a try.

Now there are some semantic inconsistencies between different database backends...
With sqlite, you can query an integer column by a string, it just doesn't return any rows; but on postgres, you will get a DataError from that...
If we catch the exception and discard the query, this will of course lead to different results.
To get it consistent, we could just return values = {} there.
Any thoughts?

@moschlar
Copy link
Contributor Author

Thinking some seconds about it, I think that in fact values = {} is a much better default...

@amol-
Copy link
Member

amol- commented Jun 21, 2013

I think that we can somehow achieve the expected behavior by using sprox validators detection to validate the search inputs. That should cope with issues with type and also cast searches according to the validators the user applied to the model. Would that be a solution?

@moschlar
Copy link
Contributor Author

Yeah, cool idea!

@moschlar moschlar reopened this Jun 24, 2013
amol- added a commit that referenced this issue Jun 25, 2013
Fix for #12 - Robustness of search field
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants