Fix running multiple queries & sorting when multiple tables are present in psql

misterbuckley commented 6 years ago

In this PR:

Fix running multiple queries through SQHExecute(File) in psql
Fix sorting when the results of multiple queries are present in psql

Things to note:

The sorting aspect of this leverages the fact that psql puts a blank line between tables in its output. Because mysql doesn't do this, and I haven't been able to figure out a way to get it to do so, sorting in mysql results with >1 table will still be broken

joereynolds commented 6 years ago

Hey @misterbuckley

I've written some tests already for sorting here https://github.com/joereynolds/SQHell.vim/blob/master/test/sqhell.vader#L73

Is there any chance you'd like to add some new ones for this?

i.e. Create a new test where the provider is psql and has 2 tables in the buffer. Write tests against that?

misterbuckley commented 6 years ago

Will do

joereynolds commented 6 years ago

Hey, there was a test failure (details below) On a related note, do you need to set g:sqh_provider in this test? Its default is MySQL.

Nice work btw!

   (10/10) [ EXPECT] (X) only the first table to be sorted by its second column
      - Expected:
           id | first_name | last_name  | birth_date 
          ----+------------+------------+------------
            7 | Anakin     | Skywalker  | 1970-06-01
            9 | Bruce      | Lee        | 2001-01-01
           10 | Grom       | Hellscream | 2002-02-02
            6 | Jean-Luc   | Picard     | 1950-11-24
            4 | Jeremy     | Clarkson   | 1977-08-01
            2 | John       | Hancock    | 2015-08-03
            3 | Peter      | Parker     | 1990-08-01
            1 | Robert     | Marley     | 1966-08-01
            8 | Tommy      | Wiseau     | 2015-07-06
            5 | Wade       | Watts      | 1977-08-01

           id |           email_address           |       user_created_date       
          ----+-----------------------------------+-------------------------------
            1 | bob.marley@onelove.com            | 2015-08-13 15:41:01.286399-04
            2 | JohnHancock@usa-1776.com          | 2015-11-12 14:55:08.658266-05
            3 | PeterParker@PickledPepper.com     | 2015-11-13 10:37:07.800893-05
            4 | jeremy@topgear.co.uk              | 2015-11-20 10:09:28.822073-05
            5 | wade.watts@gss-oasis.us           | 2015-11-20 14:43:14.944273-05
            6 | Jean-Luc@ncc-1701-d.org           | 2015-12-03 12:34:59.158075-05
            7 | anakin@skywalking.info            | 2016-01-27 10:16:17.822779-05
            8 | lisa.is.tearing.me.apart@room.com | 2015-12-31 13:11:30.638645-05
            9 | learn-jeet-kune-do@leejunfan.com  | 2016-01-06 11:54:12.403473-05
           10 | WildhammerFactChecker@blizz.us    | 2016-01-07 14:48:32.347995-05
      - Got:
           id | first_name | last_name  | birth_date 
          ----+------------+------------+------------
            1 | Robert     | Marley     | 1966-08-01

          ----+-----------------------------------+-------------------------------
            7 | Anakin     | Skywalker  | 1970-06-01
            7 | anakin@skywalking.info            | 2016-01-27 10:16:17.822779-05
            1 | bob.marley@onelove.com            | 2015-08-13 15:41:01.286399-04
            9 | Bruce      | Lee        | 2001-01-01
           id |           email_address           |       user_created_date       
           10 | Grom       | Hellscream | 2002-02-02
            6 | Jean-Luc@ncc-1701-d.org           | 2015-12-03 12:34:59.158075-05
            6 | Jean-Luc   | Picard     | 1950-11-24
            4 | Jeremy     | Clarkson   | 1977-08-01
            4 | jeremy@topgear.co.uk              | 2015-11-20 10:09:28.822073-05
            2 | John       | Hancock    | 2015-08-03
            2 | JohnHancock@usa-1776.com          | 2015-11-12 14:55:08.658266-05
            9 | learn-jeet-kune-do@leejunfan.com  | 2016-01-06 11:54:12.403473-05
            8 | lisa.is.tearing.me.apart@room.com | 2015-12-31 13:11:30.638645-05
            3 | Peter      | Parker     | 1990-08-01
            3 | PeterParker@PickledPepper.com     | 2015-11-13 10:37:07.800893-05
            8 | Tommy      | Wiseau     | 2015-07-06
            5 | Wade       | Watts      | 1977-08-01
            5 | wade.watts@gss-oasis.us           | 2015-11-20 14:43:14.944273-05
           10 | WildhammerFactChecker@blizz.us    | 2016-01-07 14:48:32.347995-05

misterbuckley commented 6 years ago

Its default is MySQL.

Huh, no wonder my test was passing locally but some of the mysql ones were failing for me (since I set g:sqh_provider = 'psql' in my vimrc). I shoulda guessed lol. How would you feel about breaking the mysql- and psql-specific tests out into their own test files, adding Before blocks that set g:sqh_provider in each, and Includeing those files in the main test file? Just to keep things a little more modular.

joereynolds commented 6 years ago

@misterbuckley sounds like a good idea, go for it :)

misterbuckley commented 6 years ago

OK I'm baffled. These tests all pass for me locally. Any ideas as to why they might not be passing on travis?

joereynolds commented 6 years ago

My only guess could be the default vim version on travis, I'll pull down your branch and see if they work for me too

joereynolds commented 6 years ago

