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

Allow numbers in literal/1 #4562

Merged

Conversation

greg-rychlewski
Copy link
Member

@greg-rychlewski greg-rychlewski commented Jan 8, 2025

There was a discussion where someone was using a database that does not allow query parameters in limit expressions but it's still desirable to have dynamic limit values. The simplest thing I can think of is to expand literal/1 to also allow numbers.

Some other considerations that I did not pursue:

  • Should we allow literal outside of fragments? That would be a bigger change and would not really make sense with the original use case of literal/1 (quoting identifiers). Also we probably don't want to encourage using this too much as it may blow up the Ecto cache.
  • Should we allow more than identifiers and numbers? Then we have an issue where a binary may be interpreted as a literal string or an identifier so we need to add literal/2 or something like that. I'm not sure it is worth it since the use cases are pretty limited.

Companion Ecto SQL PR: elixir-ecto/ecto_sql#651

@greg-rychlewski greg-rychlewski changed the title All numbers in literal/1 Allow numbers in literal/1 Jan 8, 2025
@greg-rychlewski greg-rychlewski changed the title Allow numbers in literal/1 Allow numbers in literal/1 Jan 8, 2025
@greg-rychlewski greg-rychlewski changed the title Allow numbers in literal/1 Allow numbers in literal/1 Jan 8, 2025
Copy link
Member

@josevalim josevalim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am fine with expanding it slowly as needed, as this looks good to me :)

@greg-rychlewski greg-rychlewski merged commit 3ab8279 into elixir-ecto:master Jan 8, 2025
6 checks passed
@greg-rychlewski greg-rychlewski deleted the literal_numbers branch January 8, 2025 19:17
greg-rychlewski added a commit to greg-rychlewski/ecto that referenced this pull request Jan 9, 2025
greg-rychlewski added a commit that referenced this pull request Jan 9, 2025
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

Successfully merging this pull request may close these issues.

2 participants