edgedb / rfcs

RFCs for major changes to EdgeDB
Apache License 2.0
35 stars 5 forks source link

RFC 1010: Global variables #49

Closed elprans closed 2 years ago

elprans commented 2 years ago
  1. How it will help with the ACL? Some example queries, please (could be a separate "discussion", but important to get this spec right)

I'll post a separate RFC with examples soon.

  1. The (3) above assumes that (GLOBAL x) yields empty set if not set. But does it? Or is it an error to execute such query?

Globals behave exactly like any other expression, including cardinality inference. If you want your queries to fail if a global is unset, then you must declare it as required and either use a default, a known non-empty constant expression, or assert_exists.

  1. How Python/JS API will look like? Does it mean you have to acquire a connection?

Something like:

conn = conn.with_globals(foo=bar)

The values of globals will be sent encoded in a header together with Execute or OptimisticExecute.

  1. How much impact it will have on performance? Given that right abstraction will probably have to set session variables at the start of incoming request processing even if these globals are not used by specific query.

Globals will compile into SQL query arguments, and the server will know when and how to pass the extra arguments. There should be no noticeable impact on perf other than the need to encode/decode extra data in "globals" message headers, but those are also easily cacheable.

  1. Is there a way to introspect globals? (Probably DESCRIBE GLOBAL ?)

Like every other schema object you can introspect globals via the introspection schema or get its text definition via DESCRIBE.

  1. What are interactions between globals and migrations? Off the top of my head: what happens if required is set on a property that has (GLOBAL xx) in the default? Can I use/set globals in data migrations?

Globals obey the same cardinality inference rules as other expressions. So, if you change an enclosing DDL object in a way that is incompatible with the expression containing a reference to a global, it's a schema error.

tailhook commented 2 years ago

19. What are interactions between globals and migrations? Off the top of my head: what happens if required is set on a property that has (GLOBAL xx) in the default? Can I use/set globals in data migrations?

Globals obey the same cardinality inference rules as other expressions. So, if you change an enclosing DDL object in a way that is incompatible with the expression containing a reference to a global, it's a schema error.

I mean more complicated scenarios. Like:

ALTER TYPE User {
    ALTER PROPERTY name {
        SET default := GLOBAL X;
    }
};
ALTER TYPE User {
    ALTER PROPERTY name {
        SET REQUIRED;
    }
};

Does this populate property with value for all objects when migration is applied?

If above is no-op. What about this scenario:

CREATE ALIAS Name := (SELECT GLOBAL default_name);
ALTER TYPE User {
    ALTER PROPERTY name {
        SET REQUIRED USING (SELECT Name); // or whatever syntax we have to populate required?
    }
};

Does this work? If user types (SELECT Name) in shell when doing migration, do we have a mechanism that will ask them to type GLOBAL default_name or is it an error?

Although if we "restricting globals to explicit queries and rewrite policy initially", this should not be a problem.

elprans commented 2 years ago

Does this populate property with value for all objects when migration is applied?

When a pointer is made required you must provide a fill expression that has lower cardinality bound of non-zero, which means that either 1) your GLOBAL is declared as required; or 2) you use assert_exists. Any query that depends on a required GLOBAL that is unset is automatically an error.

do we have a mechanism that will ask them to type GLOBAL default_name or is it an error?

It's an error to reference an unset REQUIRED GLOBAL. The error message can have a suggestion to SET GLOBAL in its hint and the CLI may even handle that and present a prompt.

tailhook commented 2 years ago

do we have a mechanism that will ask them to type GLOBAL default_name or is it an error?

It's an error to reference an unset REQUIRED GLOBAL. The error message can have a suggestion to SET GLOBAL in its hint and the CLI may even handle that and present a prompt.

I'm confused. What is an unset global? My reading of this RFC was that global always have either default value or equals to an empty set.

Can I make a REQUIRED global, and only set it when queries that refer to that global are used?

If if by unset you mean "= empty set" then the problem is that assert_single(GLOBAL user_id) will only trigger error on migration commit. So suggesting to SET GLOBAL at this point is unfortunate.

elprans commented 2 years ago

I'm confused. What is an unset global? My reading of this RFC was that global always have either default value or equals to an empty set.

Ah yes, I see the inconsistency. Default shouldn't be mandatory for required globals. Instead, a required global essentially inserts an implicit assert_exists around itself when referenced.

Can I make a REQUIRED global, and only set it when queries that refer to that global are used?

Yes.

If if by unset you mean "= empty set" then the problem is that assert_single(GLOBAL user_id) will only trigger error on migration commit. So suggesting to SET GLOBAL at this point is unfortunate.

Presumably you'd set it with set global in the migration script, otherwise it's a broken migration. Regardless, I think we'll restrict where globals can be used for now and worry about these details later.

msullivan commented 2 years ago

I'm going to merge this now, and then probably do some updates as I work on things