Skip to content

Commit

Permalink
Disable access policy rewrite when compiling constraints (#4248)
Browse files Browse the repository at this point in the history 
Access policies are applied _before_ the constraint is checked anyway
and performing rewrites confuses constraint IR analysis.  This was
partially fixed in #3892, but not all the way.

Fixes: #4245
  • Loading branch information
@elprans
elprans committed Aug 31, 2022
1 parent 4bbe722 commit 83719c6
Show file tree
Hide file tree
Showing 2 changed files with 145 additions and 104 deletions.
4 changes: 2 additions & 2 deletions edb/pgsql/schemamech.py
View file
Expand Up @@ -171,7 +171,7 @@ def schema_constraint_to_backend_constraint(
options = qlcompiler.CompilerOptions(
anchors={qlast.Subject().name: subject},
path_prefix_anchor=qlast.Subject().name,
apply_query_rewrites=not context.stdmode,
apply_query_rewrites=False,
singletons=singletons,
# Remap the constraint origin to the subject, so that if
# we have B <: A, and the constraint references A.foo, it
Expand Down Expand Up @@ -244,7 +244,7 @@ def schema_constraint_to_backend_constraint(
origin_options = qlcompiler.CompilerOptions(
anchors={qlast.Subject().name: origin_subject},
path_prefix_anchor=origin_path_prefix_anchor,
apply_query_rewrites=not context.stdmode,
apply_query_rewrites=False,
singletons=singletons,
)

Expand Down
245 changes: 143 additions & 102 deletions tests/test_edgeql_data_migration.py
View file
Expand Up @@ -10865,108 +10865,6 @@ async def test_edgeql_migration_uuid_array_01(self):
}
''')


class TestEdgeQLDataMigrationNonisolated(EdgeQLDataMigrationTestCase):
TRANSACTION_ISOLATION = False

async def test_edgeql_migration_eq_collections_25(self):
await self.con.execute(r"""
START MIGRATION TO {
module test {
alias Foo := [20];
}
};
POPULATE MIGRATION;
COMMIT MIGRATION;
""")

await self.con.execute(r"""
START MIGRATION TO {
module test {
}
};
POPULATE MIGRATION;
COMMIT MIGRATION;
""")

async def test_edgeql_ddl_collection_cleanup_06(self):
for _ in range(2):
await self.con.execute(r"""
CREATE FUNCTION cleanup_06(
a: int64
) -> tuple<int64, tuple<int64>>
USING EdgeQL $$
SELECT (a, ((a + 1),))
$$;
""")

await self.con.execute(r"""
DROP FUNCTION cleanup_06(a: int64)
""")

async def test_edgeql_migration_enum_01(self):
# Test some enum stuff. This needs to be nonisolated because postgres
# won't let you *use* an enum until it has been committed!
await self.migrate('''
scalar type Status extending enum<pending, in_progress, finished>;
scalar type ImportStatus extending Status;
scalar type ImportAnalyticsStatus extending Status;
type Foo { property x -> ImportStatus };
''')
await self.con.execute('''
with module test
insert Foo { x := 'pending' };
''')

await self.migrate('''
scalar type Status extending enum<
pending, in_progress, finished, wontfix>;
scalar type ImportStatus extending Status;
scalar type ImportAnalyticsStatus extending Status;
type Foo { property x -> ImportStatus };
function f(x: Status) -> str USING (<str>x);
''')

await self.migrate('''
scalar type Status extending enum<
pending, in_progress, finished, wontfix, again>;
scalar type ImportStatus extending Status;
scalar type ImportAnalyticsStatus extending Status;
type Foo { property x -> ImportStatus };
function f(x: Status) -> str USING (<str>x);
''')

await self.assert_query_result(
r"""
with module test
select <ImportStatus>'wontfix'
""",
['wontfix'],
)

await self.assert_query_result(
r"""
with module test
select f(<ImportStatus>'wontfix')
""",
['wontfix'],
)

await self.migrate('''
scalar type Status extending enum<
pending, in_progress, wontfix, again>;
scalar type ImportStatus extending Status;
scalar type ImportAnalyticsStatus extending Status;
type Foo { property x -> ImportStatus };
function f(x: Status) -> str USING (<str>x);
''')

await self.migrate('')

async def test_edgeql_migration_on_target_delete_01(self):
await self.migrate(
r"""
Expand Down Expand Up @@ -11086,6 +10984,22 @@ async def test_edgeql_migration_access_policy_01(self):
}
""")

async def test_edgeql_migration_access_policy_constraint(self):
# Make sure policies don't interfere with constraints.
await self.migrate(r"""
abstract type Base {
access policy locked allow all using (false);
}
type Tgt extending Base;
type Src {
required link tgt -> Tgt {
constraint exclusive;
}
}
""")

async def test_edgeql_migration_globals_01(self):
schema = r"""
global current_user_id -> uuid;
Expand Down Expand Up @@ -11141,6 +11055,131 @@ async def test_edgeql_migration_globals_02(self):
await self.migrate(schema)
await self.migrate(schema)

@test.xerror('''
Infinite recursion via _propagate_if_expr_refs
''')
async def test_edgeql_migration_policies_and_collections(self):
# An infinite recursion bug with this this was found by accident
# when a number of tests accidentally were in the non isolated test.
# (Simplified a bit.)
await self.migrate(r"""
abstract type Base {
access policy locked allow all using (false);
}
type Src {
required link tgt -> Base {
constraint exclusive;
}
}
""")

await self.migrate(r"""
alias Foo := 20;
""")


class TestEdgeQLDataMigrationNonisolated(EdgeQLDataMigrationTestCase):
TRANSACTION_ISOLATION = False

async def test_edgeql_migration_eq_collections_25(self):
await self.con.execute(r"""
START MIGRATION TO {
module test {
alias Foo := [20];
}
};
POPULATE MIGRATION;
COMMIT MIGRATION;
""")

await self.con.execute(r"""
START MIGRATION TO {
module test {
}
};
POPULATE MIGRATION;
COMMIT MIGRATION;
""")

async def test_edgeql_ddl_collection_cleanup_06(self):
for _ in range(2):
await self.con.execute(r"""
CREATE FUNCTION cleanup_06(
a: int64
) -> tuple<int64, tuple<int64>>
USING EdgeQL $$
SELECT (a, ((a + 1),))
$$;
""")

await self.con.execute(r"""
DROP FUNCTION cleanup_06(a: int64)
""")

async def test_edgeql_migration_enum_01(self):
# Test some enum stuff. This needs to be nonisolated because postgres
# won't let you *use* an enum until it has been committed!
await self.migrate('''
scalar type Status extending enum<pending, in_progress, finished>;
scalar type ImportStatus extending Status;
scalar type ImportAnalyticsStatus extending Status;
type Foo { property x -> ImportStatus };
''')
await self.con.execute('''
with module test
insert Foo { x := 'pending' };
''')

await self.migrate('''
scalar type Status extending enum<
pending, in_progress, finished, wontfix>;
scalar type ImportStatus extending Status;
scalar type ImportAnalyticsStatus extending Status;
type Foo { property x -> ImportStatus };
function f(x: Status) -> str USING (<str>x);
''')

await self.migrate('''
scalar type Status extending enum<
pending, in_progress, finished, wontfix, again>;
scalar type ImportStatus extending Status;
scalar type ImportAnalyticsStatus extending Status;
type Foo { property x -> ImportStatus };
function f(x: Status) -> str USING (<str>x);
''')

await self.assert_query_result(
r"""
with module test
select <ImportStatus>'wontfix'
""",
['wontfix'],
)

await self.assert_query_result(
r"""
with module test
select f(<ImportStatus>'wontfix')
""",
['wontfix'],
)

await self.migrate('''
scalar type Status extending enum<
pending, in_progress, wontfix, again>;
scalar type ImportStatus extending Status;
scalar type ImportAnalyticsStatus extending Status;
type Foo { property x -> ImportStatus };
function f(x: Status) -> str USING (<str>x);
''')

await self.migrate('')

async def test_edgeql_migration_recovery(self):
await self.con.execute(r"""
START MIGRATION TO {
Expand Down Expand Up @@ -11221,6 +11260,8 @@ async def test_edgeql_migration_recovery_in_script(self):
""")
self.assertEqual(res, [])

await self.migrate('')

async def test_edgeql_migration_recovery_commit_fail(self):
con2 = await self.connect(database=self.con.dbname)
try:
Expand Down

0 comments on commit 83719c6

Please sign in to comment.