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

infer_schema! reaches the recursion limit easily #1127

Closed
1 task done
Kazakuri opened this issue Aug 25, 2017 · 11 comments
Closed
1 task done

infer_schema! reaches the recursion limit easily #1127

Kazakuri opened this issue Aug 25, 2017 · 11 comments

Comments

@Kazakuri
Copy link

Setup

Versions

  • Rust: rustc 1.21.0-nightly (2bb8fca18 2017-08-23)
  • Diesel: 0.16.0

Problem Description

What are you trying to accomplish?

Having diesel automatically infer the schema for a table with more than ~10 columns.

It looks like the default recursion limit for macros is 64 which can be quite easy to reach with 6 lines of code being generated for each database column.

What is the expected output?

Diesel compiles with no issues.

What is the actual output?

error: recursion limit reached while expanding the macro `table_body`
  --> src\database\schema.rs:1:1
   |
1  | / table! {
2  | |     /// Representation of the `users` table.
3  | |     ///
4  | |     /// (Automatically generated by Diesel.)
...  |
84 | |     }
85 | | }
   | |_^
   |
   = help: consider adding a `#![recursion_limit="128"]` attribute to your crate
   = note: this error originates in a macro outside of the current crate

Steps to reproduce

You can use pretty much any schema, just pad it with a bunch of extra blank documentation comments.

table! {
    /// Representation of the `users` table.
    ///
    /// (Automatically generated by Diesel.)
    users (id) {
        /// The `id` column of the `users` table.
        ///
        /// Its SQL type is `Int4`.
        ///
        /// (Automatically generated by Diesel.)
        id -> Int4,
    }
    ///
    /// etc...
    ///
}

Checklist

  • I have already looked over the issue tracker for similar issues.
@killercup
Copy link
Member

That is sadly a problem we can't easily solve in Diesel. For now, just follow the compiler's advice: Add the #![recursion_limit="128"] attribute.

Personally, I set this to 1024 at the first error just to not have to deal with this again. Also, IIRC, the default (64 apparently) was recently increased in rustc, so maybe you'll see this error less often in the future.

That said, we could solve this in Diesel: We'd need to replace the macro with a procedural macro. That is not an easy feat (the macro is quite complex and has a lot of edge cases), and not currently our focus. We'll absolutely look more deeply into this once the next batch of proc-macro features becomes stable in rustc, though.

@killercup
Copy link
Member

Oh, or do you have a quick fix for this in mind? I've not looked into it more deeply, but if there was an easy change we could make to consume more doc attribute tokens per macro call we'd reduce the table macro's call stack drastically. (Still, not something I really want to spend much time on—there are more pressing matters—but I'd accept a PR for this for sure!)

@mattdeboard
Copy link

mattdeboard commented Oct 21, 2017

Hi, so the trouble I'm having is that any table with more than 24 columns (these are extant tables) throws this error. I can run diesel print-schema fine. It outputs the right schema. So since apparently !infer_schema("dotenv:DATABASE_URL", "foo") is equivalent to diesel print-schema -s foo I'm surprised this isn't working correctly.

I tried doing diesel print-schema -s foo > src/models.rs but even then, when I do cargo run, the compiler complains about the recursion limit. Can you explain what the problem is exactly?

edit: Ok apparently the answer is this is a limitation of Rust itself. Here is an explanation with pointers to more detailed explanations.

@Kazakuri
Copy link
Author

So basically my understanding of it is that the recursion limit is hit when expanding the table! macro that is generated from the diesel print-schema/!infer_schema("dotenv:DATABASE_URL", "foo") commands.

Like killercup said, the way the table! macro expands is the older(?) recursive style macros instead of a procedural style, so that every line inside the macro body is a recursively parsed.

So unless the macro gets re-written which has a lot of problems in itself, the only real solution is increasing the recursion limit of your program.

@mattdeboard
Copy link

mattdeboard commented Oct 21, 2017

