Dynamic clauses with Ecto.Query?

I am working with a legacy database which stores extra sql clauses in the database on a per customer basis that need to be added to further filter when selecting rows from another table for that customer.

The obvious approach to try is with fragment. Given a query that is set up retrieve data from the table I’m interested in as well as an extra_criteria variable string that would be something as simple as price > 200000 or city="Minneapolis", or a much more complex expression.

from(query, where: fragment(^extra_criteria))

But of course, Ecto doesn’t let me do this since it doesn’t allow strings to be interpolated as the first parameter to fragment to avoid sql injection attacks.

Since the extra_criteria data is not coming from end-users, but has been created by employees I do trust it…

Anybody have any advice on how to add these extra clause(s) to my Ecto.Query? I noticed there was going to be an unsafe_fragment added awhile back which never ended up being added – was there another alternative available?

I have also considered just building raw sql myself, but I’d really like like to use all the other nice filtering and pagination I have already set up in the system that work on Ecto.Query's.

Thanks :slight_smile:

What about dynamic/N is insufficient for what you are needing to do though? It allows you to build up dynamic conditions and filters. :slight_smile:

It could be I’m not using dynamic/2 correctly.

It gets me close. Given my previous example extra_criteria of "price > 200000"

conditions = dynamic([q], ^extra_criteria)
from query, where: ^conditions

This does not throw an error but ends up sending the entire extra_criteria as a query parameter. You end up running a query in this form:

SELECT f0.`field1`, f0.`field2` 
FROM `db`.`mytable` AS f0 
WHERE (?) 
ORDER BY f0.`id` ["price > 200000"]

Which returns no results. I have double checked that the equivalent query with the price > 20000 clause added directly to the where clause is returning results.

Because that is exactly what it should do since you are passing it in as a query argument via ^extra_criteria. :slight_smile:

Remember that SQL queries are cached, anything passed to a query argument, as with the normal SQL interfaces, ‘is’ an entire argument, it is not SQL and not inferred as SQL. Allowing something like price > 200000 as raw SQL is just begging for someone to accidentally mis-type something and suddenly wipe your whole database, that is why every SQL interface (not just with Ecto, I mean base native interfaces) highly encourage using prepared statements with arguments, not just for an efficiency boost but also for safety.

Thus you need to encode your request some other way, like say if you want to encode a comparison on a field then encode it as a tuple like {:>, :price, 200000}, then you can generate your query like:

query = Enum.reduce(all_filters, query, fn
  {:>, field, value} -> where(query, [q], field(q, ^field) > value)
  {:==, field, value} -> where(query, [q], field(q, ^field) == value)
  {:<, field, value} -> where(query, [q], field(q, ^field) < value)
  {:!=, field, value} -> where(query, [q], field(q, ^field) != value)
end)
...etc...

And of course you can make something more specialized for your input data. For example here is a copy/paste from one of my big generators:

       {:active_around, %NaiveDateTime{} = dt}, squery ->
          where(
            squery,
            [course, section, dept],
            section.ssbsect_ptrm_start_date <= ^dt and section.ssbsect_ptrm_end_date >= ^dt
          )

        {:registered, true}, squery ->
          where(
            squery,
            [course, section, dept, student_course],
            student_course.sfrstcr_rsts_code in ["RA", "RE", "RW"]
          )

        {:withdrawn, true}, squery ->
          where(
            squery,
            [course, section, dept, student_course],
            student_course.sfrstcr_rsts_code == "WD"
          )

        {:department, dept_code}, squery when is_binary(dept_code) ->
          where(squery, [course, section, dept], course.scbcrse_dept_code == ^dept_code)

        {:department, dept_code}, squery when is_list(dept_code) ->
          where(squery, [course, section, dept], course.scbcrse_dept_code in ^dept_code)

        {:subject, subject_code}, squery when is_binary(subject_code) ->
          where(squery, [course, section, dept], section.ssbsect_subj_code == ^subject_code)

        {:subject, subject_code}, squery when is_list(subject_code) ->
          where(squery, [course, section, dept], section.ssbsect_subj_code in ^subject_code)

        {:course, course_number}, squery when is_binary(course_number) ->
          where(squery, [course, section, dept], section.ssbsect_crse_numb == ^course_number)

        {:course, course_number}, squery when is_list(course_number) ->
          where(squery, [course, section, dept], section.ssbsect_crse_numb in ^course_number)

And I have a whole ton more than that in that one function (it’s a huge general lookup function). There is no way I would ever allow a user-defined filter to be injected into SQL. Even assuming no malice, it would be simple to accidentally craft something that could grab data they shouldn’t get, or could wipe the database, etc…

Remember the Golden Rule of front-end programming: Don’t Trust User Input.

^.^

4 Likes

Thanks for the detailed reply :slight_smile:

I agree with you 100%. The ability to compose queries with Elixir/Ecto based on filters as you’ve demonstrated is phenomenal and we have very similar code as well that handles all filters based on user input.

The ‘user input’ I was referring to in my initial post, however, was created by programmers - it’s just that it is coming from a row in a database, rather than the source code. I wouldn’t have set it up that way, but that is how this system is.

After some thought, however, I agree with the decision that was made to not allow this. Even if my situation is a (debatable I know) valid use case, if available that functionality would invariably be abused.

We’ll probably end up re-encoding all those filters as a long-term project.

1 Like