Okay so the mysql test fails because the sort is not numeric based, it's based on a string, so you should actually be expecting

 +----+-----------------+-------------------------------------+------------+----------------------------+--------------------------------------------+
 | id | prefix          | location                            | date       | artist                     | url                                        |
 +----+-----------------+-------------------------------------+------------+----------------------------+--------------------------------------------+
 |  9 | Various Artists | The Vic Biker's Pub, Coalville, UK  | 2017-10-27 |                            |                                            |
 |  8 | Various Artists | The North, Rhyl, UK                 | 2017-10-05 |                            |                                            |
 |  7 | Various Artists | Hyvinkää, Finland                   | 2017-04-21 |                            |                                            |
 |  6 | Various Artists | Helsinki, Finland                   | 2017-04-18 |                            |                                            |
 |  4 | Various Artists | The Station, Ashton Under Lyne, UK  | 2018-04-14 |                            |                                            |
 |  3 | Supporting      | The Live Rooms, Chester, UK         | 2017-11-23 | Wonk Unit                  | http://wonkunit.com/                       |
 |  2 | Supporting      | Tivoli, Buckley, UK                 | 2017-10-20 | The Sex Pistols Experience | http://www.sexpistolsexperience.co.uk/     |
 |  1 | Supporting      | Mcleans Pub, Deeside, UK            | 2017-09-09 | Vice Squad                 | http://vicesquad.co.uk/punkrock/           |
 | 18 | Various Artists | Swan With 2 Necks, Macclesfield, UK | 2018-06-09 |                            |                                            |
 | 17 | Event           | Sumac Centre, Nottingham, UK        | 2018-08-25 | Punks 4 Homeless           | https://www.facebook.com/punk4thehomeless/ |
 | 16 | Event           | Rewind, Wrexham, UK                 | 2018-05-10 | Focus Wales                | http://www.focuswales.com/                 |
 | 14 | Various Artists | The Lock Keeper, Chester, UK        | 2017-08-05 |                            |                                            |
 | 13 | Various Artists | The Station, Ashton Under Lyne, UK  | 2017-07-01 |                            |                                            |
 | 12 | Various Artists | Rewind, Wrexham, UK                 | 2016-11-15 |                            |                                            |
 | 11 | Various Artists | Comrades Club, Church St, Conwy     | 2017-04-30 |                            |                                            |
 | 10 | Supporting      | o2 Academy, Manchester, UK          | 2017-10-21 | The Sex Pistols Experience | http://www.sexpistolsexperience.co.uk/     |
 +----+-----------------+-------------------------------------+------------+----------------------------+--------------------------------------------+

Not what you currently have which is descending by number :)

Also, sorting results now chops off the header and then reattaches it. My guess is because of the psql change that you also reflected in the mysql file.

Here's a gif

sort-table-bug

My guess is there's a bug with how you're sorting the tables. I'm not sure why it works locally for you because I get the same output as the test (for mysql at least)

misterbuckley commented 6 years ago

That's the thing though, S on the mysql tables on my machine sorts them correctly (as if they were numbers) and the header stays put. The only thing I can think is that I'm on a mac, so maybe the sort command on OSX works differently than on linux? Otherwise I've got nothing.

Edit: I think the header being moved there is because you have 2 blank lines at the end and the mysql sort function currently just takes line 4 until the (last - 1) line, so it includes the bottom line and one of the blank lines in the sort

joereynolds commented 6 years ago

You're right about the sort formatting, it's not an issue but the sorting is definitely string based. You said so yourself in your last PR

As for the numbers being sorted incorrectly, I kinda saw that one coming. :SQHSortResults -n should sort it correctly.

You could add the -n flag to your test instead if you wanted to test it numerically instead?

joereynolds commented 6 years ago

hey @misterbuckley I've updated vim on the build to be Vim 8 instead of god knows what it originally comes with. I've also resolved a few conflicts and merged master in. Hopefully testing will be a bit more reliable for your changes now.

joereynolds commented 6 years ago

Alright. We're getting somewhere :D I've added OSx to the build, the build now builds separately on both linux and mac. It seems that on mac it does sort differently (you were right I was wrong).

You can confirm this by clicking in here

https://travis-ci.org/joereynolds/SQHell.vim

And looking at the build results of each os. Linux passes with the current tests whereas mac fails on the sort.

I'm trying to think of the best course of action here, I'm open to suggestions.

Rather than create 2 separate test directories (1 for mac, 1 for linux) I think we should just change the sort command based on the current os being used.

So in the SortResults (whatever it's called) function, we should make sure that they produce the same output on both os's.

I'd probably prefer the mac sorting behaviour to be honest, but it's your call :)

joereynolds commented 6 years ago

Hey @misterbuckley,

I've fixed the tests on mac and linux on master (made -n default). If you'd like to pull down master and re-jig your tests slightly it should all work.

I removed the test where we're ordering by the 'prefix' column in the table as it's pretty pointless with -n being on permanently.

I'll do some more work to get it not have -n hardcoded in but for now at least you can get your postgres stuff in :D

misterbuckley commented 6 years ago

Thanks for taking care of that! 🙇 I thought I'd get a chance to check it out over the weekend and never did. Here's hoping we can finally get this in.

joereynolds commented 6 years ago

More test failures, you can remove the test that orders on the prefix column by the way. It's pointless now that we have -n as the default (that's what I've done in the master branch)

joereynolds commented 6 years ago

Hey @misterbuckley

I've merged in your multiple query fix and resolved the conflicts on the branch, now all that's left is the sort stuff so hopefully that's a little bit less to do

joereynolds / SQHell.vim

Fix running multiple queries & sorting when multiple tables are present in psql #41