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

Error when using RETURNING from PostgreSQL #15

Closed
rcanepa opened this issue Jan 3, 2016 · 12 comments
Closed

Error when using RETURNING from PostgreSQL #15

rcanepa opened this issue Jan 3, 2016 · 12 comments

Comments

@rcanepa
Copy link

rcanepa commented Jan 3, 2016

Hey,

I am having troubles to define an insert query which should return the new record id. I defined the query this way:

-- :name insert-contact! :!
-- :doc Insert a contact record.
INSERT INTO contacts
(:i*:fields)
VALUES
(:v*:values)
RETURNING id;

When I execute the query through HugSQL I get a BatchUpdateException, however, If I execute the query on psql it pass without problems (returning the record's id). This is the error stacktrace:

user=> ERROR Batch entry 0 INSERT INTO contacts
(address, email, first_name, organization_name, phone, comments, identifier, last_name)
VALUES
('Apoquindo 6371','contacto@moptte.cl','Arturo','Moptte','+56 2 7781 2831','4 oficinas 12 meses.','13.121.981-K','Clode')
RETURNING id was aborted.  Call getNextException to see the cause.
java.sql.BatchUpdateException: Batch entry 0 INSERT INTO contacts
(address, email, first_name, organization_name, phone, comments, identifier, last_name)
VALUES
('Apoquindo 6371','contacto@moptte.cl','Arturo','Moptte','+56 2 7781 2831','4 oficinas 12 meses.','13.121.981-K','Clode')
RETURNING id was aborted.  Call getNextException to see the cause.
    at org.postgresql.jdbc2.AbstractJdbc2Statement$BatchResultHandler.handleError(AbstractJdbc2Statement.java:2717)
    at org.postgresql.jdbc2.AbstractJdbc2Statement$BatchResultHandler.handleResultRows(AbstractJdbc2Statement.java:2666)
    at org.postgresql.core.v3.QueryExecutorImpl$1.handleResultRows(QueryExecutorImpl.java:373)
    at org.postgresql.core.v3.QueryExecutorImpl.processResults(QueryExecutorImpl.java:1828)
    at org.postgresql.core.v3.QueryExecutorImpl.execute(QueryExecutorImpl.java:338)
    at org.postgresql.jdbc2.AbstractJdbc2Statement.executeBatch(AbstractJdbc2Statement.java:2898)
    at clojure.java.jdbc$execute_batch.invokeStatic(jdbc.clj:404)
    at clojure.java.jdbc$execute_batch.invoke(jdbc.clj:397)
    at clojure.java.jdbc$db_do_execute_prepared_statement$fn__10737.invoke(jdbc.clj:764)
    at clojure.java.jdbc$db_transaction_STAR_.invokeStatic(jdbc.clj:595)
    at clojure.java.jdbc$db_transaction_STAR_.doInvoke(jdbc.clj:568)
    at clojure.lang.RestFn.invoke(RestFn.java:425)
    at clojure.java.jdbc$db_do_execute_prepared_statement.invokeStatic(jdbc.clj:763)
    at clojure.java.jdbc$db_do_execute_prepared_statement.invoke(jdbc.clj:748)
    at clojure.java.jdbc$db_do_prepared.invokeStatic(jdbc.clj:786)
    at clojure.java.jdbc$db_do_prepared.doInvoke(jdbc.clj:770)
    at clojure.lang.RestFn.invoke(RestFn.java:464)
    at clojure.java.jdbc$execute_BANG_$execute_helper__10764.invoke(jdbc.clj:891)
    at clojure.java.jdbc$execute_BANG_.invokeStatic(jdbc.clj:896)
    at clojure.java.jdbc$execute_BANG_.doInvoke(jdbc.clj:875)
    at clojure.lang.RestFn.invoke(RestFn.java:425)
    at clojure.lang.AFn.applyToHelper(AFn.java:156)
    at clojure.lang.RestFn.applyTo(RestFn.java:132)
    at clojure.core$apply.invokeStatic(core.clj:649)
    at clojure.core$apply.invoke(core.clj:640)
    at hugsql.adapter.clojure_java_jdbc.HugsqlAdapterClojureJavaJdbc.execute(clojure_java_jdbc.clj:11)

Any thoughts on what could I be missing?

@robinheghan
Copy link

I can confirm this bug. I noticed it yesterday. My exception was "Received result when none was expected"

Using the latest version of hugsql, and funcool/clojure.jdbc adapter.

@csummers
Copy link
Member

csummers commented Jan 4, 2016

This happens because statements executed with execute (:! in HugSQL) are not expected to return result sets. You can get around this issue by using :? for your insert w/ returning statements.

However, I'm going to keep this open. The HugSQL docs should indicate this, I should add a test to show this, and I'm thinking about adding a command type that would indicate this. Also, this is tangentially related to issue #8.

@rcanepa
Copy link
Author

rcanepa commented Jan 4, 2016

@csummers great!. I can confirm that it solves the problem. All I had to do was change my query to something like this:

-- :name insert-contact! :? :1
-- :doc Insert a contact record.
INSERT INTO contacts
(:i*:fields)
VALUES
(:v*:values)
RETURNING id;

Thanks!

csummers added a commit that referenced this issue Feb 9, 2016
…xecute, :<! for sql returning clause; issues #8 and #15
@csummers
Copy link
Member

As of the 0.4.0, there is a :command type of :returning-execute, or :<! for short. This still executes as a query like :?, but provides a bit more semantic value. See Insert for an example.

@rcanepa
Copy link
Author

rcanepa commented Feb 15, 2016

Excellent!... I will update my code and let you know if I something doesn't work!... Keep the good work!!

@sthomp
Copy link

sthomp commented Jun 8, 2016

Just came across this issue and wanted to suggest adding an example to the Insert examples of how to insert with dynamic columns (ie: the (:i*:fields) notation used above). All the current examples use fixed columns.

@csummers
Copy link
Member

csummers commented Jun 8, 2016

Thanks for the suggestion. I could add a couple of examples to this effect. I think a likely scenario instead of the separate fields and values vectors would use a hashmap of {"field1" "value1"...}. This is hinted at with the generic update example in the Clojure Expressions portion of the docs.

@sthomp
Copy link

sthomp commented Jun 8, 2016

That would be really nice. The {field value .. syntax is probably the best feature about Korma that other libraries are missing

@rcanepa
Copy link
Author

rcanepa commented Jun 8, 2016

Hi @sthomp, I use that pattern (:i*:fields) in combination with something like this:

(s/defn create-office :- {s/Keyword s/Uuid}
  [db :- ConnectionMap
   office :- schemas/Office]
  (let [office (select-keys office schemas/office-db-fields) ;; remove read only fields
        fields (map name (keys office)) ;; get fields as strings
        values (vals office) ;; get values
        sql-args {:fields fields
                  :values values}]
    (execute! db :insert-office sql-args)))

@sthomp
Copy link

sthomp commented Jun 8, 2016

Thanks @rcanepa I'm also doing something similar

chadhs added a commit to chadhs/listopia that referenced this issue Dec 1, 2017
needed to update the list and item execute query meta data to be :? if
they are returning anything.

reference: layerware/hugsql#15 (comment)
chadhs added a commit to chadhs/listopia that referenced this issue Dec 28, 2017
* WIP: functional testing

- mocking out tests and then working on implementing them leveraging
  migratus to avoid state issues

* initial workup of mocking requests and scaffolding out test

- wrote a mocking function for dealing with requests generation for
  passing to handlers. initially pulled in ring-mock, but for now my own
  cs-http-request-mock is more suited to what i need
- lein test and cider test aren't both using the test profile so i'm
  manually defining the test db for now in the core test file
- my first list creation test is working well with my mock funtion
- i've defined mock tests for the other crud operations with comments,
  will tackle those next

* bugfix: create queries were returning id of 0

needed to update the list and item execute query meta data to be :? if
they are returning anything.

reference: layerware/hugsql#15 (comment)

* testing update:

- full ci pipeline test runs can be run with the test profile and
  specific database; and driven by a run-tests.sh shell script
- removed pieces trying to manipulate the db path already defined once
  cider is running; i'll just be ok with cider tests running in the
  dev profile
- updated base-mock to allow for params or route-params if defined
- added uuid->str function for use in tests
- wrote and have working tests for list deletion and item creation

* tests update:

- added read-item query to get the details of a single item
- utility function to convert from hugsql clojure map uuid to java uuid format
- reworded tests to assert truthiness when they pass
- wrote check, uncheck, and item delete tests
- updated tests to not only check for a successful 302 but verified values or deletion as well

* update the docs for queries returning an id

* refactor helper functions into a util namespace

* test refactor:

created util functions for generating test lists and items returning
maps of needed ids in the desired formats.

used new functions in tests and referenced the keys in the returned
maps.  this is a lot more readable and changing the logic can be done
in a single place now as long as the data returned is a map which
contains at least the expected keys.

* test pipeline update:

- updated to circleci v2
- added in container setup for postgres test

* setting db name at image load

* set db_url for test in ci file

* update circle ci file with db config

* ci update / fix:

- pull out pg vars
- make lein use test profile

* ci update:

- bumping up postgres
- removing db url as it's redundant

* ci debug

* ci debug

* debug ci

* debug ci

* debug ci

* ci debug

* debug ci

* ci debug

* ci debug

* ci debug

* post debug: cleaning up ci file

* ci update: add test run and uberjar build back in

* build / deploy update:

- converted deploy process to v2.0
- due to no heroku integration yet had to follow the docs here:
https://circleci.com/docs/2.0/deployment_integrations/#heroku
- minor ci fix
- updated run-tests for local use which properly sets up and cleans up

* fix working directory issue

* deploy fix: added checkout step

* minor test comment update
@Ramblurr
Copy link

@csummers Is this feature supposed to work for UPDATE statements too? Postgres also supports this.

@byrongibby
Copy link

Works with upsert using INSERT ON CONFLICT statement as well, if there was any doubt.

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

No branches or pull requests

6 participants