From 146ac416176640b5d034a00d34f4d8968764cb92 Mon Sep 17 00:00:00 2001 From: dnwpark Date: Thu, 6 Jun 2024 17:51:54 -0400 Subject: [PATCH 01/26] Create CTE when creating range for a material type. --- edb/edgeql/compiler/typegen.py | 3 +- edb/pgsql/compiler/clauses.py | 1 + edb/pgsql/compiler/context.py | 5 ++ edb/pgsql/compiler/relctx.py | 93 ++++++++++++++++++++++++---------- 4 files changed, 72 insertions(+), 30 deletions(-) diff --git a/edb/edgeql/compiler/typegen.py b/edb/edgeql/compiler/typegen.py index d8b5e78e26a..9e6d44a768b 100644 --- a/edb/edgeql/compiler/typegen.py +++ b/edb/edgeql/compiler/typegen.py @@ -402,8 +402,7 @@ def type_to_typeref( 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) + isinstance(t, s_objtypes.ObjectType) ) ) include_ancestors = ( diff --git a/edb/pgsql/compiler/clauses.py b/edb/pgsql/compiler/clauses.py index f921d38819e..9b14698bb7b 100644 --- a/edb/pgsql/compiler/clauses.py +++ b/edb/pgsql/compiler/clauses.py @@ -421,6 +421,7 @@ def fini_toplevel( *ctx.param_ctes.values(), *ctx.ptr_ctes.values(), *ctx.type_ctes.values(), + *ctx.type_inheritance_ctes.values(), ] if ctx.env.named_param_prefix is None: diff --git a/edb/pgsql/compiler/context.py b/edb/pgsql/compiler/context.py index d4ee0fd3c52..95c71ea0844 100644 --- a/edb/pgsql/compiler/context.py +++ b/edb/pgsql/compiler/context.py @@ -252,6 +252,9 @@ class CompilerContextLevel(compiler.ContextLevel): #: A set of type CTEs currently being generated pending_type_ctes: Set[RewriteKey] + #: CTEs representing types and their inherited types + type_inheritance_ctes: Dict[uuid.UUID, pgast.CommonTableExpr] + #: The logical parent of the current query in the #: query hierarchy parent_rel: Optional[pgast.Query] @@ -354,6 +357,7 @@ def __init__( self.ptr_ctes = {} self.type_ctes = {} self.pending_type_ctes = set() + self.type_inheritance_ctes = {} self.dml_stmts = {} self.parent_rel = None self.pending_query = None @@ -394,6 +398,7 @@ def __init__( self.ptr_ctes = prevlevel.ptr_ctes self.type_ctes = prevlevel.type_ctes self.pending_type_ctes = prevlevel.pending_type_ctes + self.type_inheritance_ctes = prevlevel.type_inheritance_ctes self.dml_stmts = prevlevel.dml_stmts self.parent_rel = prevlevel.parent_rel self.pending_query = prevlevel.pending_query diff --git a/edb/pgsql/compiler/relctx.py b/edb/pgsql/compiler/relctx.py index 29e0af2cf20..92bf1d20e4b 100644 --- a/edb/pgsql/compiler/relctx.py +++ b/edb/pgsql/compiler/relctx.py @@ -1646,43 +1646,80 @@ def range_for_material_objtype( # 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 + include_descendants and not for_mutation and typeref.children is not None # 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? - ctx=ctx, + 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. Date: Tue, 11 Jun 2024 15:06:24 -0400 Subject: [PATCH 02/26] Order type CTEs in sql statement. --- edb/pgsql/compiler/clauses.py | 3 +-- edb/pgsql/compiler/context.py | 6 ++++++ edb/pgsql/compiler/dml.py | 4 +++- edb/pgsql/compiler/relctx.py | 2 ++ 4 files changed, 12 insertions(+), 3 deletions(-) diff --git a/edb/pgsql/compiler/clauses.py b/edb/pgsql/compiler/clauses.py index 9b14698bb7b..227debab59b 100644 --- a/edb/pgsql/compiler/clauses.py +++ b/edb/pgsql/compiler/clauses.py @@ -420,8 +420,7 @@ def fini_toplevel( stmt.ctes[:0] = [ *ctx.param_ctes.values(), *ctx.ptr_ctes.values(), - *ctx.type_ctes.values(), - *ctx.type_inheritance_ctes.values(), + *ctx.ordered_type_ctes, ] if ctx.env.named_param_prefix is None: diff --git a/edb/pgsql/compiler/context.py b/edb/pgsql/compiler/context.py index 95c71ea0844..52ef78c7da1 100644 --- a/edb/pgsql/compiler/context.py +++ b/edb/pgsql/compiler/context.py @@ -255,6 +255,10 @@ class CompilerContextLevel(compiler.ContextLevel): #: 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 parent_rel: Optional[pgast.Query] @@ -358,6 +362,7 @@ def __init__( self.type_ctes = {} self.pending_type_ctes = set() self.type_inheritance_ctes = {} + self.ordered_type_ctes = [] self.dml_stmts = {} self.parent_rel = None self.pending_query = None @@ -399,6 +404,7 @@ def __init__( self.type_ctes = prevlevel.type_ctes self.pending_type_ctes = prevlevel.pending_type_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 diff --git a/edb/pgsql/compiler/dml.py b/edb/pgsql/compiler/dml.py index 6f5d7a1311a..77c68d6a58b 100644 --- a/edb/pgsql/compiler/dml.py +++ b/edb/pgsql/compiler/dml.py @@ -3140,6 +3140,8 @@ def compile_triggers( # available for pointers off of __old__ to use. ictx.trigger_mode = True ictx.type_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 92bf1d20e4b..bd05a622744 100644 --- a/edb/pgsql/compiler/relctx.py +++ b/edb/pgsql/compiler/relctx.py @@ -1621,6 +1621,7 @@ def range_for_material_objtype( materialized=is_global or force_cte, ) ctx.type_ctes[key] = type_cte + ctx.ordered_type_ctes.append(type_cte) type_rel = type_cte else: type_rel = type_cte @@ -1698,6 +1699,7 @@ def range_for_material_objtype( materialized=False, ) ctx.type_inheritance_ctes[typeref.id] = type_cte + ctx.ordered_type_ctes.append(type_cte) else: type_cte = ctx.type_inheritance_ctes[typeref.id] From 578f3f36197a5da82f0c45b73c6a79f07a7323e4 Mon Sep 17 00:00:00 2001 From: dnwpark Date: Tue, 11 Jun 2024 17:43:24 -0400 Subject: [PATCH 03/26] Prevent using type CTE when explaining. --- edb/edgeql/compiler/options.py | 5 + edb/edgeql/compiler/typegen.py | 6 +- edb/pgsql/compiler/__init__.py | 2 + edb/pgsql/compiler/context.py | 2 + edb/pgsql/compiler/relctx.py | 178 ++++++++++++++++++++------------ edb/server/compiler/compiler.py | 6 ++ 6 files changed, 133 insertions(+), 66 deletions(-) diff --git a/edb/edgeql/compiler/options.py b/edb/edgeql/compiler/options.py index 1b4d698fe6f..387fdc527ad 100644 --- a/edb/edgeql/compiler/options.py +++ b/edb/edgeql/compiler/options.py @@ -82,6 +82,11 @@ class GlobalCompilerOptions: #: the query plan. expand_inhviews: bool = False + #: Should type inheritance be expanded using CTEs. + #: When not explaining CTEs can be used to provide access to a type and its + #: descendents. + use_type_inheritance_ctes: 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. in_ddl_context_name: Optional[str] = None diff --git a/edb/edgeql/compiler/typegen.py b/edb/edgeql/compiler/typegen.py index 9e6d44a768b..6920d132861 100644 --- a/edb/edgeql/compiler/typegen.py +++ b/edb/edgeql/compiler/typegen.py @@ -402,7 +402,11 @@ def type_to_typeref( expr_type is s_types.ExprType.Update or expr_type is s_types.ExprType.Delete or ( - isinstance(t, s_objtypes.ObjectType) + ( + env.options.expand_inhviews or + env.options.use_type_inheritance_ctes + ) + and isinstance(t, s_objtypes.ObjectType) ) ) include_ancestors = ( diff --git a/edb/pgsql/compiler/__init__.py b/edb/pgsql/compiler/__init__.py index 344d2096de6..e6d53d6d4b9 100644 --- a/edb/pgsql/compiler/__init__.py +++ b/edb/pgsql/compiler/__init__.py @@ -67,6 +67,7 @@ def compile_ir_to_sql_tree( named_param_prefix: Optional[tuple[str, ...]] = None, expected_cardinality_one: bool = False, expand_inhviews: bool = False, + use_type_inheritance_ctes: bool = False, external_rvars: Optional[ Mapping[Tuple[irast.PathId, pgce.PathAspect], pgast.PathRangeVar] ] = None, @@ -128,6 +129,7 @@ def compile_ir_to_sql_tree( ignore_object_shapes=ignore_shapes, explicit_top_cast=explicit_top_cast, expand_inhviews=expand_inhviews, + use_type_inheritance_ctes=use_type_inheritance_ctes, singleton_mode=singleton_mode, scope_tree_nodes=scope_tree_nodes, external_rvars=external_rvars, diff --git a/edb/pgsql/compiler/context.py b/edb/pgsql/compiler/context.py index 52ef78c7da1..addd2da57f4 100644 --- a/edb/pgsql/compiler/context.py +++ b/edb/pgsql/compiler/context.py @@ -548,6 +548,7 @@ def __init__( ignore_object_shapes: bool, singleton_mode: bool, expand_inhviews: bool, + use_type_inheritance_ctes: bool, explicit_top_cast: Optional[irast.TypeRef], query_params: List[irast.Param], type_rewrites: Dict[RewriteKey, irast.Set], @@ -567,6 +568,7 @@ def __init__( self.ignore_object_shapes = ignore_object_shapes self.singleton_mode = singleton_mode self.expand_inhviews = expand_inhviews + self.use_type_inheritance_ctes = use_type_inheritance_ctes self.explicit_top_cast = explicit_top_cast self.query_params = query_params self.type_rewrites = type_rewrites diff --git a/edb/pgsql/compiler/relctx.py b/edb/pgsql/compiler/relctx.py index bd05a622744..73510255335 100644 --- a/edb/pgsql/compiler/relctx.py +++ b/edb/pgsql/compiler/relctx.py @@ -1641,11 +1641,6 @@ 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 ( include_descendants and not for_mutation @@ -1654,74 +1649,96 @@ def range_for_material_objtype( # HACK: This is a workaround for #4491 and typeref.name_hint.module not in {'cfg', 'sys'} ): - 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[pgast.SelectStmt]: + selects = [] + 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? + 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 range_for_typeref( typeref: irast.TypeRef, path_id: irast.PathId, diff --git a/edb/server/compiler/compiler.py b/edb/server/compiler/compiler.py index 7f9ea58a135..78c7d5b5141 100644 --- a/edb/server/compiler/compiler.py +++ b/edb/server/compiler/compiler.py @@ -1705,6 +1705,11 @@ def _get_compile_options( and not ctx.bootstrap_mode and not ctx.schema_reflection_mode ) or is_explain, + use_type_inheritance_ctes=( + not is_explain + and not ctx.bootstrap_mode + and not ctx.schema_reflection_mode + ), testmode=_get_config_val(ctx, '__internal_testmode'), schema_reflection_mode=( ctx.schema_reflection_mode @@ -1890,6 +1895,7 @@ def _compile_ql_query( output_format=_convert_format(ctx.output_format), backend_runtime_params=ctx.backend_runtime_params, expand_inhviews=options.expand_inhviews, + use_type_inheritance_ctes=options.use_type_inheritance_ctes, detach_params=(use_persistent_cache and cache_mode is config.QueryCacheMode.PgFunc), versioned_stdlib=True, From 9c00d28cf7bb65fe0ba944a2cfda9a41152d77ad Mon Sep 17 00:00:00 2001 From: "Michael J. Sullivan" Date: Wed, 19 Jun 2024 15:44:17 -0700 Subject: [PATCH 04/26] Default to use_type_inheritance_cte Is there a reason to turn it off ever? --- edb/edgeql/compiler/options.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/edb/edgeql/compiler/options.py b/edb/edgeql/compiler/options.py index 387fdc527ad..e1c0c24907e 100644 --- a/edb/edgeql/compiler/options.py +++ b/edb/edgeql/compiler/options.py @@ -85,7 +85,7 @@ class GlobalCompilerOptions: #: Should type inheritance be expanded using CTEs. #: When not explaining CTEs can be used to provide access to a type and its #: descendents. - use_type_inheritance_ctes: bool = False + use_type_inheritance_ctes: bool = True #: The name that can be used in a "DML is disallowed in ..." #: error. When this is not None, any DML should cause an error. From 7a925f871432e7709e8cc33ae960cff4d31502fc Mon Sep 17 00:00:00 2001 From: "Michael J. Sullivan" Date: Wed, 19 Jun 2024 16:58:22 -0700 Subject: [PATCH 05/26] Propagate type creation/deletion to functions that depend on ancestors --- edb/pgsql/delta.py | 13 ++++++++ edb/schema/delta.py | 7 +++- edb/schema/objtypes.py | 46 +++++++++++++++++++++++++ tests/test_edgeql_ddl.py | 72 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 137 insertions(+), 1 deletion(-) diff --git a/edb/pgsql/delta.py b/edb/pgsql/delta.py index 038b46fff7d..59b3e909ba2 100644 --- a/edb/pgsql/delta.py +++ b/edb/pgsql/delta.py @@ -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, 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/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 { From d847bb6e30131a986626f23ee428e2ba84a5f70c Mon Sep 17 00:00:00 2001 From: dnwpark Date: Tue, 2 Jul 2024 11:33:43 -0400 Subject: [PATCH 06/26] Separate range_for_ptrref into helper functions. --- edb/pgsql/compiler/relctx.py | 245 ++++++++++++++++++++--------------- 1 file changed, 139 insertions(+), 106 deletions(-) diff --git a/edb/pgsql/compiler/relctx.py b/edb/pgsql/compiler/relctx.py index 73510255335..af49e199d93 100644 --- a/edb/pgsql/compiler/relctx.py +++ b/edb/pgsql/compiler/relctx.py @@ -2111,6 +2111,16 @@ def table_from_ptrref( return 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 range_for_ptrref( ptrref: irast.BasePointerRef, *, dml_source: Sequence[irast.MutatingLikeStmt]=(), @@ -2126,8 +2136,6 @@ 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: @@ -2144,118 +2152,37 @@ def range_for_ptrref( else: refs = {ptrref} - 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 + include_descendants = ( + not ptrref.union_is_exhaustive # 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()) - ) - 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] - - 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() + output_cols = ('source', 'target') 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, - ) - 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, - ) - - cols = [ - 'source' if ptr_info.table_type == 'link' else 'id', - ptr_info.column_name, - ] + for orig_ptrref in refs: + assert isinstance(orig_ptrref, irast.PointerRef), \ + "expected regular PointerRef" + src_ptrrefs, include_descendants = _expand_sub_ptrrefs( + orig_ptrref, + include_descendants=include_descendants, + for_mutation=for_mutation, + ctx=ctx, + ) - table = table_from_ptrref( - src_ptrref, - ptr_info, - include_descendants=include_descendants, - for_mutation=for_mutation, - 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)) - - sub_set_ops.append((context.OverlayOp.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) - - sub_rvar = range_from_queryset( - sub_set_ops, ptrref.shortname, - prep_filter=prep_filter, path_id=path_id, ctx=ctx) + sub_rvar, cols = _range_for_sub_ptrrefs( + src_ptrrefs, + name=ptrref.name, + output_cols=output_cols, + include_descendants=include_descendants, + for_mutation=for_mutation, + path_id=path_id, + ctx=ctx, + ) sub_qry = pgast.SelectStmt( target_list=[ pgast.ResTarget( @@ -2312,7 +2239,113 @@ def prep_filter(larg: pgast.SelectStmt, rarg: pgast.SelectStmt) -> None: return range_from_queryset( set_ops, ptrref.shortname, - prep_filter=prep_filter, path_id=path_id, ctx=ctx) + prep_filter=_prep_filter, path_id=path_id, ctx=ctx) + + +def _expand_sub_ptrrefs( + ref: irast.PointerRef, + *, + include_descendants: bool, + for_mutation: bool, + ctx: context.CompilerContextLevel, +) -> tuple[list[irast.PointerRef], bool]: + # expand_inhviews helps support EXPLAIN. see + # range_for_material_objtype for details. + if ( + ctx.env.expand_inhviews + and include_descendants + and not for_mutation + ): + include_descendants = False + + 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: + return concrete_lrefs, include_descendants + else: + return lref_entries, include_descendants + + else: + return [ref], include_descendants + + +def _range_for_sub_ptrrefs( + sub_ptrrefs: Sequence[irast.PointerRef], + name: sn.QualName, + output_cols: Iterable[str], + *, + include_descendants: bool, + for_mutation: bool, + path_id: Optional[irast.PathId], + ctx: context.CompilerContextLevel, +) -> tuple[pgast.PathRangeVar, list[str]]: + sub_set_ops = [] + + for src_ptrref in sub_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, + ) + if not ptr_info: + ptr_info = pg_types.get_ptrref_storage_info( + src_ptrref, resolve_type=False, link_bias=False, + versioned=ctx.env.versioned_stdlib, + ) + + cols = [ + 'source' if ptr_info.table_type == 'link' else 'id', + ptr_info.column_name, + ] + + table = table_from_ptrref( + src_ptrref, + ptr_info, + include_descendants=include_descendants, + for_mutation=for_mutation, + 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)) + + sub_set_ops.append((context.OverlayOp.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) + + sub_rvar = range_from_queryset( + sub_set_ops, name, + prep_filter=_prep_filter, path_id=path_id, ctx=ctx) + + return sub_rvar, cols def range_for_pointer( From b2e07721c369da8c0c4088a07a7c3338ed8ad58d Mon Sep 17 00:00:00 2001 From: dnwpark Date: Tue, 2 Jul 2024 15:00:09 -0400 Subject: [PATCH 07/26] Ignore expand inhview flag and always expand. --- edb/pgsql/compiler/relctx.py | 42 ++++++++++++------------------------ edb/pgsql/compiler/relgen.py | 2 +- 2 files changed, 15 insertions(+), 29 deletions(-) diff --git a/edb/pgsql/compiler/relctx.py b/edb/pgsql/compiler/relctx.py index af49e199d93..39ad1c4116a 100644 --- a/edb/pgsql/compiler/relctx.py +++ b/edb/pgsql/compiler/relctx.py @@ -2076,13 +2076,11 @@ 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' + aspect = 'table' table_schema_name, table_name = common.update_aspect( ptr_info.table_name, aspect ) @@ -2094,9 +2092,6 @@ def table_from_ptrref( 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( @@ -2167,19 +2162,16 @@ def range_for_ptrref( for orig_ptrref in refs: assert isinstance(orig_ptrref, irast.PointerRef), \ "expected regular PointerRef" - src_ptrrefs, include_descendants = _expand_sub_ptrrefs( + src_ptrrefs = _expand_sub_ptrrefs( orig_ptrref, include_descendants=include_descendants, for_mutation=for_mutation, - ctx=ctx, ) sub_rvar, cols = _range_for_sub_ptrrefs( src_ptrrefs, name=ptrref.name, output_cols=output_cols, - include_descendants=include_descendants, - for_mutation=for_mutation, path_id=path_id, ctx=ctx, ) @@ -2247,37 +2239,35 @@ def _expand_sub_ptrrefs( *, include_descendants: bool, for_mutation: bool, - ctx: context.CompilerContextLevel, -) -> tuple[list[irast.PointerRef], bool]: +) -> list[irast.PointerRef]: # expand_inhviews helps support EXPLAIN. see # range_for_material_objtype for details. if ( - ctx.env.expand_inhviews - and include_descendants + include_descendants and not for_mutation ): include_descendants = False - lref_entries: list[irast.PointerRef] = [] - lref_entries.extend( + descendants: list[irast.PointerRef] = [] + descendants.extend( cast(Iterable[irast.PointerRef], ref.descendants()) ) - lref_entries.append(ref) + descendants.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 + 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 all the abstract ones. - if concrete_lrefs: - return concrete_lrefs, include_descendants + # generate *something*, so just do the initial one. + if concrete_descendants: + return concrete_descendants else: - return lref_entries, include_descendants + return [ref] else: - return [ref], include_descendants + return [ref] def _range_for_sub_ptrrefs( @@ -2285,8 +2275,6 @@ def _range_for_sub_ptrrefs( name: sn.QualName, output_cols: Iterable[str], *, - include_descendants: bool, - for_mutation: bool, path_id: Optional[irast.PathId], ctx: context.CompilerContextLevel, ) -> tuple[pgast.PathRangeVar, list[str]]: @@ -2316,8 +2304,6 @@ def _range_for_sub_ptrrefs( table = table_from_ptrref( src_ptrref, ptr_info, - include_descendants=include_descendants, - for_mutation=for_mutation, ctx=ctx, ) table.query.path_id = path_id diff --git a/edb/pgsql/compiler/relgen.py b/edb/pgsql/compiler/relgen.py index c6ab4026f7f..89cd5b38023 100644 --- a/edb/pgsql/compiler/relgen.py +++ b/edb/pgsql/compiler/relgen.py @@ -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() From 52bf2d628983dfdf12f4d8ded4773b86b6e5a9dd Mon Sep 17 00:00:00 2001 From: dnwpark Date: Tue, 2 Jul 2024 17:28:48 -0400 Subject: [PATCH 08/26] Ensure column name is aliased correctly. --- edb/pgsql/compiler/relctx.py | 120 ++++++++++++++++++----------------- 1 file changed, 63 insertions(+), 57 deletions(-) diff --git a/edb/pgsql/compiler/relctx.py b/edb/pgsql/compiler/relctx.py index 39ad1c4116a..882b236e125 100644 --- a/edb/pgsql/compiler/relctx.py +++ b/edb/pgsql/compiler/relctx.py @@ -2032,6 +2032,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] @@ -2042,36 +2061,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, @@ -2168,9 +2157,9 @@ def range_for_ptrref( for_mutation=for_mutation, ) - sub_rvar, cols = _range_for_sub_ptrrefs( + sub_rvar = _range_for_sub_ptrrefs( src_ptrrefs, - name=ptrref.name, + name=orig_ptrref.shortname, output_cols=output_cols, path_id=path_id, ctx=ctx, @@ -2179,11 +2168,11 @@ def range_for_ptrref( 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] ) @@ -2200,6 +2189,9 @@ def range_for_ptrref( # 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: + orig_ptr_info = _get_ptrref_storage_info(orig_ptrref, ctx=ctx) + cols = _get_ptrref_column_names(orig_ptr_info) + overlays = get_ptr_rel_overlays( orig_ptrref, dml_source=dml_source, ctx=ctx) @@ -2277,32 +2269,15 @@ def _range_for_sub_ptrrefs( *, path_id: Optional[irast.PathId], ctx: context.CompilerContextLevel, -) -> tuple[pgast.PathRangeVar, list[str]]: +) -> pgast.PathRangeVar: sub_set_ops = [] - for src_ptrref in sub_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, - ) - if not ptr_info: - ptr_info = pg_types.get_ptrref_storage_info( - src_ptrref, resolve_type=False, link_bias=False, - versioned=ctx.env.versioned_stdlib, - ) - - cols = [ - 'source' if ptr_info.table_type == 'link' else 'id', - ptr_info.column_name, - ] + for sub_ptrref in sub_ptrrefs: + ptr_info = _get_ptrref_storage_info(sub_ptrref, ctx=ctx) + cols = _get_ptrref_column_names(ptr_info) table = table_from_ptrref( - src_ptrref, + sub_ptrref, ptr_info, ctx=ctx, ) @@ -2331,7 +2306,38 @@ def _range_for_sub_ptrrefs( sub_set_ops, name, prep_filter=_prep_filter, path_id=path_id, ctx=ctx) - return sub_rvar, cols + return sub_rvar + + +def _get_ptrref_storage_info( + sub_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( + sub_ptrref, resolve_type=False, link_bias=True, + versioned=ctx.env.versioned_stdlib, + ) + if not ptr_info: + ptr_info = pg_types.get_ptrref_storage_info( + sub_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( From 7cab3c42733aac7a91075b01778f20609183f081 Mon Sep 17 00:00:00 2001 From: dnwpark Date: Tue, 2 Jul 2024 20:01:00 -0400 Subject: [PATCH 09/26] Add PathId from ptrref. --- edb/ir/pathid.py | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) 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) From 809fce0dfcacb82be6684a8aec411488bf8bf776 Mon Sep 17 00:00:00 2001 From: dnwpark Date: Thu, 4 Jul 2024 18:13:25 -0400 Subject: [PATCH 10/26] Create rvar for each ptrref component separately. --- edb/pgsql/compiler/relctx.py | 220 +++++++++++++++++++++++------------ 1 file changed, 143 insertions(+), 77 deletions(-) diff --git a/edb/pgsql/compiler/relctx.py b/edb/pgsql/compiler/relctx.py index 882b236e125..5cfa63ad28f 100644 --- a/edb/pgsql/compiler/relctx.py +++ b/edb/pgsql/compiler/relctx.py @@ -2121,8 +2121,8 @@ def range_for_ptrref( """ 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' ) @@ -2130,11 +2130,11 @@ 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 = ( @@ -2148,23 +2148,21 @@ def range_for_ptrref( set_ops = [] - for orig_ptrref in refs: - assert isinstance(orig_ptrref, irast.PointerRef), \ + for component_ref in component_refs: + assert isinstance(component_ref, irast.PointerRef), \ "expected regular PointerRef" - src_ptrrefs = _expand_sub_ptrrefs( - orig_ptrref, + + component_rvar = _range_for_component_ptrref( + component_ref, + output_cols, + dml_source=dml_source, include_descendants=include_descendants, for_mutation=for_mutation, - ) - - sub_rvar = _range_for_sub_ptrrefs( - src_ptrrefs, - name=orig_ptrref.shortname, - output_cols=output_cols, path_id=path_id, ctx=ctx, ) - sub_qry = pgast.SelectStmt( + + component_qry = pgast.SelectStmt( target_list=[ pgast.ResTarget( val=pgast.ColumnRef( @@ -2174,60 +2172,133 @@ def range_for_ptrref( ) 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: - orig_ptr_info = _get_ptrref_storage_info(orig_ptrref, ctx=ctx) - cols = _get_ptrref_column_names(orig_ptr_info) + return range_from_queryset( + set_ops, + ptrref.shortname, + path_id=path_id, + ctx=ctx, + ) - overlays = get_ptr_rel_overlays( - orig_ptrref, dml_source=dml_source, ctx=ctx) - for op, cte, cte_path_id in overlays: - rvar = rvar_for_rel(cte, ctx=ctx) +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, + ) - qry = pgast.SelectStmt( - target_list=[ - pgast.ResTarget( - val=pgast.ColumnRef( - name=[col] - ) - ) - for col in cols - ], - from_clause=[rvar], + descendant_selects = _selects_for_ptrref_descendants( + ptrref_descendants, + output_cols=output_cols, + path_id=path_id, + ctx=ctx, + ) + descentant_ops = [ + (context.OverlayOp.UNION, select) + for select in descendant_selects + ] + component_rvar = range_from_queryset( + descentant_ops, + component_ptrref.shortname, + path_id=path_id, + ctx=ctx, + ) + + # 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. + 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=[output_colname] + ), + name=output_colname ) - # 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 + 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 + ) + + set_ops.append((context.OverlayOp.UNION, component_qry)) + + 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) + + qry = pgast.SelectStmt( + target_list=[ + pgast.ResTarget( + val=pgast.ColumnRef( + name=[col] + ) ) - 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 _expand_sub_ptrrefs( - ref: irast.PointerRef, +def _get_ptrref_descendants( + ptrref: irast.PointerRef, *, include_descendants: bool, for_mutation: bool, @@ -2242,10 +2313,10 @@ def _expand_sub_ptrrefs( descendants: list[irast.PointerRef] = [] descendants.extend( - cast(Iterable[irast.PointerRef], ref.descendants()) + cast(Iterable[irast.PointerRef], ptrref.descendants()) ) - descendants.append(ref) - assert isinstance(ref, irast.PointerRef) + descendants.append(ptrref) + assert isinstance(ptrref, irast.PointerRef) # Try to only select from actual concrete types. concrete_descendants = [ @@ -2256,28 +2327,27 @@ def _expand_sub_ptrrefs( if concrete_descendants: return concrete_descendants else: - return [ref] + return [ptrref] else: - return [ref] + return [ptrref] -def _range_for_sub_ptrrefs( - sub_ptrrefs: Sequence[irast.PointerRef], - name: sn.QualName, +def _selects_for_ptrref_descendants( + ptrref_descendants: Sequence[irast.PointerRef], output_cols: Iterable[str], *, path_id: Optional[irast.PathId], ctx: context.CompilerContextLevel, -) -> pgast.PathRangeVar: - sub_set_ops = [] +) -> list[pgast.SelectStmt]: + selects = [] - for sub_ptrref in sub_ptrrefs: - ptr_info = _get_ptrref_storage_info(sub_ptrref, ctx=ctx) + 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( - sub_ptrref, + ptrref_descendant, ptr_info, ctx=ctx, ) @@ -2293,7 +2363,7 @@ def _range_for_sub_ptrrefs( qry.target_list.append( pgast.ResTarget(val=selexpr, name=output_colname)) - sub_set_ops.append((context.OverlayOp.UNION, qry)) + selects.append(qry) # We need the identity var for semi_join to work and # the source rvar so that linkprops can be found here. @@ -2302,15 +2372,11 @@ def _range_for_sub_ptrrefs( 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, name, - prep_filter=_prep_filter, path_id=path_id, ctx=ctx) - - return sub_rvar + return selects def _get_ptrref_storage_info( - sub_ptrref: irast.PointerRef, + ptrref: irast.PointerRef, *, ctx: context.CompilerContextLevel ) -> pg_types.PointerStorageInfo: @@ -2320,12 +2386,12 @@ def _get_ptrref_storage_info( # 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( - sub_ptrref, resolve_type=False, link_bias=True, + ptrref, resolve_type=False, link_bias=True, versioned=ctx.env.versioned_stdlib, ) if not ptr_info: ptr_info = pg_types.get_ptrref_storage_info( - sub_ptrref, resolve_type=False, link_bias=False, + ptrref, resolve_type=False, link_bias=False, versioned=ctx.env.versioned_stdlib, ) return ptr_info From fad929fc30bf677b265289d5e21fd7906379ec68 Mon Sep 17 00:00:00 2001 From: dnwpark Date: Thu, 4 Jul 2024 18:21:36 -0400 Subject: [PATCH 11/26] Add ptr_inheritance_ctes to pgsql compiler context. --- edb/pgsql/compiler/context.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/edb/pgsql/compiler/context.py b/edb/pgsql/compiler/context.py index addd2da57f4..7f378019fdf 100644 --- a/edb/pgsql/compiler/context.py +++ b/edb/pgsql/compiler/context.py @@ -246,6 +246,9 @@ class CompilerContextLevel(compiler.ContextLevel): #: 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] @@ -359,6 +362,7 @@ def __init__( self.rel_hierarchy = {} self.param_ctes = {} self.ptr_ctes = {} + self.ptr_inheritance_ctes = {} self.type_ctes = {} self.pending_type_ctes = set() self.type_inheritance_ctes = {} @@ -401,6 +405,7 @@ def __init__( self.rel_hierarchy = prevlevel.rel_hierarchy self.param_ctes = prevlevel.param_ctes self.ptr_ctes = prevlevel.ptr_ctes + self.ptr_inheritance_ctes = prevlevel.ptr_inheritance_ctes self.type_ctes = prevlevel.type_ctes self.pending_type_ctes = prevlevel.pending_type_ctes self.type_inheritance_ctes = prevlevel.type_inheritance_ctes From fab272e63bf90f7fe7214e463bd666fedbfacaf9 Mon Sep 17 00:00:00 2001 From: dnwpark Date: Thu, 4 Jul 2024 18:25:28 -0400 Subject: [PATCH 12/26] Create CTE for pointer references. --- edb/pgsql/compiler/relctx.py | 116 ++++++++++++++++++++++++++++++----- 1 file changed, 100 insertions(+), 16 deletions(-) diff --git a/edb/pgsql/compiler/relctx.py b/edb/pgsql/compiler/relctx.py index 5cfa63ad28f..f1fdd9be9f1 100644 --- a/edb/pgsql/compiler/relctx.py +++ b/edb/pgsql/compiler/relctx.py @@ -2211,22 +2211,106 @@ def _range_for_component_ptrref( for_mutation=for_mutation, ) - descendant_selects = _selects_for_ptrref_descendants( - ptrref_descendants, - output_cols=output_cols, - path_id=path_id, - ctx=ctx, - ) - descentant_ops = [ - (context.OverlayOp.UNION, select) - for select in descendant_selects - ] - component_rvar = range_from_queryset( - descentant_ops, - component_ptrref.shortname, - path_id=path_id, - ctx=ctx, - ) + if ( + not ctx.env.use_type_inheritance_ctes + + # 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, + ) + descentant_ops = [ + (context.OverlayOp.UNION, select) + for select in descendant_selects + ] + component_rvar = range_from_queryset( + descentant_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, + ) + + 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, + ) + + ptr_cte = pgast.CommonTableExpr( + name=ctx.env.aliases.get(f't_{component_ptrref.name}'), + query=inheritance_qry, + materialized=False, + ) + ctx.ptr_inheritance_ctes[component_ptrref.id] = ptr_cte + ctx.ordered_type_ctes.append(ptr_cte) + + 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, + ) + + # Ensure source and target columns are output + for output_colname in output_cols: + selexpr = pgast.ColumnRef( + name=[cte_rvar.alias.aliasname, output_colname]) + sctx.rel.target_list.append( + pgast.ResTarget(val=selexpr, name=output_colname) + ) + + 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, + ) + + component_rvar = rvar_for_rel( + sctx.rel, + typeref=component_ptrref.out_target, + ctx=sctx + ) # Only fire off the overlays at the end of each expanded inhview. # This only matters when we are doing expand_inhviews, and prevents From 479088e582f23af94fc9ca2b7438a0fb7c29ef62 Mon Sep 17 00:00:00 2001 From: dnwpark Date: Fri, 5 Jul 2024 16:09:23 -0400 Subject: [PATCH 13/26] Fix link properties on back links not finding output. --- edb/pgsql/compiler/relctx.py | 4 ++++ edb/pgsql/compiler/relgen.py | 15 +++++++++++++++ 2 files changed, 19 insertions(+) diff --git a/edb/pgsql/compiler/relctx.py b/edb/pgsql/compiler/relctx.py index f1fdd9be9f1..3268f0ef745 100644 --- a/edb/pgsql/compiler/relctx.py +++ b/edb/pgsql/compiler/relctx.py @@ -2257,6 +2257,10 @@ def _range_for_component_ptrref( rarg=rarg, ) + # Add the path to the CTE's query. This allows for the proper + # path mapping ot occur when processing link properties + inheritance_qry.path_id = component_ptrref_path_id + ptr_cte = pgast.CommonTableExpr( name=ctx.env.aliases.get(f't_{component_ptrref.name}'), query=inheritance_qry, diff --git a/edb/pgsql/compiler/relgen.py b/edb/pgsql/compiler/relgen.py index 89cd5b38023..811d350c618 100644 --- a/edb/pgsql/compiler/relgen.py +++ b/edb/pgsql/compiler/relgen.py @@ -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, From cfa05fe244aeeff08e437a2ef8af025f2d6df5b5 Mon Sep 17 00:00:00 2001 From: dnwpark Date: Fri, 5 Jul 2024 16:13:07 -0400 Subject: [PATCH 14/26] Rename type inheritance variables to be more general. --- edb/edgeql/compiler/options.py | 2 +- edb/edgeql/compiler/typegen.py | 2 +- edb/pgsql/compiler/__init__.py | 4 ++-- edb/pgsql/compiler/clauses.py | 2 +- edb/pgsql/compiler/context.py | 10 +++++----- edb/pgsql/compiler/dml.py | 4 ++-- edb/pgsql/compiler/relctx.py | 10 +++++----- edb/server/compiler/compiler.py | 4 ++-- 8 files changed, 19 insertions(+), 19 deletions(-) diff --git a/edb/edgeql/compiler/options.py b/edb/edgeql/compiler/options.py index e1c0c24907e..11a345b20b2 100644 --- a/edb/edgeql/compiler/options.py +++ b/edb/edgeql/compiler/options.py @@ -85,7 +85,7 @@ class GlobalCompilerOptions: #: Should type inheritance be expanded using CTEs. #: When not explaining CTEs can be used to provide access to a type and its #: descendents. - use_type_inheritance_ctes: bool = True + use_inheritance_ctes: bool = True #: 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 6920d132861..a22026d3321 100644 --- a/edb/edgeql/compiler/typegen.py +++ b/edb/edgeql/compiler/typegen.py @@ -404,7 +404,7 @@ def type_to_typeref( or ( ( env.options.expand_inhviews or - env.options.use_type_inheritance_ctes + env.options.use_inheritance_ctes ) and isinstance(t, s_objtypes.ObjectType) ) diff --git a/edb/pgsql/compiler/__init__.py b/edb/pgsql/compiler/__init__.py index e6d53d6d4b9..df496c80351 100644 --- a/edb/pgsql/compiler/__init__.py +++ b/edb/pgsql/compiler/__init__.py @@ -67,7 +67,7 @@ def compile_ir_to_sql_tree( named_param_prefix: Optional[tuple[str, ...]] = None, expected_cardinality_one: bool = False, expand_inhviews: bool = False, - use_type_inheritance_ctes: bool = False, + use_inheritance_ctes: bool = False, external_rvars: Optional[ Mapping[Tuple[irast.PathId, pgce.PathAspect], pgast.PathRangeVar] ] = None, @@ -129,7 +129,7 @@ def compile_ir_to_sql_tree( ignore_object_shapes=ignore_shapes, explicit_top_cast=explicit_top_cast, expand_inhviews=expand_inhviews, - use_type_inheritance_ctes=use_type_inheritance_ctes, + use_inheritance_ctes=use_inheritance_ctes, singleton_mode=singleton_mode, scope_tree_nodes=scope_tree_nodes, external_rvars=external_rvars, diff --git a/edb/pgsql/compiler/clauses.py b/edb/pgsql/compiler/clauses.py index 227debab59b..d2c2e7b01bc 100644 --- a/edb/pgsql/compiler/clauses.py +++ b/edb/pgsql/compiler/clauses.py @@ -420,7 +420,7 @@ def fini_toplevel( stmt.ctes[:0] = [ *ctx.param_ctes.values(), *ctx.ptr_ctes.values(), - *ctx.ordered_type_ctes, + *ctx.ordered_inheritance_ctes, ] if ctx.env.named_param_prefix is None: diff --git a/edb/pgsql/compiler/context.py b/edb/pgsql/compiler/context.py index 7f378019fdf..439609e714e 100644 --- a/edb/pgsql/compiler/context.py +++ b/edb/pgsql/compiler/context.py @@ -260,7 +260,7 @@ class CompilerContextLevel(compiler.ContextLevel): # 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] + ordered_inheritance_ctes: list[pgast.CommonTableExpr] #: The logical parent of the current query in the #: query hierarchy @@ -366,7 +366,7 @@ def __init__( self.type_ctes = {} self.pending_type_ctes = set() self.type_inheritance_ctes = {} - self.ordered_type_ctes = [] + self.ordered_inheritance_ctes = [] self.dml_stmts = {} self.parent_rel = None self.pending_query = None @@ -409,7 +409,7 @@ def __init__( self.type_ctes = prevlevel.type_ctes self.pending_type_ctes = prevlevel.pending_type_ctes self.type_inheritance_ctes = prevlevel.type_inheritance_ctes - self.ordered_type_ctes = prevlevel.ordered_type_ctes + self.ordered_inheritance_ctes = prevlevel.ordered_inheritance_ctes self.dml_stmts = prevlevel.dml_stmts self.parent_rel = prevlevel.parent_rel self.pending_query = prevlevel.pending_query @@ -553,7 +553,7 @@ def __init__( ignore_object_shapes: bool, singleton_mode: bool, expand_inhviews: bool, - use_type_inheritance_ctes: bool, + use_inheritance_ctes: bool, explicit_top_cast: Optional[irast.TypeRef], query_params: List[irast.Param], type_rewrites: Dict[RewriteKey, irast.Set], @@ -573,7 +573,7 @@ def __init__( self.ignore_object_shapes = ignore_object_shapes self.singleton_mode = singleton_mode self.expand_inhviews = expand_inhviews - self.use_type_inheritance_ctes = use_type_inheritance_ctes + self.use_inheritance_ctes = use_inheritance_ctes 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 77c68d6a58b..92364b6e483 100644 --- a/edb/pgsql/compiler/dml.py +++ b/edb/pgsql/compiler/dml.py @@ -3141,7 +3141,7 @@ def compile_triggers( ictx.trigger_mode = True ictx.type_ctes = {} ictx.type_inheritance_ctes = {} - ictx.ordered_type_ctes = [] + ictx.ordered_inheritance_ctes = [] ictx.toplevel_stmt = stmt for stage in triggers: @@ -3158,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] = ictx.ordered_type_ctes + stmt.ctes[start_ctes:start_ctes] = ictx.ordered_inheritance_ctes def compile_trigger( diff --git a/edb/pgsql/compiler/relctx.py b/edb/pgsql/compiler/relctx.py index 3268f0ef745..ab3931c0f28 100644 --- a/edb/pgsql/compiler/relctx.py +++ b/edb/pgsql/compiler/relctx.py @@ -1621,7 +1621,7 @@ def range_for_material_objtype( materialized=is_global or force_cte, ) ctx.type_ctes[key] = type_cte - ctx.ordered_type_ctes.append(type_cte) + ctx.ordered_inheritance_ctes.append(type_cte) type_rel = type_cte else: type_rel = type_cte @@ -1655,7 +1655,7 @@ def range_for_material_objtype( # 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. - not ctx.env.use_type_inheritance_ctes + not ctx.env.use_inheritance_ctes # Don't use CTEs if there is no inheritance. (ie. There is only a # single material type) @@ -1717,7 +1717,7 @@ def range_for_material_objtype( materialized=False, ) ctx.type_inheritance_ctes[typeref.id] = type_cte - ctx.ordered_type_ctes.append(type_cte) + ctx.ordered_inheritance_ctes.append(type_cte) else: type_cte = ctx.type_inheritance_ctes[typeref.id] @@ -2212,7 +2212,7 @@ def _range_for_component_ptrref( ) if ( - not ctx.env.use_type_inheritance_ctes + not ctx.env.use_inheritance_ctes # Don't use CTEs if there is no inheritance. (ie. There is only a # single ptrref) @@ -2267,7 +2267,7 @@ def _range_for_component_ptrref( materialized=False, ) ctx.ptr_inheritance_ctes[component_ptrref.id] = ptr_cte - ctx.ordered_type_ctes.append(ptr_cte) + ctx.ordered_inheritance_ctes.append(ptr_cte) else: ptr_cte = ctx.ptr_inheritance_ctes[component_ptrref.id] diff --git a/edb/server/compiler/compiler.py b/edb/server/compiler/compiler.py index 78c7d5b5141..a93f9333663 100644 --- a/edb/server/compiler/compiler.py +++ b/edb/server/compiler/compiler.py @@ -1705,7 +1705,7 @@ def _get_compile_options( and not ctx.bootstrap_mode and not ctx.schema_reflection_mode ) or is_explain, - use_type_inheritance_ctes=( + use_inheritance_ctes=( not is_explain and not ctx.bootstrap_mode and not ctx.schema_reflection_mode @@ -1895,7 +1895,7 @@ def _compile_ql_query( output_format=_convert_format(ctx.output_format), backend_runtime_params=ctx.backend_runtime_params, expand_inhviews=options.expand_inhviews, - use_type_inheritance_ctes=options.use_type_inheritance_ctes, + use_inheritance_ctes=options.use_inheritance_ctes, detach_params=(use_persistent_cache and cache_mode is config.QueryCacheMode.PgFunc), versioned_stdlib=True, From f394397417fad6f8d544a85957a5d70923bd748d Mon Sep 17 00:00:00 2001 From: dnwpark Date: Fri, 5 Jul 2024 16:15:38 -0400 Subject: [PATCH 15/26] Reorder some functions. --- edb/pgsql/compiler/relctx.py | 90 ++++++++++++++++++------------------ 1 file changed, 45 insertions(+), 45 deletions(-) diff --git a/edb/pgsql/compiler/relctx.py b/edb/pgsql/compiler/relctx.py index ab3931c0f28..3d9dbad417d 100644 --- a/edb/pgsql/compiler/relctx.py +++ b/edb/pgsql/compiler/relctx.py @@ -2061,50 +2061,6 @@ def range_from_queryset( return rvar -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 _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 range_for_ptrref( ptrref: irast.BasePointerRef, *, dml_source: Sequence[irast.MutatingLikeStmt]=(), @@ -2385,6 +2341,16 @@ def _range_for_component_ptrref( 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, *, @@ -2434,7 +2400,7 @@ def _selects_for_ptrref_descendants( ptr_info = _get_ptrref_storage_info(ptrref_descendant, ctx=ctx) cols = _get_ptrref_column_names(ptr_info) - table = table_from_ptrref( + table = _table_from_ptrref( ptrref_descendant, ptr_info, ctx=ctx, @@ -2463,6 +2429,40 @@ def _selects_for_ptrref_descendants( 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, *, From 4dccd62e91554bba1d90a5a38a368cfb88229c28 Mon Sep 17 00:00:00 2001 From: dnwpark Date: Mon, 8 Jul 2024 14:30:59 -0400 Subject: [PATCH 16/26] Replace expand_inhview flag with is_explain and update comments. --- edb/common/debug.py | 3 --- edb/edgeql/compiler/options.py | 6 +++--- edb/edgeql/compiler/typegen.py | 2 +- edb/pgsql/compiler/__init__.py | 4 ++-- edb/pgsql/compiler/context.py | 4 ++-- edb/pgsql/compiler/relctx.py | 20 +++++++++----------- edb/pgsql/compiler/relgen.py | 2 +- edb/server/compiler/compiler.py | 8 ++------ 8 files changed, 20 insertions(+), 29 deletions(-) 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 11a345b20b2..a13e0ecf2d4 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 #: Should type inheritance be expanded using CTEs. #: When not explaining CTEs can be used to provide access to a type and its diff --git a/edb/edgeql/compiler/typegen.py b/edb/edgeql/compiler/typegen.py index a22026d3321..8751ae6a137 100644 --- a/edb/edgeql/compiler/typegen.py +++ b/edb/edgeql/compiler/typegen.py @@ -403,7 +403,7 @@ def type_to_typeref( or expr_type is s_types.ExprType.Delete or ( ( - env.options.expand_inhviews or + env.options.is_explain or env.options.use_inheritance_ctes ) and isinstance(t, s_objtypes.ObjectType) diff --git a/edb/pgsql/compiler/__init__.py b/edb/pgsql/compiler/__init__.py index df496c80351..6f9895fec81 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, use_inheritance_ctes: bool = False, external_rvars: Optional[ Mapping[Tuple[irast.PathId, pgce.PathAspect], pgast.PathRangeVar] @@ -128,7 +128,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, use_inheritance_ctes=use_inheritance_ctes, singleton_mode=singleton_mode, scope_tree_nodes=scope_tree_nodes, diff --git a/edb/pgsql/compiler/context.py b/edb/pgsql/compiler/context.py index 439609e714e..e62d2339634 100644 --- a/edb/pgsql/compiler/context.py +++ b/edb/pgsql/compiler/context.py @@ -552,7 +552,7 @@ def __init__( expected_cardinality_one: bool, ignore_object_shapes: bool, singleton_mode: bool, - expand_inhviews: bool, + is_explain: bool, use_inheritance_ctes: bool, explicit_top_cast: Optional[irast.TypeRef], query_params: List[irast.Param], @@ -572,7 +572,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.use_inheritance_ctes = use_inheritance_ctes self.explicit_top_cast = explicit_top_cast self.query_params = query_params diff --git a/edb/pgsql/compiler/relctx.py b/edb/pgsql/compiler/relctx.py index 3d9dbad417d..5bd2e0a3a95 100644 --- a/edb/pgsql/compiler/relctx.py +++ b/edb/pgsql/compiler/relctx.py @@ -1609,10 +1609,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( @@ -1652,9 +1652,9 @@ def range_for_material_objtype( if ( # 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. + # 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. not ctx.env.use_inheritance_ctes # Don't use CTEs if there is no inheritance. (ie. There is only a @@ -2214,7 +2214,7 @@ def _range_for_component_ptrref( ) # Add the path to the CTE's query. This allows for the proper - # path mapping ot occur when processing link properties + # path mapping to occur when processing link properties inheritance_qry.path_id = component_ptrref_path_id ptr_cte = pgast.CommonTableExpr( @@ -2272,9 +2272,7 @@ def _range_for_component_ptrref( ctx=sctx ) - # 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. + # 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: @@ -2357,8 +2355,8 @@ def _get_ptrref_descendants( include_descendants: bool, for_mutation: bool, ) -> list[irast.PointerRef]: - # expand_inhviews helps support EXPLAIN. see - # range_for_material_objtype for details. + # When doing EXPLAIN, don't use CTEs. See range_for_material_objtype for + # details. if ( include_descendants and not for_mutation diff --git a/edb/pgsql/compiler/relgen.py b/edb/pgsql/compiler/relgen.py index 811d350c618..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 = [] diff --git a/edb/server/compiler/compiler.py b/edb/server/compiler/compiler.py index a93f9333663..39f23ef2774 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, use_inheritance_ctes=( not is_explain and not ctx.bootstrap_mode @@ -1894,7 +1890,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, use_inheritance_ctes=options.use_inheritance_ctes, detach_params=(use_persistent_cache and cache_mode is config.QueryCacheMode.PgFunc), From 46e8083eb85ebb5c2f355df71a82ce21adad30e0 Mon Sep 17 00:00:00 2001 From: dnwpark Date: Tue, 9 Jul 2024 10:48:36 -0400 Subject: [PATCH 17/26] Fix spelling. --- edb/pgsql/compiler/relctx.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/edb/pgsql/compiler/relctx.py b/edb/pgsql/compiler/relctx.py index 5bd2e0a3a95..24aa2871738 100644 --- a/edb/pgsql/compiler/relctx.py +++ b/edb/pgsql/compiler/relctx.py @@ -2180,12 +2180,12 @@ def _range_for_component_ptrref( path_id=path_id, ctx=ctx, ) - descentant_ops = [ + descendant_ops = [ (context.OverlayOp.UNION, select) for select in descendant_selects ] component_rvar = range_from_queryset( - descentant_ops, + descendant_ops, component_ptrref.shortname, path_id=path_id, ctx=ctx, From b0ffb0717b956611523d6188976eb0fa310d1412 Mon Sep 17 00:00:00 2001 From: dnwpark Date: Tue, 9 Jul 2024 17:49:01 -0400 Subject: [PATCH 18/26] Refactor range_for_material_objtype to remove recursion and extra branch. --- edb/pgsql/compiler/relctx.py | 168 +++++++++++++++++++++-------------- 1 file changed, 103 insertions(+), 65 deletions(-) diff --git a/edb/pgsql/compiler/relctx.py b/edb/pgsql/compiler/relctx.py index 24aa2871738..6f1bc7a2997 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 = ( @@ -1630,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) @@ -1641,14 +1637,17 @@ def range_for_material_objtype( rvar = rvar_for_rel( sctx.rel, lateral=lateral, typeref=typeref, ctx=sctx) - elif ( - include_descendants - and not for_mutation - and typeref.children is not None + else: + typeref_descendants = _get_typeref_descendants( + typeref, + include_descendants=( + include_descendants - # HACK: This is a workaround for #4491 - and typeref.name_hint.module not in {'cfg', 'sys'} - ): + # HACK: This is a workaround for #4491 + and typeref.name_hint.module not in {'cfg', 'sys'} + ), + 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 @@ -1659,14 +1658,14 @@ def range_for_material_objtype( # Don't use CTEs if there is no inheritance. (ie. There is only a # single material type) - or len(typeref.children) == 0 + or len(typeref_descendants) <= 1 ): - inheritance_selects = _selects_for_typeref_inheritance( - typeref, + inheritance_selects = _selects_for_typeref_descendants( + typeref_descendants, path_id, - lateral=lateral, - ignore_rewrites=ignore_rewrites, - ctx=ctx + for_mutation=for_mutation, + include_descendants=include_descendants, + ctx=ctx, ) ops = [ (context.OverlayOp.UNION, select) @@ -1694,12 +1693,12 @@ def range_for_material_objtype( ) if typeref.id not in ctx.type_inheritance_ctes: - inheritance_selects = _selects_for_typeref_inheritance( - typeref, + inheritance_selects = _selects_for_typeref_descendants( + typeref_descendants, typeref_path, - lateral=lateral, - ignore_rewrites=ignore_rewrites, - ctx=ctx + for_mutation=False, + include_descendants=False, + ctx=ctx, ) type_qry: pgast.SelectStmt = inheritance_selects[0] @@ -1740,34 +1739,6 @@ def range_for_material_objtype( rvar = rvar_for_rel( sctx.rel, lateral=lateral, typeref=typeref, ctx=sctx) - else: - assert not typeref.is_view, "attempting to generate range from view" - table_schema_name, table_name = common.get_objtype_backend_name( - typeref.id, - typeref.name_hint.module, - aspect=( - 'table' if for_mutation or not include_descendants else - 'inhview' - ), - 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, - ) - - rvar = pgast.RelRangeVar( - relation=relation, - typeref=typeref, - alias=pgast.Alias( - aliasname=env.aliases.get(typeref.name_hint.name) - ) - ) - overlays = get_type_rel_overlays(typeref, dml_source=dml_source, ctx=ctx) external_rvar = ctx.env.external_rvars.get( (path_id, pgce.PathAspect.SOURCE) @@ -1805,7 +1776,7 @@ def range_for_material_objtype( qry_rvar = pgast.RangeSubselect( subquery=qry, alias=pgast.Alias( - aliasname=env.aliases.get(hint=cte.name or '') + aliasname=ctx.env.aliases.get(hint=cte.name or '') ) ) @@ -1835,25 +1806,48 @@ def range_for_material_objtype( return rvar -def _selects_for_typeref_inheritance( +def _get_typeref_descendants( typeref: irast.TypeRef, + *, + include_descendants: bool, + for_mutation: bool, +) -> list[irast.TypeRef]: + if ( + include_descendants + and not for_mutation + ): + descendants = [typeref, *irtyputils.get_typeref_descendants(typeref)] + + # 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, *, - lateral: bool=False, - ignore_rewrites: bool=False, + for_mutation: bool, + include_descendants: bool, ctx: context.CompilerContextLevel, ) -> list[pgast.SelectStmt]: selects = [] - 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? + for subref in typeref_descendants: + rvar = _table_from_typeref( + subref, + path_id, + for_mutation=for_mutation, + include_descendants=include_descendants, ctx=ctx, ) qry = pgast.SelectStmt(from_clause=[rvar]) @@ -1866,6 +1860,50 @@ def _selects_for_typeref_inheritance( return selects +def _table_from_typeref( + typeref: irast.TypeRef, + path_id: irast.PathId, + *, + for_mutation: bool, + include_descendants: bool, + ctx: context.CompilerContextLevel, +) -> pgast.PathRangeVar: + assert isinstance(typeref.name_hint, sn.QualName) + assert not typeref.is_view, "attempting to generate range from view" + + aspect = 'table' + + if ( + include_descendants + and not for_mutation + and typeref.name_hint.module in {'cfg', 'sys', 'schema'} + ): + aspect = 'inhview' + + 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, From add6e7f8e03eaef12969a38583539f5e7dbd2784 Mon Sep 17 00:00:00 2001 From: dnwpark Date: Wed, 10 Jul 2024 10:57:32 -0400 Subject: [PATCH 19/26] Remove ptr_cte field and keep pointer ctes separate from type ctes. --- edb/pgsql/compiler/clauses.py | 4 ++-- edb/pgsql/compiler/context.py | 28 ++++++++++++---------------- edb/pgsql/compiler/dml.py | 6 +++--- edb/pgsql/compiler/relctx.py | 13 ++++++------- 4 files changed, 23 insertions(+), 28 deletions(-) diff --git a/edb/pgsql/compiler/clauses.py b/edb/pgsql/compiler/clauses.py index d2c2e7b01bc..985d980e877 100644 --- a/edb/pgsql/compiler/clauses.py +++ b/edb/pgsql/compiler/clauses.py @@ -419,8 +419,8 @@ def fini_toplevel( stmt.ctes = [] stmt.ctes[:0] = [ *ctx.param_ctes.values(), - *ctx.ptr_ctes.values(), - *ctx.ordered_inheritance_ctes, + *ctx.ptr_inheritance_ctes.values(), + *ctx.ordered_type_ctes, ] if ctx.env.named_param_prefix is None: diff --git a/edb/pgsql/compiler/context.py b/edb/pgsql/compiler/context.py index e62d2339634..129ea620c85 100644 --- a/edb/pgsql/compiler/context.py +++ b/edb/pgsql/compiler/context.py @@ -242,25 +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_inheritance_ctes: list[pgast.CommonTableExpr] + ordered_type_ctes: list[pgast.CommonTableExpr] #: The logical parent of the current query in the #: query hierarchy @@ -361,12 +357,11 @@ def __init__( self.rel = NO_STMT self.rel_hierarchy = {} self.param_ctes = {} - self.ptr_ctes = {} self.ptr_inheritance_ctes = {} - self.type_ctes = {} - self.pending_type_ctes = set() + self.type_rewrite_ctes = {} + self.pending_type_rewrite_ctes = set() self.type_inheritance_ctes = {} - self.ordered_inheritance_ctes = [] + self.ordered_type_ctes = [] self.dml_stmts = {} self.parent_rel = None self.pending_query = None @@ -404,12 +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.ptr_inheritance_ctes = prevlevel.ptr_inheritance_ctes - self.type_ctes = prevlevel.type_ctes - self.pending_type_ctes = prevlevel.pending_type_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_inheritance_ctes = prevlevel.ordered_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 @@ -472,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() diff --git a/edb/pgsql/compiler/dml.py b/edb/pgsql/compiler/dml.py index 92364b6e483..3fcb2803b12 100644 --- a/edb/pgsql/compiler/dml.py +++ b/edb/pgsql/compiler/dml.py @@ -3139,9 +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_inheritance_ctes = [] + ictx.ordered_type_ctes = [] ictx.toplevel_stmt = stmt for stage in triggers: @@ -3158,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] = ictx.ordered_inheritance_ctes + 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 6f1bc7a2997..68f5350eff7 100644 --- a/edb/pgsql/compiler/relctx.py +++ b/edb/pgsql/compiler/relctx.py @@ -1566,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: @@ -1592,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 @@ -1616,8 +1616,8 @@ def range_for_material_objtype( query=sctx.rel, materialized=is_global or force_cte, ) - ctx.type_ctes[key] = type_cte - ctx.ordered_inheritance_ctes.append(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 @@ -1716,7 +1716,7 @@ def range_for_material_objtype( materialized=False, ) ctx.type_inheritance_ctes[typeref.id] = type_cte - ctx.ordered_inheritance_ctes.append(type_cte) + ctx.ordered_type_ctes.append(type_cte) else: type_cte = ctx.type_inheritance_ctes[typeref.id] @@ -2261,7 +2261,6 @@ def _range_for_component_ptrref( materialized=False, ) ctx.ptr_inheritance_ctes[component_ptrref.id] = ptr_cte - ctx.ordered_inheritance_ctes.append(ptr_cte) else: ptr_cte = ctx.ptr_inheritance_ctes[component_ptrref.id] From 0d2f2b911a788f106c4ab6571b4cfb3446701406 Mon Sep 17 00:00:00 2001 From: dnwpark Date: Wed, 10 Jul 2024 11:07:13 -0400 Subject: [PATCH 20/26] Move assert forward in range for typeref process. --- edb/pgsql/compiler/relctx.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/edb/pgsql/compiler/relctx.py b/edb/pgsql/compiler/relctx.py index 68f5350eff7..dfeeff6eb57 100644 --- a/edb/pgsql/compiler/relctx.py +++ b/edb/pgsql/compiler/relctx.py @@ -1638,6 +1638,7 @@ def range_for_material_objtype( sctx.rel, lateral=lateral, typeref=typeref, ctx=sctx) else: + assert not typeref.is_view, "attempting to generate range from view" typeref_descendants = _get_typeref_descendants( typeref, include_descendants=( @@ -1869,7 +1870,6 @@ def _table_from_typeref( ctx: context.CompilerContextLevel, ) -> pgast.PathRangeVar: assert isinstance(typeref.name_hint, sn.QualName) - assert not typeref.is_view, "attempting to generate range from view" aspect = 'table' From 7c8352d81e352c0f159fa13ca687743dab37bf1f Mon Sep 17 00:00:00 2001 From: dnwpark Date: Thu, 11 Jul 2024 10:04:34 -0400 Subject: [PATCH 21/26] Fix CTEs in pointer conversion expressions. --- edb/pgsql/delta.py | 41 ++++++++++++++++++++++------------------- 1 file changed, 22 insertions(+), 19 deletions(-) diff --git a/edb/pgsql/delta.py b/edb/pgsql/delta.py index 59b3e909ba2..551648153a1 100644 --- a/edb/pgsql/delta.py +++ b/edb/pgsql/delta.py @@ -4320,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, @@ -4540,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, @@ -4694,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 ) @@ -4737,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}) ''' @@ -4857,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 ]: """ @@ -5156,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 From e3d754191c6b184973bd8797e8c08d3a3bc93571 Mon Sep 17 00:00:00 2001 From: dnwpark Date: Thu, 11 Jul 2024 11:31:47 -0400 Subject: [PATCH 22/26] Remove special case for cfg and sys. --- edb/pgsql/compiler/relctx.py | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/edb/pgsql/compiler/relctx.py b/edb/pgsql/compiler/relctx.py index dfeeff6eb57..535ee5387c4 100644 --- a/edb/pgsql/compiler/relctx.py +++ b/edb/pgsql/compiler/relctx.py @@ -1641,12 +1641,7 @@ def range_for_material_objtype( assert not typeref.is_view, "attempting to generate range from view" typeref_descendants = _get_typeref_descendants( typeref, - include_descendants=( - include_descendants - - # HACK: This is a workaround for #4491 - and typeref.name_hint.module not in {'cfg', 'sys'} - ), + include_descendants=include_descendants, for_mutation=for_mutation, ) if ( @@ -2131,12 +2126,7 @@ def range_for_ptrref( component_refs = {ptrref} assert isinstance(ptrref.out_source.name_hint, sn.QualName) - include_descendants = ( - not ptrref.union_is_exhaustive - - # HACK: This is a workaround for #4491 - and ptrref.out_source.name_hint.module not in {'sys', 'cfg'} - ) + include_descendants = not ptrref.union_is_exhaustive output_cols = ('source', 'target') From 6f38582fedf50252fbc856255b076668681e65b2 Mon Sep 17 00:00:00 2001 From: dnwpark Date: Thu, 11 Jul 2024 14:52:24 -0400 Subject: [PATCH 23/26] Remove special case for schema. --- edb/pgsql/compiler/relctx.py | 29 +++-------------------------- edb/server/compiler/compiler.py | 2 -- 2 files changed, 3 insertions(+), 28 deletions(-) diff --git a/edb/pgsql/compiler/relctx.py b/edb/pgsql/compiler/relctx.py index 535ee5387c4..947ba79c46b 100644 --- a/edb/pgsql/compiler/relctx.py +++ b/edb/pgsql/compiler/relctx.py @@ -1657,11 +1657,7 @@ def range_for_material_objtype( or len(typeref_descendants) <= 1 ): inheritance_selects = _selects_for_typeref_descendants( - typeref_descendants, - path_id, - for_mutation=for_mutation, - include_descendants=include_descendants, - ctx=ctx, + typeref_descendants, path_id, ctx=ctx, ) ops = [ (context.OverlayOp.UNION, select) @@ -1690,11 +1686,7 @@ def range_for_material_objtype( if typeref.id not in ctx.type_inheritance_ctes: inheritance_selects = _selects_for_typeref_descendants( - typeref_descendants, - typeref_path, - for_mutation=False, - include_descendants=False, - ctx=ctx, + typeref_descendants, typeref_path, ctx=ctx, ) type_qry: pgast.SelectStmt = inheritance_selects[0] @@ -1833,18 +1825,12 @@ def _selects_for_typeref_descendants( typeref_descendants: Sequence[irast.TypeRef], path_id: irast.PathId, *, - for_mutation: bool, - include_descendants: bool, ctx: context.CompilerContextLevel, ) -> list[pgast.SelectStmt]: selects = [] for subref in typeref_descendants: rvar = _table_from_typeref( - subref, - path_id, - for_mutation=for_mutation, - include_descendants=include_descendants, - ctx=ctx, + subref, path_id, ctx=ctx, ) qry = pgast.SelectStmt(from_clause=[rvar]) sub_path_id = path_id @@ -1860,21 +1846,12 @@ def _table_from_typeref( typeref: irast.TypeRef, path_id: irast.PathId, *, - for_mutation: bool, - include_descendants: bool, ctx: context.CompilerContextLevel, ) -> pgast.PathRangeVar: assert isinstance(typeref.name_hint, sn.QualName) aspect = 'table' - if ( - include_descendants - and not for_mutation - and typeref.name_hint.module in {'cfg', 'sys', 'schema'} - ): - aspect = 'inhview' - table_schema_name, table_name = common.get_objtype_backend_name( typeref.id, typeref.name_hint.module, diff --git a/edb/server/compiler/compiler.py b/edb/server/compiler/compiler.py index 39f23ef2774..4b2804b87c4 100644 --- a/edb/server/compiler/compiler.py +++ b/edb/server/compiler/compiler.py @@ -1703,8 +1703,6 @@ def _get_compile_options( is_explain=is_explain, use_inheritance_ctes=( not is_explain - and not ctx.bootstrap_mode - and not ctx.schema_reflection_mode ), testmode=_get_config_val(ctx, '__internal_testmode'), schema_reflection_mode=( From 1b4643bd2b7369b3df4495acebca246f3798351a Mon Sep 17 00:00:00 2001 From: dnwpark Date: Thu, 11 Jul 2024 14:55:46 -0400 Subject: [PATCH 24/26] Remove use_inheritance_ctes flag. --- edb/edgeql/compiler/options.py | 5 ----- edb/edgeql/compiler/typegen.py | 8 +------- edb/pgsql/compiler/__init__.py | 2 -- edb/pgsql/compiler/context.py | 2 -- edb/pgsql/compiler/relctx.py | 6 ++++-- edb/server/compiler/compiler.py | 4 ---- 6 files changed, 5 insertions(+), 22 deletions(-) diff --git a/edb/edgeql/compiler/options.py b/edb/edgeql/compiler/options.py index a13e0ecf2d4..d76c2171f7e 100644 --- a/edb/edgeql/compiler/options.py +++ b/edb/edgeql/compiler/options.py @@ -82,11 +82,6 @@ class GlobalCompilerOptions: #: the query plan. is_explain: bool = False - #: Should type inheritance be expanded using CTEs. - #: When not explaining CTEs can be used to provide access to a type and its - #: descendents. - use_inheritance_ctes: bool = True - #: The name that can be used in a "DML is disallowed in ..." #: error. When this is not None, any DML should cause an error. in_ddl_context_name: Optional[str] = None diff --git a/edb/edgeql/compiler/typegen.py b/edb/edgeql/compiler/typegen.py index 8751ae6a137..e172a7647c1 100644 --- a/edb/edgeql/compiler/typegen.py +++ b/edb/edgeql/compiler/typegen.py @@ -401,13 +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.is_explain or - env.options.use_inheritance_ctes - ) - 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/pgsql/compiler/__init__.py b/edb/pgsql/compiler/__init__.py index 6f9895fec81..30ef1f09cfd 100644 --- a/edb/pgsql/compiler/__init__.py +++ b/edb/pgsql/compiler/__init__.py @@ -67,7 +67,6 @@ def compile_ir_to_sql_tree( named_param_prefix: Optional[tuple[str, ...]] = None, expected_cardinality_one: bool = False, is_explain: bool = False, - use_inheritance_ctes: bool = False, external_rvars: Optional[ Mapping[Tuple[irast.PathId, pgce.PathAspect], pgast.PathRangeVar] ] = None, @@ -129,7 +128,6 @@ def compile_ir_to_sql_tree( ignore_object_shapes=ignore_shapes, explicit_top_cast=explicit_top_cast, is_explain=is_explain, - use_inheritance_ctes=use_inheritance_ctes, singleton_mode=singleton_mode, scope_tree_nodes=scope_tree_nodes, external_rvars=external_rvars, diff --git a/edb/pgsql/compiler/context.py b/edb/pgsql/compiler/context.py index 129ea620c85..ec50561b8c6 100644 --- a/edb/pgsql/compiler/context.py +++ b/edb/pgsql/compiler/context.py @@ -549,7 +549,6 @@ def __init__( ignore_object_shapes: bool, singleton_mode: bool, is_explain: bool, - use_inheritance_ctes: bool, explicit_top_cast: Optional[irast.TypeRef], query_params: List[irast.Param], type_rewrites: Dict[RewriteKey, irast.Set], @@ -569,7 +568,6 @@ def __init__( self.ignore_object_shapes = ignore_object_shapes self.singleton_mode = singleton_mode self.is_explain = is_explain - self.use_inheritance_ctes = use_inheritance_ctes self.explicit_top_cast = explicit_top_cast self.query_params = query_params self.type_rewrites = type_rewrites diff --git a/edb/pgsql/compiler/relctx.py b/edb/pgsql/compiler/relctx.py index 947ba79c46b..29316b87f02 100644 --- a/edb/pgsql/compiler/relctx.py +++ b/edb/pgsql/compiler/relctx.py @@ -1650,7 +1650,7 @@ def range_for_material_objtype( # 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. - not ctx.env.use_inheritance_ctes + ctx.env.is_explain # Don't use CTEs if there is no inheritance. (ie. There is only a # single material type) @@ -2173,7 +2173,9 @@ def _range_for_component_ptrref( ) if ( - not ctx.env.use_inheritance_ctes + # 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) diff --git a/edb/server/compiler/compiler.py b/edb/server/compiler/compiler.py index 4b2804b87c4..d717441ccf5 100644 --- a/edb/server/compiler/compiler.py +++ b/edb/server/compiler/compiler.py @@ -1701,9 +1701,6 @@ def _get_compile_options( allow_user_specified_id=_get_config_val( ctx, 'allow_user_specified_id') or ctx.schema_reflection_mode, is_explain=is_explain, - use_inheritance_ctes=( - not is_explain - ), testmode=_get_config_val(ctx, '__internal_testmode'), schema_reflection_mode=( ctx.schema_reflection_mode @@ -1889,7 +1886,6 @@ def _compile_ql_query( output_format=_convert_format(ctx.output_format), backend_runtime_params=ctx.backend_runtime_params, is_explain=options.is_explain, - use_inheritance_ctes=options.use_inheritance_ctes, detach_params=(use_persistent_cache and cache_mode is config.QueryCacheMode.PgFunc), versioned_stdlib=True, From bafd35be640c3f4ff5ebada335c3ee54d226aadf Mon Sep 17 00:00:00 2001 From: dnwpark Date: Thu, 11 Jul 2024 15:29:45 -0400 Subject: [PATCH 25/26] Remove cfg view children from non-cfg view typerefs. --- edb/ir/ast.py | 2 ++ edb/ir/typeutils.py | 31 +++++++++++++++++++++++++++++++ edb/pgsql/compiler/relctx.py | 18 +++++++++++++++++- edb/pgsql/delta.py | 35 +++++++++++++++++++---------------- edb/pgsql/inheritance.py | 8 +++++--- edb/pgsql/types.py | 30 ------------------------------ edb/server/bootstrap.py | 4 ++-- 7 files changed, 76 insertions(+), 52 deletions(-) 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/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/relctx.py b/edb/pgsql/compiler/relctx.py index 29316b87f02..e3471dcd535 100644 --- a/edb/pgsql/compiler/relctx.py +++ b/edb/pgsql/compiler/relctx.py @@ -1804,7 +1804,23 @@ def _get_typeref_descendants( include_descendants and not for_mutation ): - descendants = [typeref, *irtyputils.get_typeref_descendants(typeref)] + 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 = [ diff --git a/edb/pgsql/delta.py b/edb/pgsql/delta.py index 551648153a1..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( @@ -2000,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: @@ -3089,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) @@ -3214,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]) @@ -3342,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) ) ] @@ -3381,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)) @@ -3460,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) @@ -3490,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( @@ -3944,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 @@ -5837,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 @@ -6241,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): @@ -6798,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 @@ -6847,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 = [] @@ -6876,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/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: From 08f6fddb4154da5141965c9ea4f9448e3259394d Mon Sep 17 00:00:00 2001 From: dnwpark Date: Thu, 11 Jul 2024 16:43:04 -0400 Subject: [PATCH 26/26] Fix CTEs not being added to non-set expr statements after compile. --- edb/pgsql/compiler/__init__.py | 12 +++++++++--- edb/pgsql/compiler/clauses.py | 18 ++++++++++++------ 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/edb/pgsql/compiler/__init__.py b/edb/pgsql/compiler/__init__.py index 30ef1f09cfd..a78f5def14e 100644 --- a/edb/pgsql/compiler/__init__.py +++ b/edb/pgsql/compiler/__init__.py @@ -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 985d980e877..f8528b525ea 100644 --- a/edb/pgsql/compiler/clauses.py +++ b/edb/pgsql/compiler/clauses.py @@ -409,12 +409,9 @@ 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] = [ @@ -423,6 +420,15 @@ def fini_toplevel( *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