loopback-connector icon indicating copy to clipboard operation
loopback-connector copied to clipboard

Model-specific schema not respected by SQL building methods

Open zenflow opened this issue 9 years ago • 3 comments

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 :)

zenflow avatar Apr 22 '16 17:04 zenflow

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!

bajtos avatar May 10 '16 13:05 bajtos

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 avatar May 10 '16 15:05 raymondfeng

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

raymondfeng avatar May 10 '16 15:05 raymondfeng