Skip to content

Commit 1d0c901

Browse files
fix(redshift): do not recreate existing views on every evaluation
On engines without view binding, ViewStrategy.insert recreated an existing view on every interval evaluation. On Redshift this is a DROP + CREATE, which gives the view a new OID and causes concurrent queries reading through it to fail with "could not open relation with OID". Add an engine adapter attribute RECREATE_VIEW_ON_EVALUATION (default True) and set it to False for Redshift, so an existing regular view is only recreated on the first insert of a snapshot version, which also covers rebuilds forced by should_force_rebuild. Materialized views and all other engines are unchanged. Signed-off-by: Gandeevan Raghuraman <gandeevan.raghuraman@doordash.com>
1 parent d69c262 commit 1d0c901

4 files changed

Lines changed: 76 additions & 1 deletion

File tree

‎sqlmesh/core/engine_adapter/base.py‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,7 @@ class EngineAdapter:
114114
SCHEMA_DIFFER_KWARGS: t.Dict[str, t.Any] = {}
115115
SUPPORTS_TUPLE_IN = True
116116
HAS_VIEW_BINDING = False
117+
RECREATE_VIEW_ON_EVALUATION = True
117118
RECREATE_MATERIALIZED_VIEW_ON_EVALUATION = True
118119
SUPPORTS_REPLACE_TABLE = True
119120
SUPPORTS_GRANTS = False

‎sqlmesh/core/engine_adapter/redshift.py‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,10 @@ class RedshiftEngineAdapter(
5050
# Redshift doesn't support comments for VIEWs WITH NO SCHEMA BINDING (which we always use)
5151
COMMENT_CREATION_VIEW = CommentCreationView.UNSUPPORTED
5252
SUPPORTS_REPLACE_TABLE = False
53+
# Replacing a view (DROP + CREATE) gives it a new OID, which breaks concurrent queries that read through it.
54+
# Views WITH NO SCHEMA BINDING resolve their dependencies at query time, so an existing view only needs to be
55+
# recreated on the first insert of a snapshot version.
56+
RECREATE_VIEW_ON_EVALUATION = False
5357
SUPPORTS_GRANTS = True
5458
SUPPORTS_MULTIPLE_GRANT_PRINCIPALS = True
5559

‎sqlmesh/core/snapshot/evaluator.py‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2750,6 +2750,16 @@ def insert(
27502750
):
27512751
must_recreate_view = False
27522752

2753+
# Some engines (e.g. Redshift) replace views via DROP + CREATE, which breaks concurrent queries that read
2754+
# through the view. For those, an existing view must not be recreated on routine evaluation; only on the
2755+
# first insert, which also covers rebuilds forced by `should_force_rebuild`.
2756+
if (
2757+
not is_materialized_view
2758+
and not is_first_insert
2759+
and not self.adapter.RECREATE_VIEW_ON_EVALUATION
2760+
):
2761+
must_recreate_view = False
2762+
27532763
if self.adapter.table_exists(table_name) and not must_recreate_view:
27542764
logger.info("Skipping creation of the view '%s'", table_name)
27552765
return

‎tests/core/test_snapshot_evaluator.py‎

Lines changed: 61 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,13 @@
1818
from sqlmesh.core.audit import ModelAudit, StandaloneAudit
1919
from sqlmesh.core import dialect as d
2020
from sqlmesh.core.dialect import schema_, to_schema
21-
from sqlmesh.core.engine_adapter import EngineAdapter, create_engine_adapter, BigQueryEngineAdapter
21+
from sqlmesh.core.engine_adapter import (
22+
EngineAdapter,
23+
create_engine_adapter,
24+
BigQueryEngineAdapter,
25+
RedshiftEngineAdapter,
26+
SnowflakeEngineAdapter,
27+
)
2228
from sqlmesh.core.engine_adapter.base import MERGE_SOURCE_ALIAS, MERGE_TARGET_ALIAS
2329
from sqlmesh.core.engine_adapter.shared import (
2430
DataObject,
@@ -81,6 +87,7 @@
8187
)
8288
from sqlmesh.utils.metaprogramming import Executable
8389
from sqlmesh.utils.pydantic import list_of_fields_validator
90+
from tests.core.engine_adapter import to_sql_calls
8491

8592

8693
if t.TYPE_CHECKING:
@@ -764,6 +771,59 @@ def test_evaluate_materialized_view_not_recreated_on_evaluation(
764771
assert adapter_mock.create_view.call_count == expected_create_view_calls
765772

766773

774+
@pytest.mark.parametrize(
775+
"adapter_cls, materialized, has_intervals, expect_recreate",
776+
[
777+
# Existing view with intervals -> routine evaluation: do NOT recreate on Redshift
778+
(RedshiftEngineAdapter, False, True, False),
779+
# Existing view without intervals -> first insert (e.g. `should_force_rebuild`): recreate it
780+
(RedshiftEngineAdapter, False, False, True),
781+
# Materialized views are still recreated on every evaluation
782+
(RedshiftEngineAdapter, True, True, True),
783+
# Other engines without view binding still recreate existing views on every evaluation
784+
(SnowflakeEngineAdapter, False, True, True),
785+
(SnowflakeEngineAdapter, False, False, True),
786+
(SnowflakeEngineAdapter, True, True, True),
787+
],
788+
)
789+
def test_evaluate_existing_view_recreation(
790+
mocker: MockerFixture,
791+
make_mocked_engine_adapter,
792+
make_snapshot,
793+
adapter_cls: t.Type[EngineAdapter],
794+
materialized: bool,
795+
has_intervals: bool,
796+
expect_recreate: bool,
797+
):
798+
adapter = make_mocked_engine_adapter(adapter_cls)
799+
adapter.with_settings = lambda **kwargs: adapter # type: ignore
800+
mocker.patch.object(adapter, "table_exists", return_value=True)
801+
evaluator = SnapshotEvaluator(adapter)
802+
803+
model = SqlModel(
804+
name="test_schema.test_model",
805+
kind=ViewKind(materialized=materialized),
806+
query=parse_one("SELECT a FROM tbl"),
807+
)
808+
snapshot = make_snapshot(model)
809+
snapshot.categorize_as(SnapshotChangeCategory.BREAKING)
810+
if has_intervals:
811+
snapshot.add_interval("2023-01-01", "2023-01-01")
812+
813+
evaluator.evaluate(
814+
snapshot,
815+
start="2020-01-01",
816+
end="2020-01-02",
817+
execution_time="2020-01-02",
818+
snapshots={},
819+
)
820+
821+
view_ddl = [
822+
sql for sql in to_sql_calls(adapter) if sql.startswith(("CREATE", "DROP")) and "VIEW" in sql
823+
]
824+
assert bool(view_ddl) == expect_recreate
825+
826+
767827
def test_evaluate_materialized_view_with_partitioned_by_cluster_by(
768828
mocker: MockerFixture, adapter_mock, make_snapshot
769829
):

0 commit comments

Comments
 (0)