search

kostafey / ejc-sql

Emacs SQL client uses Clojure JDBC.
279 stars 29 forks source link

Order of columns does not follow query specification #95

Closed alf-mindshift closed 5 years ago

alf-mindshift commented 5 years ago

The output from running queries using ejc-sql does not honor the ordering of the column specification in the SQL query.

This is annoying as I often want to see the result with a specific column ordering.

The offending code seems to be the call tojava.jdbc/query from connect/eval-sql-core:

                       (j/query db stmt
                                {:as-arrays? false
                                 :row-fn clob-to-string-row})

Since we ask for the results as a list of maps and since clojure maps are not ordered we lose the neccessary information to provide the requested column ordering.

I suggest we change the code to request the results as a list of arrays which will make the problem go away.

I'm happy to create a pull request with this change if you agree with the direction this takes the project. I do not consider this a breaking change as we do not currently control the order of the returned data. Unless of course someone depends on the behaviour of connect/eval-sql-core outside of this repository...?

kostafey commented 5 years ago

I can't reproduce the issue and always get correct columns order. Looks it depends on Java and Clojure versions and theirs map implementations. The early versions of ejc-sql operated with arrays, but then this behavior was changed. See this commit: https://github.com/kostafey/ejc-sql/commit/28cc15c2466f2c5330ab3621d21cdbd27e86b533#diff-1b8a1c68c41c80c9d77345e762488666L88 You will need a bit different ejc-sql.output/print-table implementation. The previous version uses ultimately trivial ad-hoc ejc-sql.output/format-output: https://github.com/kostafey/ejc-sql/commit/28cc15c2466f2c5330ab3621d21cdbd27e86b533#diff-fab2cf336a9e8402c5fafdd9092f28c3L67

Sure, you can create a pull request to fix it, since we have a problem in some environments.

alf-mindshift commented 5 years ago

For information I'm on Java 1.8.0 and clojure version 1.9.

alf-mindshift commented 5 years ago

See #96. Please do look at it properly as I can't guarantee that I've thought about all conditions.

It works for me, but YMMV.

alf commented 5 years ago

Discovery a bug in the implementation and will fix this and clean up the print-table function a bit.

Would you mind if I sneak in a feature while I’m at it? I sometimes find it would be nice to return a transposed (rotated) result. I usually just copy the table to org-mode and transpose it there, but having this directly would be nice and easy to implement.

I suggest using the universal argument to enable that behavior.

kostafey commented 5 years ago

Sure, it's completely acceptable. A single-record result is already transposed to improve result readability. Probably, we can do it for any number of records based on additional argument.

alf-mindshift commented 5 years ago

Just to add a bit more scope creep here. If we prepend and append a | to the table format, we'll match the org-mode table format. This grants us a huge amount of functionality for free from org-table. Just to mention a few of these:

Of course, this requires us to make the results buffer writable as well, which I don't really see as a big deal.

If you prefer, I could make this behaviour optional.

alf commented 5 years ago

I will split this into two different issues and pull requests.