-
Notifications
You must be signed in to change notification settings - Fork 312
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for tsql 3 and 4 part naming #279
Conversation
lib/ecto/adapters/tds/connection.ex
Outdated
defp quote_table({server, db, schema}, name), | ||
do: Enum.map_join([quote_table(server), ".", quote_table(db), ".", quote_table(schema), ".", quote_table(name)], "", &"#{&1}") | ||
|
||
defp quote_table({db, schema}, name), | ||
do: Enum.map_join([quote_table(db), ".", quote_table(schema), ".", quote_table(name)], "", &"#{&1}") | ||
|
||
defp quote_table(prefix, name), | ||
do: Enum.map_join([quote_table(prefix), ".", quote_table(name)], "", &"#{&1}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All of these map joins are returning the structure itself, i don't think you need them:
defp quote_table({server, db, schema}, name), | |
do: Enum.map_join([quote_table(server), ".", quote_table(db), ".", quote_table(schema), ".", quote_table(name)], "", &"#{&1}") | |
defp quote_table({db, schema}, name), | |
do: Enum.map_join([quote_table(db), ".", quote_table(schema), ".", quote_table(name)], "", &"#{&1}") | |
defp quote_table(prefix, name), | |
do: Enum.map_join([quote_table(prefix), ".", quote_table(name)], "", &"#{&1}") | |
defp quote_table({server, db, schema}, name), | |
do: [quote_table(server), ".", quote_table(db), ".", quote_table(schema), ".", quote_table(name)] | |
defp quote_table({db, schema}, name), | |
do: [quote_table(db), ".", quote_table(schema), ".", quote_table(name)] | |
defp quote_table(prefix, name), | |
do: [quote_table(prefix), ".", quote_table(name)] |
Also, please add tests to test/ecto/adapters/tds_test.exs. See "delete_all with prefix" as an example, except you don't need to call delete_all
, all
is fine. Thanks!
Should it use a list of strings instead of a tuple if there can be many elements? This seems more natural to me |
I thoughT the same but given the number of elements is still bound (3), I think a tuple works better. |
Tables may be referenced in SQL Server by up to 4-part naming of `[server].[database].[schema].[table]`. These may be passed to the Tds adapter via the schema_prefix as tuples: `@schema_prefix {"database", "schema"}` and `@schema_prefix {"server", "database", "schema"}`
Tables may be referenced in SQL Server by up to 4-part naming of `[server].[database].[schema].[table]`. These may be passed to the Tds adapter via the schema_prefix as tuples: `@schema_prefix {"database", "schema"}` and `@schema_prefix {"server", "database", "schema"}`
Tables may be referenced in SQL Server by up to 4-part naming of
[server].[database].[schema].[table]
.These may be passed to the Tds adapter via the schema_prefix as tuples:
@schema_prefix {"database", "schema"}
and@schema_prefix {"server", "database", "schema"}
Fixes #278