Support for tables up to 256 columns

[LucaCappelletti94]

Hello,

I am dealing with some legacy databases which contain extremely large tables.

While I am perfectly in agreement that it is a very poor life choice to make such large tables in the first place, I am in the unenviable position to have to design the migration of data from the legacy database into the new one.

I would like to use diesel to define the migration, and therefore I would like to introduce support for tables up to 256 columns. I took a glance at the points where the 128-column-tables feature is mentioned, and I don't seem to identify any hard limit to the number of columns, at least, not on the diesel side of things, except for perhaps longer compile time requirements.

If there are no particular reason for not having a 256-column-tables optional feature, except for "people should know better than do such large tables", I would proceed to write the pull request to add support for the feature.

Thank you for your time and have a great day,
Luca

[weiznich]

This topic comes up from time to time. The main reasons why we haven't done this yet is that we expect that compile times are really bad for such a configuration. As you already opened #4518 you could maybe comment on what compile times you observe with the 128-column-tables feature and the new 256-column-tables feature. That could help us to make a better decision there.

The other thing that makes seems not to be great for me is that this introduces another feature flag that needs to be tested and ultimately there is still a limit there. That made me wonder if there would be a better solution to the underlying problem of configuring a maximal column number for diesel. Given that the code is now generated by a proc-macro we could read that value from the environment somehow and adjust the limit based on that value. Implementing that would require:

• Adding a build script to diesel-derives that
  • checks if CARGO_CFG_DIESEL_MAX_COLUMN_NUMBER is set
  • emit a cargo::rerun-if-env-changed=CARGO_CFG_DIESEL_MAX_COLUMN_NUMBER output for cargo to inform cargo to re execute this build script if that environment variable changed.
  • If it's set write the value as constant to a file in the OUT directory passed as build configuration to the build script
  • If it's not set fall back to the value configured by the feature flags and write that tho the constant in the output file
• In diesel_derives replace this block by using the constant written to the OUT directory
• Add documentation to the lib.rs file of diesel to inform users how to use this feature (by setting RUSTFLAGS='--cfg=diesel_max_column_number="128" in their environment) and calling out that larger numbers will result in really large compile times.

This approach has the obvious advantage that you can increase the limit only as much as needed instead of going to some fixed limit that is too large. The slight disadvantage is that it is a bit harder to configure that just enabling a feature flag. Given that we want to discourage using too large numbers there for compile time reasons and also as it is just a not great database design that seems to be a reasonable trade off for me.

I'm happy to accept a PR for that.

[LucaCappelletti94]

Seems reasonable - I will try to proceed how you describe.

I am unsure regarding how to benchmark the time requirements, as the unfortunate database I mentioned only has a very small number of extra large tables. I really think any situation were such large number of columns are needed is a pathological case, so there isn't really a strong incentive to make it compile faster.

Diesel there is primarily needed to enforce data correctness, not necessarily efficiency.

[weiznich]

I am unsure regarding how to benchmark the time requirements, as the unfortunate database I mentioned only has a very small number of extra large tables. I really think any situation were such large number of columns are needed is a pathological case, so there isn't really a strong incentive to make it compile faster.

I'm mostly interested in the following times here:

• cargo check -p diesel -F 256-column-tables vs cargo check -p diesel -F 128-column-tables
• cargo check -p your-crate -F diesel/256-column-tables vs cargo check -p your-crate -F 256-column-tables

I expect both times to be larger for the 256 column variant, but I would like to get a feeling for how much they are larger. That's not a hard blocker, just something that might be interesting to know and that we should mention as relative factor in the documentation (i.e. compiling takes x times more time if you double the number of columns from 128 to 256)

[LucaCappelletti94]

Here are the time requirements of running cargo check -p diesel -F X-column-tables, where X is the number of columns:

32:

real 0m4.588s
user 0m3.586s
sys 0m0.917s

64:

real 0m20.584s
user 0m19.701s
sys 0m1.837s

128:

real 1m36.756s
user 1m31.994s
sys 0m5.669s

256:

real 11m12.366s
user 10m24.963s
sys 0m47.064s

[weiznich]

Thanks for performing this measurements. Looks a bit more than quadratic for me.

[LucaCappelletti94]

Yes, the compile time are certainly somewhat unfortunate. Your proposal of switching to a user-defined constant would allow for increasing the compile time strictly of the required amounts, which seems quite sensible to me.