Skip to content

Conversation

@jonesmelton
Copy link

This changes suresql/defqueries to return a table of queries, as well as defining them globally. I considered using a :local flag in the options struct to switch between the two, but since the return value is currently always nil I don't believe this breaks any of the API. I can still switch it back to that if it's preferable though.

Uses:

  • Queries can be passed around in normal bindings as function arguments, in app middleware, etc instead of in dynamics. This is my use case, since circlet wipes dynamics even if with-dyns is used.
  • Can bind queries "as needed" rather than up front, which can simply code using threads. The db connection can't be marshaled across a thread boundary (and would probably be a bad idea anyway see 2.6). Having the functions available in lexical scope gives more options for how to mitigate that.
    • Another approach could be to separate query parsing and binding them to a connection. They could take the connection at invocation time. This felt like a significant change to the approach. I think the API could be kept the same by using partial application carefully, but it would be more complex than what I did here.

I don't know why I had to change the imports in the tests, I was getting "could not find module" errors. I'm new to janet and have only used import with a relative path symbol, not a string. Maybe it's just an incompatibility with an older version? I'm on 1.25.1-meson. Again if that style is preferred I'm happy to change it back.

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.

1 participant