search

loopbackio / loopback-connector

Building blocks for LoopBack connectors
Other
35 stars 99 forks source link

Model-specific schema not respected by SQL building methods #47

Open zenflow opened 8 years ago

zenflow commented 8 years ago

We are working with a legacy database that has our data divided among different schemas.

As per the documentation, I should be able to set the schema for an individual model. https://docs.strongloop.com/display/public/LB/Model+definition+JSON+file#ModeldefinitionJSONfile-Datasource-specificoptions

However, SQLConnector does not specify any schema in the SQL statements that it builds, even though there is a schema method that properly determines which schema to use for a given model.

We should use this schema method to always specify the schema in generated SQL.

@raymondfeng @bajtos What are the chances of this being implemented and released as version 3? It doesn't look like a complicated change, so I could probably implement it myself and submit a PR, if that increases the chances.

Are there any problems you can see with making this change? I know this would break loopback-connector-db2 since it needlessly overrides the schema method with a schema string property in the constructor, which is why I believe it should be released in the next major version.

Thanks :)

bajtos commented 8 years ago

Hi @zenflow, sorry for the delay. I am not very familiar with this aspect of connectors, but your proposal looks good to me in principle. To prevent breaking changes, perhaps we could add a new datasource setting to enable this new behaviour?

It would be great if you could contribute this change yourself!

raymondfeng commented 8 years ago

It's a general configuration scheme.

  1. The data source settings should have the option to specify default DB schema.
  2. Each model can override it using https://docs.strongloop.com/display/public/LB/Model+definition+JSON+file#ModeldefinitionJSONfile-Datasource-specificoptions

Most connectors should have supported 2. For example, https://github.com/strongloop/loopback-connector-mssql/blob/master/lib/mssql.js#L382. If you see otherwise, please let us know which one has issues.

raymondfeng commented 8 years ago

If you propose to enhance https://github.com/strongloop/loopback-connector/blob/master/lib/sql.js#L229 to consider schema, +1.