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 Enumerable for query args instead of Array #207

Merged

Conversation

lwakefield
Copy link
Contributor

This is a little bit of a "see if it sticks", in the sense that I don't have a full grasp of what might break as a result of this proposed change (even though specs pass!)

The impetus for this change, is getting named bindings to work. On top of these changes, I can now do the following:

class SQLite3::Statement
    private def bind_arg(index, tuple : Tuple)
        k, v = tuple
        idx = LibSQLite3.bind_parameter_index(self, ":"+k.to_s)
        bind_arg(idx, v)
    end
end

db.query_all("select 'hello '||:msg", args: {:msg => "world!"}, as: {String})

So - WDYT?

Copy link
Member

@bcardiff bcardiff left a comment

Choose a reason for hiding this comment

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

Regarding lifting Array to Enumerable it seems time has come #110 (comment) :-)

On the named argument api though I am not so sure if treating Tuples like named argument is the accurate thing. Sure is convenient, but what if you want to send an actual tuple as value? An alternative would be to have a separate record NamedArgument which can be implemented by each driver. The api become less attractive though

db.query_all("select 'hello '||:msg", args: [NamedArgument(:msg, "world!")], as: {String})

Maybe for convenience we can have a named arg named_args that is implemented on top of the above construct (or not).

db.query_all("select 'hello '||:msg", named_args: {msg: "world!"}, as: {String})

@straight-shoota straight-shoota changed the title Change args to use the Enumerable type instead of Array Allow Enumerable for query args instead of Array Jul 3, 2024
@straight-shoota straight-shoota merged commit 532ae07 into crystal-lang:master Jul 3, 2024
5 of 6 checks passed
@lwakefield lwakefield deleted the change-args-to-enumerable branch July 3, 2024 18:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants