Query params in config commands #5670
Conversation
| # ConfigSet and ConfigReset don't like being part of a Set, so bail early | ||
| if isinstance(ir.expr, (irast.ConfigSet, irast.ConfigReset)): | ||
| ir.expr.scope_tree = ctx.path_scope | ||
| ir.expr.globals = list(ctx.env.query_globals.values()) | ||
| ir.expr.params = params | ||
| ir.expr.schema = ctx.env.schema |
There was a problem hiding this comment.
@msullivan from the top of your head, is there an obvious blocker that prevents ConfigCommand be a part of Statement?
If you don't think of one, I'll try to fit it in (in a following PR)
There was a problem hiding this comment.
The top-level thing is that Statement wants expr to be a Set and apparently "ConfigSet and ConfigReset don't like being part of a Set, so bail early".
I think I skipped investigating that the last time I hit this because I didn't (and, honestly, don't) understand the backend config compiler and didn't want to go down a rabbit hole, but I think it should be possible
| @@ -404,6 +404,20 @@ async def test_edgeql_globals_12(self): | |||
| [16] | |||
| ) | |||
|
|
|||
| async def test_edgeql_globals_13(self): | |||
There was a problem hiding this comment.
Could we add a test with config commands too? I seem to recall we might try to use the static evaluator on those so it's worth seeing what happens
There was a problem hiding this comment.
I'm not sure if this is possible. The non-global "config set" commands we have are:
set module mod;set alias al as module mod;
https://www.edgedb.com/docs/reference/edgeql/sess_set_alias#set
There was a problem hiding this comment.
| @@ -65,7 +65,7 @@ def compile_ir_to_sql_tree( | |||
| ] | |||
| ] = None, | |||
| backend_runtime_params: Optional[pgparams.BackendRuntimeParams]=None, | |||
| ) -> Tuple[pgast.Base, context.Environment]: | |||
There was a problem hiding this comment.
Maybe these functions ought to return a namedtuple or a dataclass. Doesn't need to happen now though.
Followup for #5670 - replace a few tuples in return types with dataclasses, - create "complexity bottle necks" of invocations of pgcompiler and pgcodegen - with this I mean that I tried to reduce the number of entry points to specific parts of the codebase, - I've also converted a few classmethods into regular functions, because they were doing some `super().__class__(**kwargs)` that I find ugly, - move entrypoints and "important stuff" to the top of a file and auxilary functions and classes down (important stuff is generally anything public, used beyond current file).
Followup for #5670 - replace a few tuples in return types with dataclasses, - create "complexity bottle necks" of invocations of pgcompiler and pgcodegen - with this I mean that I tried to reduce the number of entry points to specific parts of the codebase, - I've also converted a few classmethods into regular functions, because they were doing some `super().__class__(**kwargs)` that I find ugly, - move entrypoints and "important stuff" to the top of a file and auxilary functions and classes down (important stuff is generally anything public, used beyond current file).
Followup for #5670 - replace a few tuples in return types with dataclasses, - create "complexity bottle necks" of invocations of pgcompiler and pgcodegen - with this I mean that I tried to reduce the number of entry points to specific parts of the codebase, - I've also converted a few classmethods into regular functions, because they were doing some `super().__class__(**kwargs)` that I find ugly, - move entrypoints and "important stuff" to the top of a file and auxilary functions and classes down (important stuff is generally anything public, used beyond current file).
Closes #4934