Clean up syntax for insert row / insert multiple row to improve options handling
Description
Environment
Activity

Sean CorfieldApril 10, 2016 at 7:29 AM
Implemented in 0.5.6. Clojure Guides updated. Release queued.

Sean CorfieldApril 10, 2016 at 1:05 AM
We could also just deprecate insert!
altogether and introduce insert-one!
and insert-multi!
but that breaks the asymmetry of the other methods.
Note that execute!
already has a slightly strange calling sequence for applying a SQL statement multiple times:
That's required because a SQL parameter can have an array value so we need to distinguish between those cases explicitly – and it wouldn't help with insert!
since we can tell the difference between a single row and a series of rows. However, it does highlight that when inserting values by column, the proposed sequence of value vectors isn't entirely safe since you might be trying to insert such an array-valued SQL parameter
So perhaps a better path here is to deprecate the multi-row version of insert!
and use it purely for single row insertions, and then add insert-multi!
for the multiple row case – and it would require sequence of row values. This would still require some disambiguation logic for the arity-4 case but it would be simple, like the original [db table cols-or-rows values-or-opts]
case above. This approach would mean that insert-multi!
would not need to support any legacy (deprecated) cases and therefore could be "clean".
It wouldn't address the ambiguity of (insert! db table map-1 map-2)
for legacy support but that horribleness would only be temporary until 0.6.0 came along.

Sean CorfieldApril 10, 2016 at 12:45 AM
This is a fair bit more tricky than create-table-ddl
because there are several arities and argument combinations supported by a single function today (which, in hindsight, was clearly a bad idea!):
In that third case, the column names can be omitted – nil
can be passed instead of that vector.
In addition to all of that, we want to be able to optionally pass an options map at the end.
For the multiple row cases, we could require the rows to be a sequence of maps (for case 2 above) or a sequence of vectors (for case 3):
We can distinguish those cases easily enough:
We can even add the optional map of options:
This ignores the single row case, but we can add that in and we're still fine, although the logic is getting ugly:
However, we cannot also maintain complete backward compatibility with the current calling sequence because of this case:
We can disambiguate all other forms of the combined old and new syntax (we can tell sequence of rows from sequence of cols by (map? (first rows-or-cols))
) and for the old columns syntax, we have repeated vectors optionally followed by a keyword - either deprecated options unrolled or the :options
flag.
We could assume that the second map is options if a) it is empty or b) contains only keys from the set that insert!
allows, namely :transaction?
and :entities
. That might conflict with inserting into a table with a column named entities
where all the other columns were optional (are there databases where transaction?
would be a valid column name?), so we would additionally need to check that the entities
value was something function-like...
And we'd still need to support keywords in the argument list, even if they are deprecated, until 0.6.0 comes along.
Details
Assignee
Sean CorfieldSean CorfieldReporter
Sean CorfieldSean CorfieldPriority
Major
Details
Details
Assignee

Reporter

Priority

Version 0.5.5 introduced a single options map, flagged by the
:options
argument. This is a compromise while I figure out the better way to changeinsert!
to be a better behaved function, instead of being inherently variadic.