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

Add support for the "RETURNING" clause. #96

Closed
wants to merge 0 commits into from

Conversation

jrdoane
Copy link
Contributor

@jrdoane jrdoane commented Oct 16, 2015

In databases such as Oracle and PostgreSQL, the "RETURNING" SQL keyword denotes a list of columns to be returned on an INSERT, UPDATE, or DELETE statement. This has the convinient side-effect of making DML statements behave the same as SELECT statements that return records, instead of a pass/fail. This is a very powerful feature to support because it enables a developer to get back default field values or the actually record that was inserted if a TRIGGER is potentially altering the INSERTED/UPDATED/DELETED data.

This change should be 100% backwards compatible with existing queries and only extends queries that contain the :returning stanza.

(format
  {:insert-into :foo
   :values [{:a 1}]
   :returning [:someDefaultId :a]}
  :quoting :ansi)
=> ["INSERT INTO \"foo\" (\"a\") VALUES (1) RETURNING \"someDefaultId\", \"a\""]

This contains added entries in README.md for this feature as well as the recently merged :allow-dashed-names setting.

@piranha
Copy link

piranha commented Oct 16, 2015

Although it's not hard to add it yourself (I have it in my codebase), I would say having it by default would be nice.

If anybody's interested in another voice, that is. :)

@ddellacosta
Copy link
Collaborator

I'm kind of on the fence here, as RETURNING is not part of the SQL standard, but at the same point it's kinda not a big deal and it's nice if everyone who wants to use these things doesn't have to implement it themselves...hrm. Think we really need to consider some vendor-specific namespaces or something.

@MichaelBlume @jkk anyone else, thoughts? I also made similar comments here FWIW and @dball had some reasonable counterpoints...that one needs some resolution too (sorry dball).

@jrdoane
Copy link
Contributor Author

jrdoane commented Oct 18, 2015

@ddellacosta I like the idea of a vendor specific namespaces but, supporting that generically with the way that HoneySQL is right now might require some legwork. I wasn't completely certain if this should go in either but, the thing that pushed me was that it wasn't only PostgreSQL that supports RETURNING, namely Oracle (not that the population of Clojure devs using Oracle is huge.) This seemed like the easier solution with that all considered.

As @piranha said, it's not hard to add this to your own codebase but, it's a very convenient feature to utilize if it's available (in my opinion.)

I would definitely be more interested in a way to separate out vendor specific functionality in a clean way. Such a mechanism might make implementing the DDL side of things significantly easier as well given the differences between vendors but, would require some work to develop I suspect.

Not that I have a ton of free time to dev, I would be willing to think about it a bit if no one else is actively. While I think that supporting standard SQL is a must, I don't think that simply catering to the lowest common denominator is going to provide a feature-rich experience. While some software packages want to be able to support different kinds of databases, I don't and have been turned off from libraries for that reason (such as KormaSQL.)

@MichaelBlume
Copy link
Collaborator

@jrdoane While we're figuring this out, would you mind very much if I stole your README text for allow-dashed-names from this pull (with attribution of course)?

@jrdoane
Copy link
Contributor Author

jrdoane commented Oct 19, 2015

@MichaelBlume Absolutely. Go ahead and throw it in. That really should have been part of the other request but I didn't think about it until I was thinking about this problem.

@MichaelBlume
Copy link
Collaborator

So my inclination is still to leave this out of honeysql, simply because "is it ANSI SQL?" is the only clear line I can see apart from including every clause any vendor introduces, which we could work on forever. I'm happy to be overruled if people feel this puts too much of a burden on developers for particularly widely-used clauses.

For reference, here's what including 'returning in an individual project/in an extension library would look like:

https://gist.github.com/MichaelBlume/b108fbe1813f64cd58d4

@drusellers
Copy link

Think we really need to consider some vendor-specific namespaces or something.

I would love to see a well documented way to have vendor specific stuff, especially with PGSQL JSON syntax.

@jrdoane jrdoane closed this Apr 14, 2016
@seancorfield seancorfield added the vendor-specific I think this belongs in a dialect, i.e., not the default behavior? label Jun 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
vendor-specific I think this belongs in a dialect, i.e., not the default behavior?
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants