reject queries that are not string #116
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
If you pass an ActiveRecord::Relation, it will be executed when the call is analyzed for logging purposes, potentially timing out the migration.
|
|
Author
|
Note: I haven't been able to execute the tests locally; there doesn't seem to be documentation on how to do that. I assume I need to configure a database somehow? |
jonatas
reviewed
May 6, 2025
| # SQL | ||
| # | ||
| def create_continuous_aggregate(table_name, query, **options) | ||
| raise ArgumentError, 'query must be a string' unless query.is_a?(String) |
Contributor
There was a problem hiding this comment.
Just thinking about old migrations that are already in place.
Suggested change
| raise ArgumentError, 'query must be a string' unless query.is_a?(String) | |
| query = query.to_sql if query.respond_to(:to_sql) | |
| raise ArgumentError, 'query must be a string or implement the `to_sql` method ' unless query.is_a?(String) |
Author
There was a problem hiding this comment.
You're right to think about backwards compatibility, but if I apply your changes then this PR fixes nothing.
The problem is caused by allowing people to pass an ActiveRecord::Relation as an input to create_continuous_aggregate.
If you want to preserve backwards compatibility for now, we could look into making it a warning instead of an exception.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
If you pass an ActiveRecord::Relation, it will be executed when the call is analyzed for logging purposes, potentially timing out the migration.
Fix #115
I'm not quite sure why github id displaying my commit as unverified, it should be signed as usual.