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

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

Open jtcohen6 opened this issue 1 year ago • 1 comments

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

jtcohen6 avatar Apr 19 '24 09:04 jtcohen6

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

amychen1776 avatar Apr 19 '24 12:04 amychen1776

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 avatar May 01 '24 20:05 dataders

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

Accordingly, there are in this adapter

  • 10 instances of relation.include(database=False) and
  • 4 instances of [{{relation.schema}}].[{{relation.identifier}}]

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

dataders avatar May 01 '24 21:05 dataders