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

Optional expansion of namespaces in keywords #247

Closed
fvides opened this issue Sep 4, 2019 · 3 comments
Closed

Optional expansion of namespaces in keywords #247

fvides opened this issue Sep 4, 2019 · 3 comments
Labels
needs analysis I need to think about this! needs information Unclear -- please explain in more detail.
Milestone

Comments

@fvides
Copy link

fvides commented Sep 4, 2019

Hello. I'm using honeysql for the first time and liking it a lot for now. Thank for this.

That said, I've encountered a problem that I'm not sure how to resolve. I'm trying to insert tuples as maps using fully qualified keywords, and I find that HoneySQL format expands them fully, so the resulting key is not valid anymore. For example:

user> (-> (insert ::my_table)
      (values {::id 38 ::name "Name of the tuple"})
      format)
=> 
["INSERT INTO user/my_table VALUES (user/id, ?), (user/name, ?)"
 38
 "Name of the tuple"]      

I've found that the function quote-identifier in ns honeysql.format does just that, inconditionally.

Am I missing something? Would be well received a PR with a new parameter for optionaly enabling or disabling this option?

Thanks again!

@seancorfield
Copy link
Owner

That is explicitly done per 5b197f9 three years ago. See discussions in #131 and #132

I agree that it is weird behavior and I'd be surprised if anyone is relying on it in production. If you supply a quoting strategy, you get valid SQL (and it allows you to have / in your table and/or column names -- if your database allows it):

user=> (-> (insert-into ::my_table)
           (values {::id 38 ::name "Name of the tuple"})
           (h/format :quoting :mysql))
["INSERT INTO `user/my_table` VALUES (`user/id`, ?), (`user/name`, ?)" 38 "Name of the tuple"]
user=> 

Since this is deliberate, existing behavior, I am not comfortable changing it, but I will note that clojure.java.jdbc and next.jdbc.sql do not preserve the namespace as part of a keyword used to generate SQL.

@jrdoane was the originator of the namespaced keyword behavior here so I'll see if he has any input. It certainly does seem more problematic in this age of Spec and more widespread us of namespaced keywords in data.

@seancorfield seancorfield added needs analysis I need to think about this! needs information Unclear -- please explain in more detail. labels Sep 6, 2019
@seancorfield seancorfield added this to the 0.9.7 milestone Sep 6, 2019
@fvides
Copy link
Author

fvides commented Sep 10, 2019

Many thanks for closing this issue.

@seancorfield
Copy link
Owner

Thanks for bringing it up -- the change of behavior in 0.9.0 predates my role as maintainer so I hadn't noticed it (other than in cleaning up the PR/issue associated with the change).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs analysis I need to think about this! needs information Unclear -- please explain in more detail.
Projects
None yet
Development

No branches or pull requests

2 participants