Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Raise error when computed global is set or reset. #7374

Merged
merged 2 commits into from
May 24, 2024
Merged

Conversation

dnwpark
Copy link
Contributor

@dnwpark dnwpark commented May 21, 2024

After set global foo := false;, attempting to run set global lol := false; produces the following error:

error: ConfigurationError: global 'foo' is computed from an expression and cannot be modified
  ┌─ <query>:1:12
  │
1 │ set global foo := false;
  │            ^^^ error

close #7368

@dnwpark dnwpark marked this pull request as draft May 21, 2024 22:08
if isinstance(expr, (qlast.ConfigSet, qlast.ConfigReset)):
if glob.get_expr(ctx.env.schema):
raise errors.ConfigurationError(
f"global '{glob_name}' is computed from an expression and "
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd probably just say "computed global 'whatever' may not be modified" or some such

Copy link
Member

@msullivan msullivan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems good for set global. We want to also check what happens when the client specifies the values via the protocol

@dnwpark
Copy link
Contributor Author

dnwpark commented May 22, 2024

@msullivan I saw the issue mentioned that. With these changes, it seems that setting the global silently deletes it from the state.

@msullivan
Copy link
Member

@msullivan I saw the issue mentioned that. With these changes, it seems that setting the global silently deletes it from the state.

I'm not sure what you mean by that

@dnwpark dnwpark marked this pull request as ready for review May 24, 2024 16:30
@dnwpark dnwpark merged commit 37317ea into master May 24, 2024
23 checks passed
@dnwpark dnwpark deleted the set-computed-global branch May 24, 2024 22:33
@dnwpark
Copy link
Contributor Author

dnwpark commented May 24, 2024

Also added a test to the python client here: edgedb/edgedb-python#494

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Don't allow setting a global if it was declared with a computable expression
2 participants