Skip to content

Commit

Permalink
Fix issue with abstract types, exclusive constraints, and ANALYZE. (#…
Browse files Browse the repository at this point in the history
…7454)

When we have inheritance views expanded (either by config or because
of ANALYZE), we fail to raise exclusive constraint errors caused by
`UPDATE`ing an abstract type such that two different subtypes have
conflicting values for a multi property. (See
`test_constraints_abstract_object_01`.)

This is because that query relies on explicitly looking for conflicts
in the query we generate, and doing that relies on pointer overlays
being used to look at the new values.

The fix in `range_for_ptrref` is to expand the inhviews while keeping them grouped based on which abstract type they are part of, then to add the overlays for the abstract type onto that expanded group.

---------

Co-authored-by: Michael J. Sullivan <[email protected]>
  • Loading branch information
dnwpark and msullivan authored Jun 13, 2024
1 parent 6070769 commit a2b0c3b
Show file tree
Hide file tree
Showing 2 changed files with 80 additions and 62 deletions.
134 changes: 72 additions & 62 deletions edb/pgsql/compiler/relctx.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
List,
Set,
NamedTuple,
cast,
)

import uuid
Expand Down Expand Up @@ -1946,17 +1947,13 @@ def range_for_ptrref(
refs = {ptrref.computed_link_alias}
else:
refs = {ptrref}
assert isinstance(ptrref, irast.PointerRef), \
"expected regular PointerRef"
overlays = get_ptr_rel_overlays(
ptrref, dml_source=dml_source, ctx=ctx)

include_descendants = not ptrref.union_is_exhaustive

assert isinstance(ptrref.out_source.name_hint, sn.QualName)
# expand_inhviews helps support EXPLAIN. see
# range_for_material_objtype for details.
lrefs: List[irast.BasePointerRef]
lrefs: dict[irast.PointerRef, list[irast.PointerRef]]
if (
ctx.env.expand_inhviews
and include_descendants
Expand All @@ -1966,78 +1963,91 @@ def range_for_ptrref(
and ptrref.out_source.name_hint.module not in {'sys', 'cfg'}
):
include_descendants = False
lrefs = []
lrefs = {}
for ref in list(refs):
lrefs.extend(ref.descendants())
lrefs.append(ref)
concrete_lrefs = [
ref for ref in lrefs if not ref.out_source.is_abstract
]
# If there aren't any concrete types, we still need to
# generate *something*, so just do all the abstract ones.
if concrete_lrefs:
lrefs = concrete_lrefs
else:
lrefs = list(refs)

for src_ptrref in lrefs:
assert isinstance(src_ptrref, irast.PointerRef), \
"expected regular PointerRef"

# Most references to inline links are dispatched to a separate
# code path (_new_inline_pointer_rvar) by new_pointer_rvar,
# but when we have union pointers, some might be inline. We
# always use the link table if it exists (because this range
# needs to contain any link properties, for one reason.)
ptr_info = pg_types.get_ptrref_storage_info(
src_ptrref, resolve_type=False, link_bias=True,
)
if not ptr_info:
assert ptrref.union_components
assert isinstance(ref, irast.PointerRef), \
"expected regular PointerRef"
lref_entries: list[irast.PointerRef] = []
lref_entries.extend(
cast(Iterable[irast.PointerRef], ref.descendants())
)
lref_entries.append(ref)
assert isinstance(ref, irast.PointerRef)

# Try to only select from actual concrete types.
concrete_lrefs = [
ref for ref in lref_entries if not ref.out_source.is_abstract
]
# If there aren't any concrete types, we still need to
# generate *something*, so just do all the abstract ones.
if concrete_lrefs:
lrefs[ref] = concrete_lrefs
else:
lrefs[ref] = lref_entries

else:
lrefs = {}
for ref in list(refs):
assert isinstance(ref, irast.PointerRef), \
"expected regular PointerRef"
lrefs[ref] = [ref]

for orig_ptrref, src_ptrrefs in lrefs.items():
for src_ptrref in src_ptrrefs:
# Most references to inline links are dispatched to a separate
# code path (_new_inline_pointer_rvar) by new_pointer_rvar,
# but when we have union pointers, some might be inline. We
# always use the link table if it exists (because this range
# needs to contain any link properties, for one reason.)
ptr_info = pg_types.get_ptrref_storage_info(
src_ptrref, resolve_type=False, link_bias=False,
src_ptrref, resolve_type=False, link_bias=True,
)
if not ptr_info:
assert ptrref.union_components

cols = [
'source' if ptr_info.table_type == 'link' else 'id',
ptr_info.column_name,
]
ptr_info = pg_types.get_ptrref_storage_info(
src_ptrref, resolve_type=False, link_bias=False,
)

table = table_from_ptrref(
src_ptrref,
ptr_info,
include_descendants=include_descendants,
for_mutation=for_mutation,
ctx=ctx,
)
table.query.path_id = path_id
cols = [
'source' if ptr_info.table_type == 'link' else 'id',
ptr_info.column_name,
]

qry = pgast.SelectStmt()
qry.from_clause.append(table)
table = table_from_ptrref(
src_ptrref,
ptr_info,
include_descendants=include_descendants,
for_mutation=for_mutation,
ctx=ctx,
)
table.query.path_id = path_id

# Make sure all property references are pulled up properly
for colname, output_colname in zip(cols, output_cols):
selexpr = pgast.ColumnRef(
name=[table.alias.aliasname, colname])
qry.target_list.append(
pgast.ResTarget(val=selexpr, name=output_colname))
qry = pgast.SelectStmt()
qry.from_clause.append(table)

set_ops.append(('union', qry))
# Make sure all property references are pulled up properly
for colname, output_colname in zip(cols, output_cols):
selexpr = pgast.ColumnRef(
name=[table.alias.aliasname, colname])
qry.target_list.append(
pgast.ResTarget(val=selexpr, name=output_colname))

set_ops.append(('union', qry))

# We need the identity var for semi_join to work and
# the source rvar so that linkprops can be found here.
if path_id:
target_ref = qry.target_list[1].val
pathctx.put_path_identity_var(qry, path_id, var=target_ref)
pathctx.put_path_source_rvar(qry, path_id, table)
# We need the identity var for semi_join to work and
# the source rvar so that linkprops can be found here.
if path_id:
target_ref = qry.target_list[1].val
pathctx.put_path_identity_var(qry, path_id, var=target_ref)
pathctx.put_path_source_rvar(qry, path_id, table)

# Only fire off the overlays at the end of each expanded inhview.
# This only matters when we are doing expand_inhviews, and prevents
# us from repeating the overlays many times in that case.
if src_ptrref in refs and not for_mutation:
if orig_ptrref in refs and not for_mutation:
overlays = get_ptr_rel_overlays(
src_ptrref, dml_source=dml_source, ctx=ctx)
orig_ptrref, dml_source=dml_source, ctx=ctx)

for op, cte, cte_path_id in overlays:
rvar = rvar_for_rel(cte, ctx=ctx)
Expand Down
8 changes: 8 additions & 0 deletions tests/test_constraints.py
Original file line number Diff line number Diff line change
Expand Up @@ -2063,6 +2063,14 @@ async def test_constraints_abstract_object_01(self):
async with self.assertRaisesRegexTx(
edgedb.ConstraintViolationError,
"messages violates exclusivity constraint"
):
await self.con.execute("""
analyze
update ChatBase set { messages += 'hello world' };
""")
async with self.assertRaisesRegexTx(
edgedb.ConstraintViolationError,
"messages violates exclusivity constraint"
):
await self.con.execute("""
update ChatBase set { messages := 'hello world' };
Expand Down

0 comments on commit a2b0c3b

Please sign in to comment.