@Kazakuri Ya I have added that to the top of my main.rs but it's still happening. Any idea?

Also I added the huge-tables feature to diesel, but STILL getting the error. There are less than 52 columns on the table :-\

@ConnorBP
Copy link

This issue is from 2 years ago... I guess nobody got around to it because this still happens if you want to ever use docs on the schema.

@weiznich
Copy link
Member

@ConnorBP
a) infer_schema! is deprecated and not supported any more.
b) This could easily fixed by increasing the recursion limit as the compiler suggests.

@ConnorBP
Copy link

@ConnorBP
a) infer_schema! is deprecated and not supported any more.
b) This could easily fixed by increasing the recursion limit as the compiler suggests.

If infer_schema is deprecated then why is diesel itself using it? I never used infer_schema but the auto generated schema.rs gives me an error about this if I let the generator add comments. Maybe this should be a separate issue about removing the deprecated calls to this function from the schema.rs generator but either way this ancient issue still exists at the moment. Yes for now I can force the compiler to add more recursion, but that will make building significantly slower than before. I can also disable comments generation, which I did, but that is also not ideal and is a rather clunky workaround for a fundamentally flawed system that has existed for years on ancient rust features and deprecated diesel features.

@weiznich
Copy link
Member

weiznich commented May 10, 2019

If infer_schema is deprecated then why is diesel itself using it? I never used infer_schema but the auto generated schema.rs gives me an error about this if I let the generator add comments. Maybe this should be a separate issue about removing the deprecated calls to this function from the schema.rs generator but either way this ancient issue still exists at the moment.

We are talking about different things. infer_schema!, the macro is deprecated. diesel print-schema which uses the same code internally is not deprecated.

Yes for now I can force the compiler to add more recursion, but that will make building significantly slower than before. I can also disable comments generation, which I did, but that is also not ideal and is a rather clunky workaround for a fundamentally flawed system that has existed for years on ancient rust features and deprecated diesel features.

As far as I see there is no real way forward to fix that. We want a macro that is able to handle all cases and provide a consistent API. This makes the macro quite complex, which requires adding those recursion limit attribute. One option here is to make that macro simpler by removing options which would a) break all/some existing code, b) remove some use cases. Another option would be to rewrite that as procedural macro, which requires a huge amount of work without any direct gain. Given that no one has currently much time to work on more urging issues/features I don't think that will happen anytime soon. (Also such a rewrite needs to prove that all existing valid code works, which is no simple task)

@ConnorBP
Copy link

We are talking about different things. infer_schema!, the macro is deprecated. diesel print-schema which uses the same code internally is not deprecated.

I am just curious why infer_schema! is for some reason causing errors from the comments in the auto generated file then if it is as you say deprecated and I am not personally using it. My best guess is that the underlying library is itself using this "deprecated" macro. If it is deprecated a better / newer alternative surely exists already and shouldn't it be not using the deprecated macro? Or is there simply no alternative and therefor it shouldn't be "deprecated".

@weiznich
Copy link
Member

I am just curious why infer_schema! is for some reason causing errors from the comments in the auto generated file then if it is as you say deprecated and I am not personally using it.

This issue is about infer_schema! not about diesel print-schema 😉 (So if you are talking about diesel print-schema your issue probably does not belong here 😉. As written below, I think it's neither an issue with infer_schema! nor with diesel print-schema but with the table! macro. (To be discussed if this is an real issue or only some minor improvement…))

My best guess is that the underlying library is itself using this "deprecated" macro. If it is deprecated a better / newer alternative surely exists already and shouldn't it be not using the deprecated macro? Or is there simply no alternative and therefor it shouldn't be "deprecated".

Both, infer_schema! and diesel print-schema used the same code internally to generate the corresponding table calls. (Also the problem is neither in infer_schema! nor in diesel print-schema. It's simply that the table! macro is to complex. The same thing could happen while manually writing a lot of table! calls.)

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

No branches or pull requests

5 participants