Improvements suggestions

[Oreilles]

• Add schema and comment support in tableInfo and columnInfo for all vendors
• Add primary information in tableInfo
• Add is_view information in tableInfo and stop filtering them out
• Delegate name aliasing and type conversion to the database query to bypass conversion from RawColumn to Column
• Refactor knex queries to raw SQL to improve readability when appropriate
• Unify test between vendors
• Move table creation SQL code to test specs.

Opinion? 🥬

[gpetrov]

Initially I was for using Knex queries wherever possible, but this turned a bit to more a hack with all those Knex raw mixtures.

Eventually it is all raw queries, so we might just enter them directly instead double translating then through Knex.

[rijkvanzanten]

Fully agree with @gpetrov here! The only perks to the Knex approach are the conditional "if A then query.where(A)" thingies, but those should be easy enough to refactor too.

For the other points:

✅ Add schema and comment support in tableInfo and columnInfo for all vendors

(for the vendors that support them. IIRC MySQL doesn't differentiate between schema/database, and sqlite doesn't have comments f.e.)

✅ Add primary information in tableInfo
✅ Add is_view information in tableInfo and stop filtering them out
✅ Delegate name aliasing and type conversion to the database query to bypass conversion from RawColumn to Column

The only reason it isn't like that right now is my lack of knowledge of all the specific SQL casting stuff in the different vendors 😁

✅ Refactor knex queries to raw SQL to improve readability when appropriate
❓ Unify test between vendors

Mind clarifying what you mean by unify in this case?

✅ Move table creation SQL code to test specs.

[Oreilles]

IIRC MySQL doesn't differentiate between schema/database

It's just that the default schema in MySQL is named after its database, instead of being named public like others vendors - but it behaves exactly like them.

Mind clarifying what you mean by unify in this case?

Creating the exact same table configuration in each database so the expected result from each test can be shared (and maybe also database initialization if we move it to knex) to avoid code duplication, and reduce chances of error when adding a new feature (like the generated attribute, or upcoming is_view ).

The only reason it isn't like that right now is my lack of knowledge of all the specific SQL casting stuff in the different vendors 😁

Okay, I'll get to it 👍

[rijkvanzanten]

Awesome! Thank you @Oreilles ❤️

[rijkvanzanten]

It's just that the default schema in MySQL is named after its database, instead of being named public like others vendors - but it behaves exactly like them.

Are you sure about that? From MySQL's reference manual:

In MySQL, physically, a schema is synonymous with a database. You can substitute the keyword SCHEMA instead of DATABASE in MySQL SQL syntax, for example using CREATE SCHEMA instead of CREATE DATABASE.

Some other database products draw a distinction. For example, in the Oracle Database product, a schema represents only a part of a database: the tables and other objects owned by a single user.

As far as I can tell, MySQL treats SCHEMA == DATABASE and schema's don't have behavioral characteristics you'd find in the other vendors 🤔

[Oreilles]

OK, that's weird... It means that their concept of a database is different than others. (You can't query table from a database while being in another one in other vendors, but apparently with MySQL you can - because it behaves like a schema).

[rijkvanzanten]

So it'd be better to say that they don't support databases, but only support schemas? 🤔