microsoft / dbt-synapse

dbt adapter for Azure Synapse Dedicated SQL Pools
https://dbt-msft.github.io/dbt-msft-docs/docs/dbt-synapse/overview
MIT License
69 stars 29 forks source link

Set `database=False` in `SynapseRelation.include_policy` instead of overriding `ref` macro #239

Closed jtcohen6 closed 4 months ago

jtcohen6 commented 4 months ago

Hi! I'm one of the folks who helps maintain dbt-core.

We noticed while debugging some failures in our automated tests that dbt-synapse overrides the ref macro here:

https://github.com/microsoft/dbt-synapse/blob/8f4a6a3ab82ea20652bc162b0acbaee461702445/dbt/include/synapse/macros/materializations/models/materialized_view/create_materialized_view_as.sql#L1-L5

This isn't the right way of achieving the desired behavior (rendering relations without the database identifier), and it precludes users from being able to specify two-argument ref.

Instead, if the goal is to render relations without a three-level hierarchy — schema.identifier instead of database.schema.identifier — dbt-synapse should do what dbt-spark does. Something like:

@dataclass
class SynapseIncludePolicy(Policy):
    database: bool = False
    schema: bool = True
    identifier: bool = True

@dataclass(frozen=True, eq=False, repr=False)
class SynapseRelation(BaseRelation):
    include_policy: Policy = field(default_factory=lambda: SynapseIncludePolicy())
amychen1776 commented 4 months ago

@prdpsvs would you be able to take a look at this?

dataders commented 4 months ago

so I found out why ref was overridden and why within create_materialized_view_as.sql of all places.

The Create Materialized View as Select (CMVAS) statement in Azure Synapse does not support three-part names in either the name of the materialized view or any table or view mentioned within the SELECT statement.

Materialized Views in Azure Synapse cannot re I get the following error when executing TestMaterializedViewsBasicSynapse:

Names must be in two-part format

Database Error in model my_mv (models/my_mv.sql)
  ('42000', "[42000] [Microsoft][ODBC Driver 18 for SQL Server][SQL Server]
Cannot schema bind view 'test.my_mv' because name 'dbtmsft.azsyn.cipool.test.my_seed' is invalid for schema binding.
Names must be in two-part format and an object cannot reference itself.
(4512) (SQLExecDirectW)")
dataders commented 4 months ago

another update -- this isn't just a limitation of materialized view. it's all of Azure Synapse.

Accordingly, there are in this adapter

Once the Relation class is properly implemented, all 14 of those can simply be relation. More to come tomorrow