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

infer_schema! generates error: no rules expected the token `)` #754

Closed
pwestrich opened this Issue Feb 23, 2017 · 13 comments

Comments

Projects
None yet
2 participants
@pwestrich

pwestrich commented Feb 23, 2017

I'm working on integrating Diesel into one of my projects at work instead of using the MySQL drivers directly. As soon as I add the line infer_schema!("dotenv:DATABASE_URL"); anywhere in my project, I get the error mentioned in the title: error: no rules expected the token ')'. I can't for the life of me figure out what it means or what causes it.

My .env file exists and is fine, as the diesel command line tool has no objections with it. I'm importing diesel and diesel_codegen in the correct order in my lib.rs, as I've seen that sometimes causes problems. I have also tried varying import statements, though none are required in any of the examples.

I have tried building the examples, and they work fine. That leads me to believe it's a problem in my project, however, I have no idea where to start looking, as this error is pretty cryptic.

Rust version: rustc 1.15.1 (021bd294c 2017-02-08)
Diesel version: 0.11.4
Diesel features: mysql

@sgrif

This comment has been minimized.

Member

sgrif commented Feb 24, 2017

Can you run cargo rustc -- -Ztrace-macros and paste the last few lines before the error?

@pwestrich

This comment has been minimized.

pwestrich commented Feb 24, 2017

I get the exact same error message:

$cargo rustc --bin main -- -Ztrace-macros
   Compiling project v0.1.0 (file:///Users/pwestrich/Documents/Repositories/project)
error: no rules expected the token `)`
 --> src/schema.rs:2:1
  |
2 | infer_schema!("dotenv:DATABASE_URL");
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |
  = note: this error originates in a macro outside of the current crate

error: Could not compile `project`.

To learn more, run the command again with --verbose.
@sgrif

This comment has been minimized.

Member

sgrif commented Feb 24, 2017

What about without --bin main? Alternatively, can you replace the infer_schema! call with the output of diesel print-schema?

@pwestrich

This comment has been minimized.

pwestrich commented Feb 24, 2017

Without that flag, cargo gets angry at me, because I have both a library and an executable in the same crate.

When I replace infer_schema!("dotenv:DATABASE_URL"); with the output from diesel print-schema, I get the same error message, except this time, it's the table! macro that causes it.

Now if I run it with the --lib flag (something I hadn't thought of until now), I get a good 6k lines of output from cargo here.

@sgrif

This comment has been minimized.

Member

sgrif commented Feb 24, 2017

Hm. Well I'm not sure why the error message you're seeing complains about ')', but the issue is that we don't yet support unsigned types. If you change the type of Student.RiskFactor to be a plain float (or use the output of diesel print-schema instead of infer_schema! and manually remove the unsigned), does that resolve your problem? (Also go home MySQL, you're drunk. Unsigned floats are not a thing.)

@pwestrich

This comment has been minimized.

pwestrich commented Feb 24, 2017

How in the world did I get an unsigned float? That's certainly not a thing that exists.

I'll go check my schema and make sure I don't have any imaginary types floating around.

@sgrif

This comment has been minimized.

Member

sgrif commented Feb 24, 2017

If you specify ZEROFILL for a numeric column, MySQL automatically adds the UNSIGNED attribute to the column.

Maybe that?

@sgrif

This comment has been minimized.

Member

sgrif commented Feb 24, 2017

If nothing else we should be giving a specific and helpful error message here.

@pwestrich

This comment has been minimized.

pwestrich commented Feb 24, 2017

That would certainly be nice.

Removing the unsigned attributes, I'm now getting a new set of error messages. I'm guessing form them that you don't support the char type either? Is there a list I can reference somewhere?

@sgrif

This comment has been minimized.

Member

sgrif commented Feb 24, 2017

http://docs.diesel.rs/diesel/types/index.html is the list of the supported types across all backends. You can also see the types Mysql specifically supports so far by looking at the list of HasSqlType impls here. We can probably just add type Char = VarChar, they should be effectively the same from Diesel's point of view.

@sgrif

This comment has been minimized.

Member

sgrif commented Feb 24, 2017

If you could actually give a list of the types that you need which aren't supported, that would be extremely useful information for feature prioritization

sgrif added a commit that referenced this issue Feb 24, 2017

Give a more specific error message for unsigned types
We don't yet support these types in MySQL, but the error a user will
recieve is not terribly helpful, since the type will contain a space so
the `table!` macro invocation will just have bad tokens.

This gives a more specific error message for unsigned types, as well as
ensuring that we get a reasonable error for other types would result in
a bad macro invocation. (I'm not actually aware of any other types which
have a space, but I'd rather not dig through the entire manual right
now)

In theory we could exclude unsigned floats from this, since unsigned
floats aren't actually a thing and that "type" in MySQL is basically
just a check constraint, but I'd rather hold off and actually represent
in Diesel that the type is different when we add support for unsigned
integer types.

Fixes #754.
@pwestrich

This comment has been minimized.

pwestrich commented Feb 24, 2017

I'm not using any other crazy types, so I don't think I'll need any added, though I'm sure it would be nice in the future to support all the types the respective databases do at some point.

I'll just make my chars into varchars for the time being.

@sgrif

This comment has been minimized.

Member

sgrif commented Feb 24, 2017

I'm sure it would be nice in the future to support all the types the respective databases do at some point.

That's roughly the plan (though types that have more involved capabilities such as geometry types will likely live in third party crates)

sgrif added a commit that referenced this issue Feb 24, 2017

Give a more specific error message for unsigned types
We don't yet support these types in MySQL, but the error a user will
recieve is not terribly helpful, since the type will contain a space so
the `table!` macro invocation will just have bad tokens.

This gives a more specific error message for unsigned types, as well as
ensuring that we get a reasonable error for other types would result in
a bad macro invocation. (I'm not actually aware of any other types which
have a space, but I'd rather not dig through the entire manual right
now)

In theory we could exclude unsigned floats from this, since unsigned
floats aren't actually a thing and that "type" in MySQL is basically
just a check constraint, but I'd rather hold off and actually represent
in Diesel that the type is different when we add support for unsigned
integer types.

Fixes #754.

sgrif added a commit that referenced this issue Feb 24, 2017

Give a more specific error message for unsigned types
We don't yet support these types in MySQL, but the error a user will
recieve is not terribly helpful, since the type will contain a space so
the `table!` macro invocation will just have bad tokens.

This gives a more specific error message for unsigned types, as well as
ensuring that we get a reasonable error for other types would result in
a bad macro invocation. (I'm not actually aware of any other types which
have a space, but I'd rather not dig through the entire manual right
now)

In theory we could exclude unsigned floats from this, since unsigned
floats aren't actually a thing and that "type" in MySQL is basically
just a check constraint, but I'd rather hold off and actually represent
in Diesel that the type is different when we add support for unsigned
integer types.

Fixes #754.

@sgrif sgrif closed this in b4266be Feb 25, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment