Convert row results to array based instead of object based

[MasterOdin]

Many (all?) of the providers suffer from the inability to properly display duplicate column names as they return the data as an object:

{
  "col1": "test",
  "col2": "foo",
  // etc...
}

where if you run a query that uses the same name for 2+ columns, then there'll be a collision and then it's undefined behavior what'll happen then.

Instead, we should always return the result-set as a flat array (e.g. ["test", "foo"]) which we can then combine with the fields field which is an array of column objects to determine the appropriate column information for displaying.

[tomch3ng]

@MasterOdin I would like to take a crack at this. I see that the latest version of the mysql2 package supports this. I will check the others as well.

Does this test case describe what you're looking for? Anything else I should keep in mind? Thoughts on how to do this without making this a breaking change for all clients?

it('should correctly handle duplicate column names in joins', async () => {
  await dbConn.executeQuery(`
    INSERT INTO roles (id, name)
    VALUES ('999', 'developer')
  `);
  
  await dbConn.executeQuery(`
    INSERT INTO users (id, username, email, password, roleid, createdat)
    VALUES ('1', 'maxcnunes', '[email protected]', '123456', '999', '2016-10-25')
  `);

  const results = await dbConn.executeQuery(
    'select * from users u join roles r on (r.id = u.role_id)',
  );

  expect(results).to.have.length(1);
  const [result] = results;

  const idcolumns = <number[]>(
    (<{ name: string }[]>result.fields)
      .map((item, index) => (item.name === 'id' ? index : ''))
      .filter(String)
  );

  expect(idcolumns).to.have.length(2);

  expect(result).to.have.nested.property('rows[0][idcolumns[0]]').eql(1);
  expect(result).to.have.nested.property('rows[0][idcolumns[1]]').eql(999);
});

[MasterOdin]

I started some work on this in https://github.com/sqlectron/sqlectron-db-core/commits/array_results branch. That has a basic test case that demonstrates the desired behavior:

            it('should execute single query with duplicate column names', async () => {
              const results = await dbConn.executeQuery('select 1 as foo, 2 as foo, 3 as bar');

              expect(results).to.have.length(1);
              const [result] = results;

              expect(result).to.have.property('command').to.eql('SELECT');
              expect(result).to.have.deep.property('rowCount').to.eql(1);

              expect(result).to.have.nested.property('fields[0].name').to.eql('foo');
              expect(result).to.have.nested.property('fields[1].name').to.eql('foo');
              expect(result).to.have.nested.property('fields[2].name').to.eql('bar');

              expect(result).to.have.nested.property('rows[0]').have.lengthOf(3);
              expect(result).to.have.nested.property('rows[0][0]').to.eql(1);
              expect(result).to.have.nested.property('rows[0][1]').to.eql(2);
              expect(result).to.have.nested.property('rows[0][2]').to.eql(3);
            });

Essentially, the result object is shaped like:

{
  fields: [
    {
      name: 'foo'
    },
    {
      name: 'foo'
    },
    {
      name: 'bar'
    }
  ],
  rows: [
    [1, 2, 3]
  ]
}

The client (e.g. sqlectron-gui) would then be responsible for taking this info and generating the table to show to the user.

I expect this to be a breaking change across all adapters except for sqlite3 as it does not support this yet (see TryGhost/node-sqlite3#1445). This being BC incompatible is fine as the current method is objectively wrong in that some queries return "wrong" results (duplicate columns clobber each other) as compared to running it directly against the DB.

We may wish to do further disambiguation ourselves (e.g. parse the second instance of foo to foo 2), but that can be done in a follow-up PR if someone requests it.