diff --git a/edb/common/debug.py b/edb/common/debug.py index f3d445b741d..5b719181497 100644 --- a/edb/common/debug.py +++ b/edb/common/debug.py @@ -116,9 +116,6 @@ class flags(metaclass=FlagsMeta): edgeql_disable_normalization = Flag( doc="Disable EdgeQL normalization (constant extraction etc)") - edgeql_expand_inhviews = Flag( - doc="Force the EdgeQL compiler to expand inhviews *always*") - graphql_compile = Flag( doc="Debug GraphQL compiler.") diff --git a/edb/edgeql/compiler/options.py b/edb/edgeql/compiler/options.py index 1b4d698fe6f..d76c2171f7e 100644 --- a/edb/edgeql/compiler/options.py +++ b/edb/edgeql/compiler/options.py @@ -77,10 +77,10 @@ class GlobalCompilerOptions: #: definitions. func_params: Optional[s_func.ParameterLikeList] = None - #: Should the backend compiler expand out inheritance views instead of - #: using them. This is needed by EXPLAIN to maintain alias names in + #: Should the backend compiler expand inheritance CTEs in place. + #: This is needed by EXPLAIN to maintain alias names in #: the query plan. - expand_inhviews: bool = False + is_explain: bool = False #: The name that can be used in a "DML is disallowed in ..." #: error. When this is not None, any DML should cause an error. diff --git a/edb/edgeql/compiler/typegen.py b/edb/edgeql/compiler/typegen.py index d8b5e78e26a..e172a7647c1 100644 --- a/edb/edgeql/compiler/typegen.py +++ b/edb/edgeql/compiler/typegen.py @@ -401,10 +401,7 @@ def type_to_typeref( include_children = ( expr_type is s_types.ExprType.Update or expr_type is s_types.ExprType.Delete - or ( - env.options.expand_inhviews - and isinstance(t, s_objtypes.ObjectType) - ) + or isinstance(t, s_objtypes.ObjectType) ) include_ancestors = ( expr_type is s_types.ExprType.Insert diff --git a/edb/ir/ast.py b/edb/ir/ast.py index bbaff11ac20..9559f05a022 100644 --- a/edb/ir/ast.py +++ b/edb/ir/ast.py @@ -173,6 +173,8 @@ class TypeRef(ImmutableBase): is_scalar: bool = False # True, if this describes a view is_view: bool = False + # True, if this describes a cfg view + is_cfg_view: bool = False # True, if this describes an abstract type is_abstract: bool = False # True, if the collection type is persisted in the schema diff --git a/edb/ir/pathid.py b/edb/ir/pathid.py index 4da6d4794b7..b34475c9e44 100644 --- a/edb/ir/pathid.py +++ b/edb/ir/pathid.py @@ -257,8 +257,6 @@ def from_typeref( which case it is used instead. Args: - schema: - A schema instance where the type *t* is defined. typeref: The descriptor of a type of the variable being defined. namespace: @@ -278,6 +276,28 @@ def from_typeref( pid._namespace = frozenset(namespace) return pid + @classmethod + def from_ptrref( + cls, + ptrref: irast.PointerRef, + *, + namespace: AbstractSet[Namespace] = frozenset(), + ) -> PathId: + """Return a ``PathId`` instance for a given :class:`ir.ast.PointerRef` + + Args: + ptrref: + The descriptor of a ptr of the variable being defined. + namespace: + Optional namespace in which the variable is defined. + + Returns: + A ``PathId`` instance of type described by *ptrref*. + """ + pid = cls.from_typeref(ptrref.out_source, namespace=namespace) + pid = pid.extend(ptrref=ptrref) + return pid + @classmethod def new_dummy(cls, name: str) -> PathId: name_hint = s_name.QualName(module='__derived__', name=name) diff --git a/edb/ir/typeutils.py b/edb/ir/typeutils.py index 6a5ae22bbd2..934a5be0201 100644 --- a/edb/ir/typeutils.py +++ b/edb/ir/typeutils.py @@ -61,6 +61,36 @@ TypeRefCache = dict[TypeRefCacheKey, 'irast.TypeRef'] +# Modules where all the "types" in them are really just custom views +# provided by metaschema. +VIEW_MODULES = ('sys', 'cfg') + + +def is_cfg_view( + obj: s_obj.Object, + schema: s_schema.Schema, +) -> bool: + return ( + isinstance(obj, (s_objtypes.ObjectType, s_pointers.Pointer)) + and ( + obj.get_name(schema).module in VIEW_MODULES + or bool( + (cfg_object := schema.get( + 'cfg::ConfigObject', + type=s_objtypes.ObjectType, default=None + )) + and ( + nobj := ( + obj if isinstance(obj, s_objtypes.ObjectType) + else obj.get_source(schema) + ) + ) + and nobj.issubclass(schema, cfg_object) + ) + ) + ) + + def is_scalar(typeref: irast.TypeRef) -> bool: """Return True if *typeref* describes a scalar type.""" return typeref.is_scalar @@ -400,6 +430,7 @@ def _typeref( is_scalar=t.is_scalar(), is_abstract=t.get_abstract(schema), is_view=t.is_view(schema), + is_cfg_view=is_cfg_view(t, schema), is_opaque_union=t.get_is_opaque_union(schema), needs_custom_json_cast=needs_custom_json_cast, sql_type=sql_type, diff --git a/edb/pgsql/compiler/__init__.py b/edb/pgsql/compiler/__init__.py index 344d2096de6..a78f5def14e 100644 --- a/edb/pgsql/compiler/__init__.py +++ b/edb/pgsql/compiler/__init__.py @@ -66,7 +66,7 @@ def compile_ir_to_sql_tree( singleton_mode: bool = False, named_param_prefix: Optional[tuple[str, ...]] = None, expected_cardinality_one: bool = False, - expand_inhviews: bool = False, + is_explain: bool = False, external_rvars: Optional[ Mapping[Tuple[irast.PathId, pgce.PathAspect], pgast.PathRangeVar] ] = None, @@ -127,7 +127,7 @@ def compile_ir_to_sql_tree( type_rewrites=type_rewrites, ignore_object_shapes=ignore_shapes, explicit_top_cast=explicit_top_cast, - expand_inhviews=expand_inhviews, + is_explain=is_explain, singleton_mode=singleton_mode, scope_tree_nodes=scope_tree_nodes, external_rvars=external_rvars, @@ -156,9 +156,15 @@ def compile_ir_to_sql_tree( qtree = dispatch.compile(ir_expr, ctx=ctx) dml.compile_triggers(triggers, qtree, ctx=ctx) - if isinstance(ir_expr, irast.Set) and not singleton_mode: - assert isinstance(qtree, pgast.Query) - clauses.fini_toplevel(qtree, ctx) + if not singleton_mode: + if isinstance(ir_expr, irast.Set): + assert isinstance(qtree, pgast.Query) + clauses.fini_toplevel(qtree, ctx) + + elif isinstance(qtree, pgast.Query): + # Other types of expressions may compile to queries which may + # use inheritance CTEs. Ensure they are added here. + clauses.insert_ctes(qtree, ctx) if detach_params: detached_params_idx = { diff --git a/edb/pgsql/compiler/clauses.py b/edb/pgsql/compiler/clauses.py index f921d38819e..f8528b525ea 100644 --- a/edb/pgsql/compiler/clauses.py +++ b/edb/pgsql/compiler/clauses.py @@ -409,20 +409,26 @@ def scan_check_ctes( )) -def fini_toplevel( - stmt: pgast.Query, ctx: context.CompilerContextLevel) -> None: - - scan_check_ctes(stmt, ctx.env.check_ctes, ctx=ctx) - - # Type rewrites go first. +def insert_ctes( + stmt: pgast.Query, ctx: context.CompilerContextLevel +) -> None: if stmt.ctes is None: stmt.ctes = [] stmt.ctes[:0] = [ *ctx.param_ctes.values(), - *ctx.ptr_ctes.values(), - *ctx.type_ctes.values(), + *ctx.ptr_inheritance_ctes.values(), + *ctx.ordered_type_ctes, ] + +def fini_toplevel( + stmt: pgast.Query, ctx: context.CompilerContextLevel) -> None: + + scan_check_ctes(stmt, ctx.env.check_ctes, ctx=ctx) + + # Type rewrites and inheritance CTEs go first. + insert_ctes(stmt, ctx) + if ctx.env.named_param_prefix is None: # Adding unused parameters into a CTE diff --git a/edb/pgsql/compiler/context.py b/edb/pgsql/compiler/context.py index d4ee0fd3c52..ec50561b8c6 100644 --- a/edb/pgsql/compiler/context.py +++ b/edb/pgsql/compiler/context.py @@ -242,15 +242,21 @@ class CompilerContextLevel(compiler.ContextLevel): #: CTEs representing decoded parameters param_ctes: Dict[str, pgast.CommonTableExpr] - #: CTEs representing pointer tables that we need to force to be - #: materialized for performance reasons. - ptr_ctes: Dict[uuid.UUID, pgast.CommonTableExpr] + #: CTEs representing pointers and their inherited pointers + ptr_inheritance_ctes: Dict[uuid.UUID, pgast.CommonTableExpr] #: CTEs representing types, when rewritten based on access policy - type_ctes: Dict[FullRewriteKey, pgast.CommonTableExpr] + type_rewrite_ctes: Dict[FullRewriteKey, pgast.CommonTableExpr] #: A set of type CTEs currently being generated - pending_type_ctes: Set[RewriteKey] + pending_type_rewrite_ctes: Set[RewriteKey] + + #: CTEs representing types and their inherited types + type_inheritance_ctes: Dict[uuid.UUID, pgast.CommonTableExpr] + + # Type and type inheriance CTEs in creation order. This ensures type CTEs + # referring to other CTEs are in the correct order. + ordered_type_ctes: list[pgast.CommonTableExpr] #: The logical parent of the current query in the #: query hierarchy @@ -351,9 +357,11 @@ def __init__( self.rel = NO_STMT self.rel_hierarchy = {} self.param_ctes = {} - self.ptr_ctes = {} - self.type_ctes = {} - self.pending_type_ctes = set() + self.ptr_inheritance_ctes = {} + self.type_rewrite_ctes = {} + self.pending_type_rewrite_ctes = set() + self.type_inheritance_ctes = {} + self.ordered_type_ctes = [] self.dml_stmts = {} self.parent_rel = None self.pending_query = None @@ -391,9 +399,11 @@ def __init__( self.rel = prevlevel.rel self.rel_hierarchy = prevlevel.rel_hierarchy self.param_ctes = prevlevel.param_ctes - self.ptr_ctes = prevlevel.ptr_ctes - self.type_ctes = prevlevel.type_ctes - self.pending_type_ctes = prevlevel.pending_type_ctes + self.ptr_inheritance_ctes = prevlevel.ptr_inheritance_ctes + self.type_rewrite_ctes = prevlevel.type_rewrite_ctes + self.pending_type_rewrite_ctes = prevlevel.pending_type_rewrite_ctes + self.type_inheritance_ctes = prevlevel.type_inheritance_ctes + self.ordered_type_ctes = prevlevel.ordered_type_ctes self.dml_stmts = prevlevel.dml_stmts self.parent_rel = prevlevel.parent_rel self.pending_query = prevlevel.pending_query @@ -456,7 +466,9 @@ def __init__( self.disable_semi_join = frozenset() self.force_optional = frozenset() self.intersection_narrowing = {} - self.pending_type_ctes = set(prevlevel.pending_type_ctes) + self.pending_type_rewrite_ctes = set( + prevlevel.pending_type_rewrite_ctes + ) elif mode == ContextSwitchMode.NEWSCOPE: self.path_scope = prevlevel.path_scope.new_child() @@ -536,7 +548,7 @@ def __init__( expected_cardinality_one: bool, ignore_object_shapes: bool, singleton_mode: bool, - expand_inhviews: bool, + is_explain: bool, explicit_top_cast: Optional[irast.TypeRef], query_params: List[irast.Param], type_rewrites: Dict[RewriteKey, irast.Set], @@ -555,7 +567,7 @@ def __init__( self.expected_cardinality_one = expected_cardinality_one self.ignore_object_shapes = ignore_object_shapes self.singleton_mode = singleton_mode - self.expand_inhviews = expand_inhviews + self.is_explain = is_explain self.explicit_top_cast = explicit_top_cast self.query_params = query_params self.type_rewrites = type_rewrites diff --git a/edb/pgsql/compiler/dml.py b/edb/pgsql/compiler/dml.py index 6f5d7a1311a..3fcb2803b12 100644 --- a/edb/pgsql/compiler/dml.py +++ b/edb/pgsql/compiler/dml.py @@ -3139,7 +3139,9 @@ def compile_triggers( # FIXME: I think we actually need to keep the old type_ctes # available for pointers off of __old__ to use. ictx.trigger_mode = True - ictx.type_ctes = {} + ictx.type_rewrite_ctes = {} + ictx.type_inheritance_ctes = {} + ictx.ordered_type_ctes = [] ictx.toplevel_stmt = stmt for stage in triggers: @@ -3156,7 +3158,7 @@ def compile_triggers( # Install any newly created type CTEs before the CTEs created from # this trigger compilation but after anything compiled before. - stmt.ctes[start_ctes:start_ctes] = list(ictx.type_ctes.values()) + stmt.ctes[start_ctes:start_ctes] = ictx.ordered_type_ctes def compile_trigger( diff --git a/edb/pgsql/compiler/relctx.py b/edb/pgsql/compiler/relctx.py index 29e0af2cf20..e3471dcd535 100644 --- a/edb/pgsql/compiler/relctx.py +++ b/edb/pgsql/compiler/relctx.py @@ -1546,16 +1546,12 @@ def range_for_material_objtype( ctx: context.CompilerContextLevel, ) -> pgast.PathRangeVar: - env = ctx.env - if not is_global: typeref = typeref.real_material_type if not is_global and not path_id.is_objtype_path(): raise ValueError('cannot create root rvar for non-object path') - relation: Union[pgast.Relation, pgast.CommonTableExpr] - assert isinstance(typeref.name_hint, sn.QualName) dml_source_key = ( @@ -1570,7 +1566,7 @@ def range_for_material_objtype( (rewrite := ctx.env.type_rewrites.get(rw_key)) is not None or force_cte ) - and rw_key not in ctx.pending_type_ctes + and rw_key not in ctx.pending_type_rewrite_ctes and not for_mutation ): if not rewrite: @@ -1596,9 +1592,9 @@ def range_for_material_objtype( include_overlays = False type_rel: pgast.BaseRelation | pgast.CommonTableExpr - if (type_cte := ctx.type_ctes.get(key)) is None: + if (type_cte := ctx.type_rewrite_ctes.get(key)) is None: with ctx.newrel() as sctx: - sctx.pending_type_ctes.add(rw_key) + sctx.pending_type_rewrite_ctes.add(rw_key) sctx.pending_query = sctx.rel # Normally we want to compile type rewrites without # polluting them with any sort of overlays, but when @@ -1609,10 +1605,10 @@ def range_for_material_objtype( sctx.rel_overlays = context.RelOverlays() dispatch.visit(rewrite, ctx=sctx) - # If we are expanding inhviews, we also expand type + # If we are explaining, we also expand type # rewrites, so don't populate type_ctes. The normal # case is to stick it in a CTE and cache that, though. - if ctx.env.expand_inhviews and not is_global: + if ctx.env.is_explain and not is_global: type_rel = sctx.rel else: type_cte = pgast.CommonTableExpr( @@ -1620,7 +1616,8 @@ def range_for_material_objtype( query=sctx.rel, materialized=is_global or force_cte, ) - ctx.type_ctes[key] = type_cte + ctx.type_rewrite_ctes[key] = type_cte + ctx.ordered_type_ctes.append(type_cte) type_rel = type_cte else: type_rel = type_cte @@ -1629,7 +1626,7 @@ def range_for_material_objtype( cte_rvar = rvar_for_rel( type_rel, typeref=typeref, - alias=env.aliases.get('t'), + alias=ctx.env.aliases.get('t'), ctx=ctx, ) pathctx.put_path_id_map(sctx.rel, path_id, rewrite.path_id) @@ -1640,77 +1637,95 @@ def range_for_material_objtype( rvar = rvar_for_rel( sctx.rel, lateral=lateral, typeref=typeref, ctx=sctx) - # When we are compiling a query for EXPLAIN, expand out type references - # to an explicit union of all the types, rather than relying on the - # inheritance views. This allows postgres to actually give us back the - # alias names that we use for relations, which we use to track which - # parts of the query are being referred to. - elif ( - ctx.env.expand_inhviews - and include_descendants - and not for_mutation - and typeref.children is not None + else: + assert not typeref.is_view, "attempting to generate range from view" + typeref_descendants = _get_typeref_descendants( + typeref, + include_descendants=include_descendants, + for_mutation=for_mutation, + ) + if ( + # When we are compiling a query for EXPLAIN, expand out type + # references to an explicit union of all the types, rather than + # using a CTE. This allows postgres to actually give us back the + # alias names that we use for relations, which we use to track which + # parts of the query are being referred to. + ctx.env.is_explain + + # Don't use CTEs if there is no inheritance. (ie. There is only a + # single material type) + or len(typeref_descendants) <= 1 + ): + inheritance_selects = _selects_for_typeref_descendants( + typeref_descendants, path_id, ctx=ctx, + ) + ops = [ + (context.OverlayOp.UNION, select) + for select in inheritance_selects + ] - # HACK: This is a workaround for #4491 - and typeref.name_hint.module not in {'cfg', 'sys'} - ): - ops = [] - typerefs = [typeref, *irtyputils.get_typeref_descendants(typeref)] - all_abstract = all(subref.is_abstract for subref in typerefs) - for subref in typerefs: - if subref.is_abstract and not all_abstract: - continue - rvar = range_for_material_objtype( - subref, path_id, lateral=lateral, - include_descendants=False, - include_overlays=False, - ignore_rewrites=ignore_rewrites, # XXX: Is this right? + rvar = range_from_queryset( + ops, + typeref.name_hint, + lateral=lateral, + path_id=path_id, + typeref=typeref, + tag='expanded-inhview', ctx=ctx, ) - qry = pgast.SelectStmt(from_clause=[rvar]) - sub_path_id = path_id - pathctx.put_path_value_rvar(qry, sub_path_id, rvar) - pathctx.put_path_source_rvar(qry, sub_path_id, rvar) - ops.append((context.OverlayOp.UNION, qry)) + else: + typeref_path: irast.PathId = irast.PathId.from_typeref( + typeref, + # If there are backlinks and the path revisits a type, a + # semi-join is produced. This ensures that the rvar produced + # does not have a duplicate path var. + # For example: (select A.b. list[irast.TypeRef]: + if ( + include_descendants + and not for_mutation + ): + descendants = [ + typeref, + *( + descendant + for descendant in irtyputils.get_typeref_descendants(typeref) + + # XXX: Exclude sys/cfg tables from non sys/cfg inheritance CTEs. + # This probably isn't *really* what we want to do, but until we + # figure that out, do *something* so that DDL isn't + # excruciatingly slow because of the cost of explicit id + # checks. See #5168. + if ( + not descendant.is_cfg_view + or typeref.is_cfg_view + ) + ) + ] + + # Try to only select from actual concrete types. + concrete_descendants = [ + subref for subref in descendants if not subref.is_abstract + ] + # If there aren't any concrete types, we still need to + # generate *something*, so just do the initial one. + if concrete_descendants: + return concrete_descendants + else: + return [typeref] + + else: + return [typeref] + + +def _selects_for_typeref_descendants( + typeref_descendants: Sequence[irast.TypeRef], + path_id: irast.PathId, + *, + ctx: context.CompilerContextLevel, +) -> list[pgast.SelectStmt]: + selects = [] + for subref in typeref_descendants: + rvar = _table_from_typeref( + subref, path_id, ctx=ctx, + ) + qry = pgast.SelectStmt(from_clause=[rvar]) + sub_path_id = path_id + pathctx.put_path_value_rvar(qry, sub_path_id, rvar) + pathctx.put_path_source_rvar(qry, sub_path_id, rvar) + + selects.append(qry) + + return selects + + +def _table_from_typeref( + typeref: irast.TypeRef, + path_id: irast.PathId, + *, + ctx: context.CompilerContextLevel, +) -> pgast.PathRangeVar: + assert isinstance(typeref.name_hint, sn.QualName) + + aspect = 'table' + + table_schema_name, table_name = common.get_objtype_backend_name( + typeref.id, + typeref.name_hint.module, + aspect=aspect, + catenate=False, + versioned=ctx.env.versioned_stdlib, + ) + + relation = pgast.Relation( + schemaname=table_schema_name, + name=table_name, + path_id=path_id, + type_or_ptr_ref=typeref, + ) + + return pgast.RelRangeVar( + relation=relation, + typeref=typeref, + alias=pgast.Alias( + aliasname=ctx.env.aliases.get(typeref.name_hint.name) + ) + ) + + def range_for_typeref( typeref: irast.TypeRef, path_id: irast.PathId, @@ -1945,6 +2058,25 @@ def range_from_queryset( typeref=typeref, ) + elif any( + ( + target.name is not None + and isinstance(target.val, pgast.ColumnRef) + and target.name != target.val.name[-1] + ) + for target in set_ops[0][1].target_list + ): + # A column name name is being changed + rvar = pgast.RangeSubselect( + subquery=set_ops[0][1], + lateral=lateral, + tag=tag, + alias=pgast.Alias( + aliasname=ctx.env.aliases.get(objname.name), + ), + typeref=typeref, + ) + else: # Just one class table, so return it directly from_rvar = set_ops[0][1].from_clause[0] @@ -1955,75 +2087,6 @@ def range_from_queryset( return rvar -def _make_link_table_cte( - ptrref: irast.PointerRef, - relation: pgast.Relation, - *, - ctx: context.CompilerContextLevel, -) -> pgast.Relation: - if (ptr_cte := ctx.ptr_ctes.get(ptrref.id)) is None: - ptr_select = pgast.SelectStmt( - target_list=[ - pgast.ResTarget(val=pgast.ColumnRef(name=[pgast.Star()])), - ], - from_clause=[pgast.RelRangeVar(relation=relation)], - ) - ptr_cte = pgast.CommonTableExpr( - name=ctx.env.aliases.get(f'p_{ptrref.name}'), - query=ptr_select, - materialized=True, - ) - ctx.ptr_ctes[ptrref.id] = ptr_cte - - # We've set up the CTE to be a perfect drop in replacement for - # the real table (with a select *), so instead of pointing the - # rvar at the CTE (which would require routing path ids), just - # treat it like a base relation. - return pgast.Relation( - name=ptr_cte.name, - type_or_ptr_ref=ptrref, - ) - - -def table_from_ptrref( - ptrref: irast.PointerRef, - ptr_info: pg_types.PointerStorageInfo, - *, - include_descendants: bool = True, - for_mutation: bool = False, - ctx: context.CompilerContextLevel, -) -> pgast.RelRangeVar: - """Return a Table corresponding to a given Link.""" - - aspect = 'table' if for_mutation or not include_descendants else 'inhview' - table_schema_name, table_name = common.update_aspect( - ptr_info.table_name, aspect - ) - - typeref = ptrref.out_source if ptrref else None - relation = pgast.Relation( - schemaname=table_schema_name, - name=table_name, - type_or_ptr_ref=ptrref, - ) - - if aspect == 'inhview' and _needs_cte(ptrref.out_source): - relation = _make_link_table_cte(ptrref, relation, ctx=ctx) - - # Pseudo pointers (tuple and type intersection) have no schema id. - sobj_id = ptrref.id if isinstance(ptrref, irast.PointerRef) else None - rvar = pgast.RelRangeVar( - schema_object_id=sobj_id, - typeref=typeref, - relation=relation, - alias=pgast.Alias( - aliasname=ctx.env.aliases.get(ptrref.shortname.name) - ) - ) - - return rvar - - def range_for_ptrref( ptrref: irast.BasePointerRef, *, dml_source: Sequence[irast.MutatingLikeStmt]=(), @@ -2039,11 +2102,9 @@ def range_for_ptrref( `ptrref` taking source inheritance into account. """ - output_cols = ('source', 'target') - if ptrref.union_components: - refs = ptrref.union_components - if only_self and len(refs) > 1: + component_refs = ptrref.union_components + if only_self and len(component_refs) > 1: raise errors.InternalServerError( 'unexpected union link' ) @@ -2051,181 +2112,406 @@ def range_for_ptrref( # This is a little funky, but in an intersection, the pointer # needs to appear in *all* of the tables, so we just pick any # one of them. - refs = {next(iter((ptrref.intersection_components)))} + component_refs = {next(iter((ptrref.intersection_components)))} elif ptrref.computed_link_alias: - refs = {ptrref.computed_link_alias} + component_refs = {ptrref.computed_link_alias} else: - refs = {ptrref} + component_refs = {ptrref} + assert isinstance(ptrref.out_source.name_hint, sn.QualName) 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: dict[irast.PointerRef, list[irast.PointerRef]] - if ( - ctx.env.expand_inhviews - and include_descendants - and not for_mutation + output_cols = ('source', 'target') - # HACK: This is a workaround for #4491 - and ptrref.out_source.name_hint.module not in {'sys', 'cfg'} - ): - include_descendants = False - lrefs = {} - for ref in list(refs): - assert isinstance(ref, irast.PointerRef), \ - "expected regular PointerRef" - lref_entries: list[irast.PointerRef] = [] - lref_entries.extend( - cast(Iterable[irast.PointerRef], ref.descendants()) + set_ops = [] + + for component_ref in component_refs: + assert isinstance(component_ref, irast.PointerRef), \ + "expected regular PointerRef" + + component_rvar = _range_for_component_ptrref( + component_ref, + output_cols, + dml_source=dml_source, + include_descendants=include_descendants, + for_mutation=for_mutation, + path_id=path_id, + ctx=ctx, + ) + + component_qry = pgast.SelectStmt( + target_list=[ + pgast.ResTarget( + val=pgast.ColumnRef( + name=[output_colname] + ), + name=output_colname + ) + for output_colname in output_cols + ], + from_clause=[component_rvar] + ) + if path_id: + target_ref = pgast.ColumnRef( + name=[component_rvar.alias.aliasname, output_cols[1]] + ) + pathctx.put_path_identity_var( + component_qry, path_id, var=target_ref + ) + pathctx.put_path_source_rvar( + component_qry, path_id, component_rvar ) - 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 + set_ops.append((context.OverlayOp.UNION, component_qry)) - else: - lrefs = {} - for ref in list(refs): - assert isinstance(ref, irast.PointerRef), \ - "expected regular PointerRef" - lrefs[ref] = [ref] - - def prep_filter(larg: pgast.SelectStmt, rarg: pgast.SelectStmt) -> None: - # Set up the proper join on the source field and clear the target list - # of the rhs of a filter overlay. - assert isinstance(larg.target_list[0].val, pgast.ColumnRef) - assert isinstance(rarg.target_list[0].val, pgast.ColumnRef) - rarg.where_clause = astutils.join_condition( - larg.target_list[0].val, rarg.target_list[0].val) - rarg.target_list.clear() + return range_from_queryset( + set_ops, + ptrref.shortname, + path_id=path_id, + ctx=ctx, + ) - set_ops = [] - for orig_ptrref, src_ptrrefs in lrefs.items(): - sub_set_ops = [] - - 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=True, - versioned=ctx.env.versioned_stdlib, +def _range_for_component_ptrref( + component_ptrref: irast.PointerRef, + output_cols: Sequence[str], + *, + dml_source: Sequence[irast.MutatingLikeStmt], + include_descendants: bool, + for_mutation: bool, + path_id: Optional[irast.PathId], + ctx: context.CompilerContextLevel, +) -> pgast.PathRangeVar: + ptrref_descendants = _get_ptrref_descendants( + component_ptrref, + include_descendants=include_descendants, + for_mutation=for_mutation, + ) + + if ( + # When we are compiling a query for EXPLAIN, expand out pointer + # references in place. See range_for_material_objtype for more details. + ctx.env.is_explain + + # Don't use CTEs if there is no inheritance. (ie. There is only a + # single ptrref) + or len(ptrref_descendants) <= 1 + ): + descendant_selects = _selects_for_ptrref_descendants( + ptrref_descendants, + output_cols=output_cols, + path_id=path_id, + ctx=ctx, + ) + descendant_ops = [ + (context.OverlayOp.UNION, select) + for select in descendant_selects + ] + component_rvar = range_from_queryset( + descendant_ops, + component_ptrref.shortname, + path_id=path_id, + ctx=ctx, + ) + + else: + component_ptrref_path_id: irast.PathId = irast.PathId.from_ptrref( + component_ptrref, + ).ptr_path() + + if component_ptrref.id not in ctx.ptr_inheritance_ctes: + descendant_selects = _selects_for_ptrref_descendants( + ptrref_descendants, + output_cols=output_cols, + path_id=component_ptrref_path_id, + ctx=ctx, ) - if not ptr_info: - assert ptrref.union_components - ptr_info = pg_types.get_ptrref_storage_info( - src_ptrref, resolve_type=False, link_bias=False, - versioned=ctx.env.versioned_stdlib, + inheritance_qry: pgast.SelectStmt = descendant_selects[0] + for rarg in descendant_selects[1:]: + inheritance_qry = pgast.SelectStmt( + op='union', + all=True, + larg=inheritance_qry, + rarg=rarg, ) - cols = [ - 'source' if ptr_info.table_type == 'link' else 'id', - ptr_info.column_name, - ] + # Add the path to the CTE's query. This allows for the proper + # path mapping to occur when processing link properties + inheritance_qry.path_id = component_ptrref_path_id - table = table_from_ptrref( - src_ptrref, - ptr_info, - include_descendants=include_descendants, - for_mutation=for_mutation, - ctx=ctx, + ptr_cte = pgast.CommonTableExpr( + name=ctx.env.aliases.get(f't_{component_ptrref.name}'), + query=inheritance_qry, + materialized=False, ) - table.query.path_id = path_id + ctx.ptr_inheritance_ctes[component_ptrref.id] = ptr_cte - qry = pgast.SelectStmt() - qry.from_clause.append(table) + else: + ptr_cte = ctx.ptr_inheritance_ctes[component_ptrref.id] + + with ctx.subrel() as sctx: + cte_rvar = rvar_for_rel( + ptr_cte, + typeref=component_ptrref.out_target, + ctx=ctx, + ) + if path_id is not None and path_id != component_ptrref_path_id: + pathctx.put_path_id_map( + sctx.rel, + path_id, + component_ptrref_path_id, + ) + include_rvar( + sctx.rel, + cte_rvar, + component_ptrref_path_id, + pull_namespace=False, + ctx=sctx, + ) - # Make sure all property references are pulled up properly - for colname, output_colname in zip(cols, output_cols): + # Ensure source and target columns are output + for output_colname in output_cols: selexpr = pgast.ColumnRef( - name=[table.alias.aliasname, colname]) - qry.target_list.append( - pgast.ResTarget(val=selexpr, name=output_colname)) + name=[cte_rvar.alias.aliasname, output_colname]) + sctx.rel.target_list.append( + pgast.ResTarget(val=selexpr, name=output_colname) + ) - sub_set_ops.append((context.OverlayOp.UNION, qry)) + target_ref = sctx.rel.target_list[1].val + pathctx.put_path_identity_var( + sctx.rel, component_ptrref_path_id, var=target_ref + ) + pathctx.put_path_source_rvar( + sctx.rel, + component_ptrref_path_id, + cte_rvar, + ) - # 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) - - sub_rvar = range_from_queryset( - sub_set_ops, ptrref.shortname, - prep_filter=prep_filter, path_id=path_id, ctx=ctx) - sub_qry = pgast.SelectStmt( + component_rvar = rvar_for_rel( + sctx.rel, + typeref=component_ptrref.out_target, + ctx=sctx + ) + + # Add overlays at the end of each expanded inheritance. + overlays = get_ptr_rel_overlays( + component_ptrref, dml_source=dml_source, ctx=ctx) + if overlays and not for_mutation: + set_ops = [] + + component_qry = pgast.SelectStmt( target_list=[ pgast.ResTarget( val=pgast.ColumnRef( - name=[colname] + name=[output_colname] ), name=output_colname ) - for colname, output_colname in zip(cols, output_cols) + for output_colname in output_cols ], - from_clause=[sub_rvar] + from_clause=[component_rvar] ) if path_id: target_ref = pgast.ColumnRef( - name=[sub_rvar.alias.aliasname, output_cols[1]] + name=[component_rvar.alias.aliasname, output_cols[1]] + ) + pathctx.put_path_identity_var( + component_qry, path_id, var=target_ref + ) + pathctx.put_path_source_rvar( + component_qry, path_id, component_rvar ) - pathctx.put_path_identity_var(sub_qry, path_id, var=target_ref) - pathctx.put_path_source_rvar(sub_qry, path_id, sub_rvar) - set_ops.append((context.OverlayOp.UNION, sub_qry)) + set_ops.append((context.OverlayOp.UNION, component_qry)) - # 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 orig_ptrref in refs and not for_mutation: - overlays = get_ptr_rel_overlays( - orig_ptrref, dml_source=dml_source, ctx=ctx) + orig_ptr_info = _get_ptrref_storage_info(component_ptrref, ctx=ctx) + cols = _get_ptrref_column_names(orig_ptr_info) - for op, cte, cte_path_id in overlays: - rvar = rvar_for_rel(cte, ctx=ctx) + for op, cte, cte_path_id in overlays: + rvar = rvar_for_rel(cte, ctx=ctx) - qry = pgast.SelectStmt( - target_list=[ - pgast.ResTarget( - val=pgast.ColumnRef( - name=[col] - ) + qry = pgast.SelectStmt( + target_list=[ + pgast.ResTarget( + val=pgast.ColumnRef( + name=[col] ) - for col in cols - ], - from_clause=[rvar], - ) - # Set up identity var, source rvar for reasons discussed above - if path_id: - target_ref = pgast.ColumnRef( - name=[rvar.alias.aliasname, cols[1]]) - pathctx.put_path_identity_var( - qry, cte_path_id, var=target_ref ) - pathctx.put_path_source_rvar(qry, cte_path_id, rvar) - pathctx.put_path_id_map(qry, path_id, cte_path_id) + for col in cols + ], + from_clause=[rvar], + ) + # Set up identity var, source rvar for reasons discussed above + if path_id: + target_ref = pgast.ColumnRef( + name=[rvar.alias.aliasname, cols[1]]) + pathctx.put_path_identity_var( + qry, cte_path_id, var=target_ref + ) + pathctx.put_path_source_rvar(qry, cte_path_id, rvar) + pathctx.put_path_id_map(qry, path_id, cte_path_id) - set_ops.append((op, qry)) + set_ops.append((op, qry)) - return range_from_queryset( - set_ops, ptrref.shortname, - prep_filter=prep_filter, path_id=path_id, ctx=ctx) + component_rvar = range_from_queryset( + set_ops, component_ptrref.shortname, + prep_filter=_prep_filter, path_id=path_id, ctx=ctx) + + return component_rvar + + +def _prep_filter(larg: pgast.SelectStmt, rarg: pgast.SelectStmt) -> None: + # Set up the proper join on the source field and clear the target list + # of the rhs of a filter overlay. + assert isinstance(larg.target_list[0].val, pgast.ColumnRef) + assert isinstance(rarg.target_list[0].val, pgast.ColumnRef) + rarg.where_clause = astutils.join_condition( + larg.target_list[0].val, rarg.target_list[0].val) + rarg.target_list.clear() + + +def _get_ptrref_descendants( + ptrref: irast.PointerRef, + *, + include_descendants: bool, + for_mutation: bool, +) -> list[irast.PointerRef]: + # When doing EXPLAIN, don't use CTEs. See range_for_material_objtype for + # details. + if ( + include_descendants + and not for_mutation + ): + include_descendants = False + + descendants: list[irast.PointerRef] = [] + descendants.extend( + cast(Iterable[irast.PointerRef], ptrref.descendants()) + ) + descendants.append(ptrref) + assert isinstance(ptrref, irast.PointerRef) + + # Try to only select from actual concrete types. + concrete_descendants = [ + ref for ref in descendants if not ref.out_source.is_abstract + ] + # If there aren't any concrete types, we still need to + # generate *something*, so just do the initial one. + if concrete_descendants: + return concrete_descendants + else: + return [ptrref] + + else: + return [ptrref] + + +def _selects_for_ptrref_descendants( + ptrref_descendants: Sequence[irast.PointerRef], + output_cols: Iterable[str], + *, + path_id: Optional[irast.PathId], + ctx: context.CompilerContextLevel, +) -> list[pgast.SelectStmt]: + selects = [] + + for ptrref_descendant in ptrref_descendants: + ptr_info = _get_ptrref_storage_info(ptrref_descendant, ctx=ctx) + cols = _get_ptrref_column_names(ptr_info) + + table = _table_from_ptrref( + ptrref_descendant, + ptr_info, + ctx=ctx, + ) + table.query.path_id = path_id + + qry = pgast.SelectStmt() + qry.from_clause.append(table) + + # 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)) + + selects.append(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) + + return selects + + +def _table_from_ptrref( + ptrref: irast.PointerRef, + ptr_info: pg_types.PointerStorageInfo, + *, + ctx: context.CompilerContextLevel, +) -> pgast.RelRangeVar: + """Return a Table corresponding to a given Link.""" + + aspect = 'table' + table_schema_name, table_name = common.update_aspect( + ptr_info.table_name, aspect + ) + + typeref = ptrref.out_source if ptrref else None + relation = pgast.Relation( + schemaname=table_schema_name, + name=table_name, + type_or_ptr_ref=ptrref, + ) + + # Pseudo pointers (tuple and type intersection) have no schema id. + sobj_id = ptrref.id if isinstance(ptrref, irast.PointerRef) else None + rvar = pgast.RelRangeVar( + schema_object_id=sobj_id, + typeref=typeref, + relation=relation, + alias=pgast.Alias( + aliasname=ctx.env.aliases.get(ptrref.shortname.name) + ) + ) + + return rvar + + +def _get_ptrref_storage_info( + ptrref: irast.PointerRef, + *, + ctx: context.CompilerContextLevel +) -> pg_types.PointerStorageInfo: + # 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( + ptrref, resolve_type=False, link_bias=True, + versioned=ctx.env.versioned_stdlib, + ) + if not ptr_info: + ptr_info = pg_types.get_ptrref_storage_info( + ptrref, resolve_type=False, link_bias=False, + versioned=ctx.env.versioned_stdlib, + ) + return ptr_info + + +def _get_ptrref_column_names( + ptr_info: pg_types.PointerStorageInfo +) -> list[str]: + return [ + 'source' if ptr_info.table_type == 'link' else 'id', + ptr_info.column_name, + ] def range_for_pointer( diff --git a/edb/pgsql/compiler/relgen.py b/edb/pgsql/compiler/relgen.py index c6ab4026f7f..0bbd8442aac 100644 --- a/edb/pgsql/compiler/relgen.py +++ b/edb/pgsql/compiler/relgen.py @@ -243,7 +243,7 @@ def get_set_rvar( rvars = _get_expr_set_rvar(ir_set.expr, ir_set, ctx=subctx) relctx.update_scope_masks(ir_set, rvars.main.rvar, ctx=subctx) - if ctx.env.expand_inhviews: + if ctx.env.is_explain: for srvar in rvars.new: if not srvar.rvar.ir_origins: srvar.rvar.ir_origins = [] @@ -897,7 +897,7 @@ def process_set_as_link_property_ref( {spec.id for spec in rptr_specialization} if rptr_specialization is not None else None ) - if ctx.env.expand_inhviews and ptr_ids and rptr_specialization: + if ptr_ids and rptr_specialization: ptr_ids.update( x.id for spec in rptr_specialization for x in spec.descendants() @@ -927,6 +927,21 @@ def process_set_as_link_property_ref( ), ), ) + elif isinstance(link_rvar.query, pgast.SelectStmt): + # When processing link properties into a CTE, map the current link + # path id into the form used by the CTE. + + for from_rvar in link_rvar.query.from_clause: + if ( + isinstance(from_rvar, pgast.RelRangeVar) + and isinstance(from_rvar.relation, pgast.CommonTableExpr) + and from_rvar.relation.query.path_id is not None + ): + pathctx.put_path_id_map( + link_rvar.query, + link_path_id, + from_rvar.relation.query.path_id + ) rvars.append(SetRVar( link_rvar, diff --git a/edb/pgsql/delta.py b/edb/pgsql/delta.py index 038b46fff7d..86d1c466d92 100644 --- a/edb/pgsql/delta.py +++ b/edb/pgsql/delta.py @@ -241,7 +241,7 @@ def schedule_inhview_source_update( assert isinstance(ctx.op, CompositeMetaCommand) src = ptr.get_source(schema) - if src and types.is_cfg_view(src, schema): + if src and irtyputils.is_cfg_view(src, schema): assert isinstance(src, s_sources.Source) self.pgops.add( CompositeMetaCommand._refresh_fake_cfg_view_cmd( @@ -1169,6 +1169,19 @@ def _compile_edgeql_function( ) -> s_expr.CompiledExpression: if isinstance(body, s_expr.CompiledExpression): return body + + # HACK: When an object type selected by a function (via + # inheritance) is dropped, the function gets + # recompiled. Unfortunately, 'caused' subcommands run *before* + # the object is actually deleted, and so we would ordinarily + # still try to select from the deleted object. To avoid + # needing to add *another* type of subcommand, we work around + # this by temporarily stripping all objects that are about to + # be deleted from the schema. + for ctx in context.stack: + if isinstance(ctx.op, s_objtypes.DeleteObjectType): + schema = schema.delete(ctx.op.scls) + return s_funcs.compile_function( schema, context, @@ -1987,7 +2000,7 @@ def constraint_is_effective( ): return False - if types.is_cfg_view(subject, schema): + if irtyputils.is_cfg_view(subject, schema): return False match subject: @@ -3076,7 +3089,7 @@ def attach_alter_table(self, context): @staticmethod def _get_table_name(obj, schema) -> tuple[str, str]: - is_internal_view = types.is_cfg_view(obj, schema) + is_internal_view = irtyputils.is_cfg_view(obj, schema) aspect = 'dummy' if is_internal_view else None return common.get_backend_name( schema, obj, catenate=False, aspect=aspect) @@ -3201,7 +3214,7 @@ def _get_select_from( cols = [] special_cols = ['tableoid', 'xmin', 'cmin', 'xmax', 'cmax', 'ctid'] - if not types.is_cfg_view(obj, schema): + if not irtyputils.is_cfg_view(obj, schema): cols.extend([(col, col, True) for col in special_cols]) else: cols.extend([('NULL', col, False) for col in special_cols]) @@ -3329,8 +3342,8 @@ def get_inhview( # excruciatingly slow because of the cost of explicit id # checks. See #5168. and ( - not types.is_cfg_view(child, schema) - or types.is_cfg_view(obj, schema) + not irtyputils.is_cfg_view(child, schema) + or irtyputils.is_cfg_view(obj, schema) ) ] @@ -3368,7 +3381,7 @@ def update_base_inhviews_on_rebase( context: sd.CommandContext, obj: CompositeObject, ) -> None: - if types.is_cfg_view(obj, schema): + if irtyputils.is_cfg_view(obj, schema): self._refresh_fake_cfg_view(obj, schema, context) bases = set(obj.get_bases(schema).objects(schema)) @@ -3447,7 +3460,7 @@ def create_inhview( ) -> None: assert types.has_table(obj, schema) - if types.is_cfg_view(obj, schema): + if irtyputils.is_cfg_view(obj, schema): self._refresh_fake_cfg_view(obj, schema, context) inhview = self.get_inhview(schema, obj, exclude_ptrs=exclude_ptrs) @@ -3477,7 +3490,7 @@ def alter_inhview( ) -> None: assert types.has_table(obj, schema) - if types.is_cfg_view(obj, schema): + if irtyputils.is_cfg_view(obj, schema): self._refresh_fake_cfg_view(obj, schema, context) inhview = self.get_inhview( @@ -3931,8 +3944,8 @@ def _fixup_configs( # configs, since those need to be created after the standard # schema is in place. if not ( - types.is_cfg_view(scls, eff_schema) - and scls.get_name(eff_schema).module not in types.VIEW_MODULES + irtyputils.is_cfg_view(scls, eff_schema) + and scls.get_name(eff_schema).module not in irtyputils.VIEW_MODULES ): return @@ -4307,7 +4320,7 @@ def _alter_pointer_cardinality( source_rel_alias = f'source_{uuidgen.uuid1mc()}' if self.conv_expr is not None: - (conv_expr_ctes, _) = self._compile_conversion_expr( + (conv_expr_ctes, _, _) = self._compile_conversion_expr( ptr, self.conv_expr, source_rel_alias, @@ -4527,7 +4540,7 @@ def _alter_pointer_optionality( source_rel_alias = f'source_{uuidgen.uuid1mc()}' - (conv_expr_ctes, _) = self._compile_conversion_expr( + (conv_expr_ctes, _, _) = self._compile_conversion_expr( ptr, fill_expr, source_rel_alias, @@ -4681,16 +4694,19 @@ def _alter_pointer_type(self, pointer, schema, orig_schema, context): # supports arbitrary queries, but requires a temporary column, # which is populated with the transition query and then used as the # source for the SQL USING clause. - (cast_expr_sql, expr_is_nullable) = self._compile_conversion_expr( - pointer, - cast_expr, - source_rel_alias, - schema=schema, - orig_schema=orig_schema, - context=context, - check_non_null=is_required and not is_multi, - produce_ctes=False, + (cast_expr_ctes, cast_expr_sql, expr_is_nullable) = ( + self._compile_conversion_expr( + pointer, + cast_expr, + source_rel_alias, + schema=schema, + orig_schema=orig_schema, + context=context, + check_non_null=is_required and not is_multi, + produce_ctes=False, + ) ) + assert cast_expr_sql is not None need_temp_col = ( (is_multi and expr_is_nullable) or changing_col_type ) @@ -4724,7 +4740,9 @@ def _alter_pointer_type(self, pointer, schema, orig_schema, context): self.pgops.add(alter_table) target_col = temp_column.name + update_with = f'WITH {cast_expr_ctes}' if cast_expr_ctes else '' update_qry = f''' + {update_with} UPDATE {tab} AS {qi(source_rel_alias)} SET {qi(target_col)} = ({cast_expr_sql}) ''' @@ -4844,7 +4862,8 @@ def _compile_conversion_expr( produce_ctes: bool = True, allow_globals: bool=False, ) -> Tuple[ - str, # SQL + str, # CTE SQL + Optional[str], # Query SQL bool, # is_nullable ]: """ @@ -5143,17 +5162,14 @@ def _compile_conversion_expr( # compile to SQL ctes_sql = codegen.generate_ctes_source(ctes) - return (ctes_sql, nullable) + return (ctes_sql, None, nullable) else: - # There should be no CTEs when prodoce_ctes==False, since this will - # will happen only when changing type (cast_expr), which cannot - # contain DML. - assert len(ctes) == 0 - + # keep CTEs and select separate + ctes_sql = codegen.generate_ctes_source(ctes) select_sql = codegen.generate_source(sql_tree) - return (select_sql, nullable) + return (ctes_sql, select_sql, nullable) def schedule_endpoint_delete_action_update( self, link, orig_schema, schema, context @@ -5821,7 +5837,7 @@ def _create_property( (default := prop.get_default(schema)) and not prop.is_pure_computable(schema) and not fills_required - and not types.is_cfg_view(src.scls, schema) # sigh + and not irtyputils.is_cfg_view(src.scls, schema) # sigh # link properties use SQL defaults and shouldn't need # us to do it explicitly (which is good, since # _alter_pointer_optionality doesn't currently work on @@ -6225,7 +6241,10 @@ def get_target_objs(self, link, schema): x for obj in objs for x in obj.descendants(schema)} return { obj for obj in objs - if not obj.is_view(schema) and not types.is_cfg_view(obj, schema) + if ( + not obj.is_view(schema) + and not irtyputils.is_cfg_view(obj, schema) + ) } def get_orphan_link_ancestors(self, link, schema): @@ -6782,7 +6801,7 @@ def apply( if ( not isinstance(source, s_objtypes.ObjectType) - or types.is_cfg_view(source, eff_schema) + or irtyputils.is_cfg_view(source, eff_schema) ): continue @@ -6831,7 +6850,7 @@ def apply( delete_target_targets = set() for target in all_affected_targets: - if types.is_cfg_view(target, schema): + if irtyputils.is_cfg_view(target, schema): continue deferred_links = [] @@ -6860,7 +6879,7 @@ def apply( source = link.get_source(schema) if ( not source.is_material_object_type(schema) - or types.is_cfg_view(source, schema) + or irtyputils.is_cfg_view(source, schema) ): continue diff --git a/edb/pgsql/inheritance.py b/edb/pgsql/inheritance.py index e1a1694d26e..c4b88682ca0 100644 --- a/edb/pgsql/inheritance.py +++ b/edb/pgsql/inheritance.py @@ -7,6 +7,8 @@ from edb.schema import sources as s_sources from edb.schema import schema as s_schema +from edb.ir import typeutils as irtyputils + from . import ast as pgast from . import types from . import common @@ -69,8 +71,8 @@ def get_inheritance_view( # excruciatingly slow because of the cost of explicit id # checks. See #5168. and ( - not types.is_cfg_view(child, schema) - or types.is_cfg_view(obj, schema) + not irtyputils.is_cfg_view(child, schema) + or irtyputils.is_cfg_view(obj, schema) ) ] @@ -125,7 +127,7 @@ def _get_select_from( system_cols = ['tableoid', 'xmin', 'cmin', 'xmax', 'cmax', 'ctid'] for sys_col_name in system_cols: val: pgast.BaseExpr - if not types.is_cfg_view(obj, schema): + if not irtyputils.is_cfg_view(obj, schema): val = pgast.ColumnRef(name=(table_rvar_name, sys_col_name)) else: val = pgast.NullConstant() diff --git a/edb/pgsql/types.py b/edb/pgsql/types.py index 7120689adc2..0bc7c9d00f4 100644 --- a/edb/pgsql/types.py +++ b/edb/pgsql/types.py @@ -768,36 +768,6 @@ def _ptrref_storable_in_pointer(ptrref: irast.BasePointerRef) -> bool: ) -# Modules where all the "types" in them are really just custom views -# provided by metaschema. -VIEW_MODULES = ('sys', 'cfg') - - -def is_cfg_view( - obj: s_obj.Object, - schema: s_schema.Schema, -) -> bool: - return ( - isinstance(obj, (s_objtypes.ObjectType, s_pointers.Pointer)) - and ( - obj.get_name(schema).module in VIEW_MODULES - or bool( - (cfg_object := schema.get( - 'cfg::ConfigObject', - type=s_objtypes.ObjectType, default=None - )) - and ( - nobj := ( - obj if isinstance(obj, s_objtypes.ObjectType) - else obj.get_source(schema) - ) - ) - and nobj.issubclass(schema, cfg_object) - ) - ) - ) - - def has_table( obj: Optional[s_obj.InheritingObject], schema: s_schema.Schema ) -> bool: diff --git a/edb/schema/delta.py b/edb/schema/delta.py index aa345080334..69f5e711ce7 100644 --- a/edb/schema/delta.py +++ b/edb/schema/delta.py @@ -2088,6 +2088,7 @@ def _propagate_if_expr_refs( context: CommandContext, *, action: str, + include_self: bool=True, include_ancestors: bool=False, extra_refs: Optional[Dict[so.Object, List[str]]]=None, filter: Type[so.Object] | Tuple[Type[so.Object], ...] | None = None, @@ -2104,7 +2105,9 @@ def _propagate_if_expr_refs( fixer = None scls = self.scls - expr_refs = s_expr.get_expr_referrers(schema, scls) + expr_refs: dict[so.Object, list[str]] = {} + if include_self: + expr_refs.update(s_expr.get_expr_referrers(schema, scls)) if include_ancestors and isinstance(scls, so.InheritingObject): for anc in scls.get_ancestors(schema).objects(schema): expr_refs.update(s_expr.get_expr_referrers(schema, anc)) @@ -3203,6 +3206,8 @@ def _create_finalize( context: CommandContext, ) -> s_schema.Schema: if not context.canonical: + # This is rarely triggered. + schema = self._finalize_affected_refs(schema, context) self.validate_object(schema, context) return schema diff --git a/edb/schema/objtypes.py b/edb/schema/objtypes.py index 0e7ea829ee3..0d567e3589e 100644 --- a/edb/schema/objtypes.py +++ b/edb/schema/objtypes.py @@ -32,6 +32,7 @@ from . import annos as s_anno from . import constraints from . import delta as sd +from . import functions as s_func from . import inheriting from . import links from . import properties @@ -560,6 +561,28 @@ def _get_ast_node( else: return super()._get_ast_node(schema, context) + def _create_finalize( + self, + schema: s_schema.Schema, + context: sd.CommandContext, + ) -> s_schema.Schema: + if ( + not context.canonical + and self.scls.is_material_object_type(schema) + ): + # Propagate changes to any functions that depend on + # ancestor types in order to recompute the inheritance + # situation. + schema = self._propagate_if_expr_refs( + schema, + context, + action='creating an object type', + include_ancestors=True, + filter=s_func.Function, + ) + + return super()._create_finalize(schema, context) + class RenameObjectType( ObjectTypeCommand, @@ -701,3 +724,26 @@ def _get_ast( return None else: return super()._get_ast(schema, context, parent_node=parent_node) + + def _delete_finalize( + self, + schema: s_schema.Schema, + context: sd.CommandContext, + ) -> s_schema.Schema: + if ( + not context.canonical + and self.scls.is_material_object_type(schema) + ): + # Propagate changes to any functions that depend on + # ancestor types in order to recompute the inheritance + # situation. + schema = self._propagate_if_expr_refs( + schema, + context, + action='deleting an object type', + include_self=False, + include_ancestors=True, + filter=s_func.Function, + ) + + return super()._delete_finalize(schema, context) diff --git a/edb/server/bootstrap.py b/edb/server/bootstrap.py index c57fbc01ba6..53059f5fac4 100644 --- a/edb/server/bootstrap.py +++ b/edb/server/bootstrap.py @@ -51,6 +51,7 @@ from edb import edgeql from edb.ir import statypes +from edb.ir import typeutils as irtyputils from edb.edgeql import ast as qlast from edb.common import debug @@ -79,7 +80,6 @@ from edb.server import pgcon from edb.pgsql import common as pg_common -from edb.pgsql import types as pg_types from edb.pgsql import dbops from edb.pgsql import delta as delta_cmds from edb.pgsql import metaschema @@ -1135,7 +1135,7 @@ async def create_branch( multi_properties = { prop for prop in schema.get_objects(type=s_props.Property) if prop.get_cardinality(schema).is_multi() - and prop.get_name(schema).module not in pg_types.VIEW_MODULES + and prop.get_name(schema).module not in irtyputils.VIEW_MODULES } for mprop in multi_properties: diff --git a/edb/server/compiler/compiler.py b/edb/server/compiler/compiler.py index 7f9ea58a135..d717441ccf5 100644 --- a/edb/server/compiler/compiler.py +++ b/edb/server/compiler/compiler.py @@ -1700,11 +1700,7 @@ def _get_compile_options( ctx, 'apply_access_policies'), allow_user_specified_id=_get_config_val( ctx, 'allow_user_specified_id') or ctx.schema_reflection_mode, - expand_inhviews=( - debug.flags.edgeql_expand_inhviews - and not ctx.bootstrap_mode - and not ctx.schema_reflection_mode - ) or is_explain, + is_explain=is_explain, testmode=_get_config_val(ctx, '__internal_testmode'), schema_reflection_mode=( ctx.schema_reflection_mode @@ -1889,7 +1885,7 @@ def _compile_ql_query( expected_cardinality_one=ctx.expected_cardinality_one, output_format=_convert_format(ctx.output_format), backend_runtime_params=ctx.backend_runtime_params, - expand_inhviews=options.expand_inhviews, + is_explain=options.is_explain, detach_params=(use_persistent_cache and cache_mode is config.QueryCacheMode.PgFunc), versioned_stdlib=True, diff --git a/tests/test_edgeql_ddl.py b/tests/test_edgeql_ddl.py index 868c89c81b5..d122fc303df 100644 --- a/tests/test_edgeql_ddl.py +++ b/tests/test_edgeql_ddl.py @@ -5139,6 +5139,78 @@ async def test_edgeql_ddl_function_38(self): ); ''') + async def test_edgeql_ddl_function_inh_01(self): + await self.con.execute(""" + create abstract type T; + create function countall() -> int64 USING (count(T)); + """) + + await self.assert_query_result( + """SELECT countall()""", + [0], + ) + await self.con.execute(""" + create type S1 extending T; + insert S1; + """) + await self.assert_query_result( + """SELECT countall()""", + [1], + ) + await self.con.execute(""" + create type S2 extending T; + insert S2; + insert S2; + """) + await self.assert_query_result( + """SELECT countall()""", + [3], + ) + await self.con.execute(""" + drop type S2; + """) + + await self.assert_query_result( + """SELECT countall()""", + [1], + ) + + async def test_edgeql_ddl_function_inh_02(self): + await self.con.execute(""" + create abstract type T { create multi property n -> int64 }; + create function countall() -> int64 USING (sum(T.n)); + """) + + await self.assert_query_result( + """SELECT countall()""", + [0], + ) + await self.con.execute(""" + create type S1 extending T; + insert S1 { n := {3, 4} }; + """) + await self.assert_query_result( + """SELECT countall()""", + [7], + ) + await self.con.execute(""" + create type S2 extending T; + insert S2 { n := 1 }; + insert S2 { n := {2, 2, 2} }; + """) + await self.assert_query_result( + """SELECT countall()""", + [14], + ) + await self.con.execute(""" + drop type S2; + """) + + await self.assert_query_result( + """SELECT countall()""", + [7], + ) + async def test_edgeql_ddl_function_rename_01(self): await self.con.execute(""" CREATE FUNCTION foo(s: str) -> str {