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

next.jdbc.sql/insert! ignores :options when using next.jdbc/with-options and next.jdbc/with-logging together. #242

Closed
lucassousaf opened this issue Mar 6, 2023 · 3 comments
Assignees
Labels
bug Something isn't working

Comments

@lucassousaf
Copy link

Describe the bug
next.jdbc.sql/insert! ignores the :options specified using the next.jdbc/with-options wrapper if next.jdbc/with-logging is also used. This only happens if you use the next.jdbc/with-logging wrapper after using next.jdbc/with-options. If you use the wrappers in reverse order, everything works as expected.

To Reproduce
To reproduce run the following code:

(defn foo
  []
  (let [db-spec (-> (jdbc/get-datasource {:dbtype "postgresql"
                                          :dbname "postgres"
                                          :user "postgres"
                                          :host "postgres"
                                          :password "sa"
                                          :port 5432})
                    (jdbc/with-options jdbc/unqualified-snake-kebab-opts)
                    (jdbc/with-logging prn prn))]
    (jdbc.sql/insert! db-spec :foo {:id 1 :foo-type "foo"})))

The code throws an exception:

ERROR: syntax error at or near \"-\"\n  Position: 25

If you reverse the order of the wrappers, it works.

Expected behavior
The order of the wrappers shouldn't affect the behavior of next.jdbc.sql/insert!.

project.clj/deps.edn

:dependencies [[org.clojure/clojure "1.11.0"]
               [com.github.seancorfield/next.jdbc "1.3.847"]
               [org.postgresql/postgresql "42.5.3"]]

Environment (please complete the following information):

  • OS: GNU/Linux
  • Java Version: jdk-17-eclipse-temurin
  • Clojure Version: 1.11.0
  • Database: Timescaledb (PostgreSQL 15)
  • Driver Library Version: org.postgresql/postgresql 42.5.3

Additional Context
We think that the problem is in the call to next.jdbc.sql.builder/for-insert function in next.jdbc.sql/insert!. It doesn't take into account the nesting of :connectables.

@seancorfield seancorfield self-assigned this Mar 6, 2023
@seancorfield seancorfield added the bug Something isn't working label Mar 6, 2023
@seancorfield
Copy link
Owner

The problem isn't the call to for-insert but a more fundamental issue that wrapped connectables just don't compose properly at all. When with-options is wrapped around with-logging, it works more or less by accident.

I can "solve" the problem by making with-logging aware of with-options (several places in the code already special case this, because the default options wrapper is "special" and is baked into a lot of the code) but anyone writing their own wrapped connectables is going to run into this same problem at some point.

Providing default options was a terrible idea -- I caved to various people complaining about having to pass options explicitly, because they didn't want to use the default builders. I wish I'd never budged on that... since it leads directly to problems like this 😒

seancorfield added a commit that referenced this issue Mar 13, 2023
@seancorfield
Copy link
Owner

OK, I've pushed a small change to with-logging -- can you test against the develop (or the latest 1.3.999-SNAPSHOT) to see if it solves this problem?

@lucassousaf
Copy link
Author

Yep, I tested the 1.3.999-SNAPSHOT release and it works fine!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants