dbt-sqlserver icon indicating copy to clipboard operation
dbt-sqlserver copied to clipboard

updates the create schema statement

Open vigyuv opened this issue 3 years ago • 3 comments

Adds authorization based on the create schema syntax defined by Microsoft. https://docs.microsoft.com/en-us/sql/t-sql/statements/create-schema-transact-sql?view=sql-server-ver15#remarks

resolves: #153

vigyuv avatar Aug 11 '22 19:08 vigyuv

@jhoolachan @w0ut0 we think this might solve #153! any chance you want to pip install this branch and see if it works for you? or at least copy the sqlserver__create_schema() macro into your project's macros/ dir and see if it works when you also provide an auth config when defining a custom schema...

also perhaps the auth field is better defined in the profiles.yml because it's not likely that there'd be more than one authorization setting used by a single user?

dataders avatar Aug 12 '22 17:08 dataders

Hi @dataders, thanks for this, and sorry for the delay in testing! unfortunately this does not solve our issue, I still have to make sure I add the user manually to the database first (he is not implicitly created because his group exists in the DB). Some logs:

12:42:48.647585 [info ] [MainThread]: Found 302 models, 373 tests, 0 snapshots, 0 analyses, 502 macros, 0 operations, 3 seed files, 73 sources, 2 exposures, 0 metrics
12:42:48.671691 [info ] [MainThread]: 
12:42:48.672493 [debug] [MainThread]: Acquiring new sqlserver connection "master"
12:42:48.674095 [debug] [ThreadPool]: Acquiring new sqlserver connection "list_<dbname>"
12:42:48.688168 [debug] [ThreadPool]: SQLServer adapter: Using sqlserver connection "list_<dbname>".
12:42:48.688431 [debug] [ThreadPool]: SQLServer adapter: On list_<dbname>: USE "<dbname>";
   select  name as [schema]
   from sys.schemas
 
12:42:48.688610 [debug] [ThreadPool]: Opening a new connection, currently in state init
12:42:48.688785 [debug] [ThreadPool]: SQLServer adapter: Using connection string: DRIVER={ODBC Driver 17 for SQL Server};SERVER=dl-<server_id>-sqlserver.database.windows.net,1433;Database=<dbname>;Encrypt=Yes;Application Name=dbt-sqlserver/1.1.0
12:42:49.239272 [debug] [ThreadPool]: SQLServer adapter: Connected to db: <dbname>
12:42:49.243371 [debug] [ThreadPool]: SQLServer adapter: SQL status: OK in 0.55 seconds
12:42:49.246550 [debug] [ThreadPool]: On list_<dbname>: Close
12:42:49.248534 [debug] [ThreadPool]: Acquiring new sqlserver connection "create_<dbname>_dbt_<user_first_name>"
12:42:49.248939 [debug] [ThreadPool]: Acquiring new sqlserver connection "create_<dbname>_dbt_<user_first_name>"
12:42:49.249290 [debug] [ThreadPool]: Creating schema "_ReferenceKey(database='<dbname>', schema='dbt_<user_first_name>', identifier=None)"
12:42:49.259538 [debug] [ThreadPool]: SQLServer adapter: Using sqlserver connection "create_<dbname>_dbt_<user_first_name>".
12:42:49.259777 [debug] [ThreadPool]: SQLServer adapter: On create_<dbname>_dbt_<user_first_name>: USE [<dbname>];
   IF NOT EXISTS (SELECT * FROM sys.schemas WHERE name = 'dbt_<user_first_name>')
   BEGIN
   EXEC('CREATE SCHEMA [dbt_<user_first_name>]')
   END
 
12:42:49.259939 [debug] [ThreadPool]: Opening a new connection, currently in state closed
12:42:49.260118 [debug] [ThreadPool]: SQLServer adapter: Using connection string: DRIVER={ODBC Driver 17 for SQL Server};SERVER=dl-<server_id>-sqlserver.database.windows.net,1433;Database=<dbname>;Encrypt=Yes;Application Name=dbt-sqlserver/1.1.0
12:42:49.325325 [debug] [ThreadPool]: SQLServer adapter: Connected to db: <dbname>
12:42:49.330244 [debug] [ThreadPool]: SQLServer adapter: SQL status: OK in 0.07 seconds
12:42:49.331522 [debug] [ThreadPool]: SQLServer adapter: Database error: ('42000', "[42000] [Microsoft][ODBC Driver 17 for SQL Server][SQL Server]Cannot find the user '', because it does not exist or you do not have permission. (15151) (SQLMoreResults)")
12:42:49.331775 [debug] [ThreadPool]: On create_<dbname>_dbt_<user_first_name>: ROLLBACK
12:42:49.331944 [debug] [ThreadPool]: On create_<dbname>_dbt_<user_first_name>: Close
12:42:49.333972 [debug] [MainThread]: Connection 'master' was properly closed.
12:42:49.334234 [debug] [MainThread]: Connection 'create_<dbname>_dbt_<user_first_name>' was properly closed.

The only thing I see that would help is creating the user if it doesn't exist yet on schema creation, but I'm not sure that is something we want.

w0ut0 avatar Aug 29 '22 12:08 w0ut0

I think we need something like this. Right now everything is always owned by dbo. You're using a configuration variable, can we make it more seamless in some cases? E.g. if the user uses SQL authentication, shouldn't we just use SQL username?

sdebruyn avatar Sep 12 '22 20:09 sdebruyn

I ran into the same issue but then on Synapse at a customer a few months ago. We ended up giving the users more permissions. The nasty part about this is the following part:

You can give permissions to AAD groups if you create those as external database principals. We added the auto provisioning feature to the adapter to make that part of the flow nicer. But when you are signed in as a user of an AAD group and you don't have the right roles, you cannot create schemas. Since, implicitly, it will try to link the schema to a user which it has to create, but it does not create AAD users.

What is the key missing piece here is that Microsoft should allow us to let AAD groups own schemas, then the default would be okay and this PR would not even be needed.

I don't think we should provide a solution for this inside the adapter as there is no way of knowing (from the side of the adapter) what your AAD username is. (CURRENT_USER gives the name of your group, or - if you have the db_owner role - it just gives dbo).

In our case we had to give the users the CONTROL on the database, we should further research how we can create and use schemas without such grants.

sdebruyn avatar May 16 '23 15:05 sdebruyn

I am going to rebase this PR, rename the config setting to make it clearer what it does and add tests for it.

sdebruyn avatar May 16 '23 17:05 sdebruyn

This doesn't seem to work, the config is ignored at the stage the schema is created

sdebruyn avatar May 16 '23 19:05 sdebruyn

Closing in favor of #382

sdebruyn avatar May 17 '23 09:05 sdebruyn