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

Consider adding error expression type for better error cascading #1947

Open
christopherswenson opened this issue Oct 3, 2024 · 1 comment
Labels
L3 Complex multi-file change requiring discussion language An issue in the Malloy language

Comments

@christopherswenson
Copy link
Contributor

If you have:

aggregate: 
  x is count()
  y is x * 2

You get two errors:

  • "x is not defined"
  • "cannot use a scalar value in an aggregate"

The first error is correct. The second error is confusing, and happens because the result of looking up x defaults to a scalar expression type, which then fails the expression type check of aggregate. We could just not emit that error if the data type is error, but that would remove some helpful instances of the error message, e.g. with aggregate: y is scalar_sql_native_typed_thing + 1 (where the data type is error but the expression type is known to be scalar).

In general, we are also not super careful of setting the expressionType correctly when we use errorFor or loggedErrorExpr; in many cases we know something to be an aggregate, but we generate an error/scalar when there are issues.

@mtoy-googly-moogly
Copy link
Collaborator

Interesting currently error is a data type and that bothers me because that ends up in the ir

i'd be happy if switching it to an expression type was helpful because those don't make it into the ir

@christopherswenson christopherswenson added language An issue in the Malloy language L3 Complex multi-file change requiring discussion labels Dec 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
L3 Complex multi-file change requiring discussion language An issue in the Malloy language
Projects
None yet
Development

No branches or pull requests

2 participants