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

Potential bug in numeric handling where_clause for filters #1681

Open
simonw opened this issue Mar 22, 2022 · 2 comments
Open

Potential bug in numeric handling where_clause for filters #1681

simonw opened this issue Mar 22, 2022 · 2 comments

Comments

@simonw
Copy link
Owner

simonw commented Mar 22, 2022

Note that Datasette does already have special logic to convert parameters to integers for numeric comparisons like >:

def where_clause(self, table, column, value, param_counter):
converted = self.format.format(value)
if self.numeric and converted.isdigit():
converted = int(converted)
if self.no_argument:
kwargs = {"c": column}
converted = None
else:
kwargs = {"c": column, "p": f"p{param_counter}", "t": table}
return self.sql_template.format(**kwargs), converted

Though... it looks like there's a bug in that? It doesn't account for float values - "3.5".isdigit() return False - probably for the best, because int(3.5) would break that value anyway.

Originally posted by @simonw in #1671 (comment)

@simonw
Copy link
Owner Author

simonw commented Mar 22, 2022

My hunch is that this mechanism doesn't actually do anything useful at all, because of the type conversion that automatically happens for data from tables based on the column type affinities, see:

So either remove the self.numeric type conversion bit entirely, or prove that it is necessary and upgrade it to be able to handle floating point values too.

@simonw
Copy link
Owner Author

simonw commented Mar 22, 2022

I would expect this to break against SQL views that include calculated columns though - something like this:

create view this_will_break as select pk + 1 as pk_plus_one, 0.5 as score from searchable;

Confirmed: the filter interface for that view plain doesn't work for any comparison against that table - except for score > 0 since 0 is converted to an integer. 0.1 breaks though because it doesn't get converted as it doesn't match .isdigit().

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

No branches or pull requests

1 participant