Deriving PartialEq, PartialOrd, Eq, Ord and Hash for table and column markers

[LucaCappelletti94]

Backend

PostgreSQL, MySQL, SQLite, Others

What do you want to add to diesel?

When working with structs which have one or more such markers, with some fields of the structs being from core like tuples, not having the ability to derive PartialEq et similia adds a lot to boiler plating to the code.

Would it be acceptable to add such derives to the table!-generated marker structs?

Implementation notes

No response

Additional details

No response

Checklist

• I have already looked over the issue tracker and the discussion forum for similar possible closed issues.

[weiznich]

Thanks for filling this proposal. Given that all these derives add additional code to what's already generated by the table! macro I would be interested in having at least some estimate on how this affects the number of code lines generated by a full expansion of the table! macro and how this impacts compilation times of down stream crates.

[LucaCappelletti94]

Comparing the main and extended-derives respectively, adding a column generates 198 and 242 lines of code respectively, an increase of 45 lines per column. Adding a table, subtracted the effect of the columns, adds 535 and 622 lines of code respectively, an increase of 88 lines per table.

The number of lines above are determined in the following way: I determined using cargo expand the size of the documents with 1 table and 1 column, 1 table and 2 columns, 2 tables with one column each, and them executed some trivial subtractions as follows:

fn main() {
    // Number of lines with no adjustements
    let extended_one_column = 871;
    let extended_two_columns = 1113;
    let extended_two_tables = 1736; // Tables with 1 column
    let original_one_column = 740;
    let original_two_columns = 937;
    let original_two_tables = 1472; // Tables with 1 column
    
    // Footprints
    let original_column_fp = original_two_columns - original_one_column;
    let extended_column_fp = extended_two_columns - extended_one_column;
    let original_table_fp = original_two_tables - original_one_column - original_column_fp;
    let extended_table_fp = extended_two_tables - extended_one_column - extended_column_fp;
    
    println!("original_column_fp: {original_column_fp}");
    println!("extended_column_fp: {extended_column_fp}, increased by {} lines", extended_column_fp - original_column_fp);
    println!("original_table_fp: {original_table_fp}");
    println!("extended_table_fp: {extended_table_fp}, increase by {} lines", extended_table_fp - original_table_fp);
}

Which outputs:

original_column_fp: 197
extended_column_fp: 242, increased by 45 lines
original_table_fp: 535
extended_table_fp: 623, increase by 88 lines

Regarding compile time, I wrote a short python script to rerun the cargo clean -p diesel-derive-strategies && cargo build --feature {diesel_branch} on a lib such as the following one:

#[cfg(feature = "diesel_main")]
use diesel_main as diesel;
#[cfg(feature = "diesel_extended")]
use diesel_extended as diesel;

diesel::table! {
    animals (id) {
        id -> Integer,
        name -> Text,
    }
}

diesel::table! {
    dogs (id) {
        id -> Integer,
        breed -> Text,
    }
}

Averaging out the compile time of 100 runs for either diesel branches and paying attention to avoiding having the compiler optimize stuff away, I got 0.1646s ± 0.0028s for the main and 0.1713s ± 0.0030s for the extended. Executing a Wilcoxon signed-rank test (although the tests order is reasonably independent), we get a p-value of 4.47e-17 suggesting that the difference is statistically significant, with the main branch of course being faster. Still, the difference is ~4%. For reference, compiling diesel in the same context takes about 8.4 seconds.

[LucaCappelletti94]

Additionally, another good reason to facilitate the use of these derives is that for any manual non-derived implementation of these traits which is made necessary due to the absence of these derives, one also need to test the generated code much more extensively then what would be otherwise possible with the simpler derives. Manual code, I think, is much more error-prone than something generated from a core derive like PartialEq.

[weiznich]

Thanks for providing these details. I did another measurement with a larger schema.rs file (60 tables) and the results are as following:

hyperfine -p 'cargo check --manifest-path {dir}/Cargo.toml && cargo clean --manifest-path {dir}/Cargo.toml -p diesel_testing' -L dir /tmp/diesel_testing_release,/tmp/diesel_testing_patch,/tmp/diesel_testing_main 'cargo check --manifest-path {dir}/Cargo.toml'
Benchmark 1: cargo check --manifest-path /tmp/diesel_testing_release/Cargo.toml
  Time (mean ± σ):      4.322 s ±  0.025 s    [User: 4.107 s, System: 0.195 s]
  Range (min … max):    4.281 s …  4.354 s    10 runs
 
Benchmark 2: cargo check --manifest-path /tmp/diesel_testing_patch/Cargo.toml
  Time (mean ± σ):      3.208 s ±  0.062 s    [User: 3.009 s, System: 0.183 s]
  Range (min … max):    3.129 s …  3.310 s    10 runs
 
Benchmark 3: cargo check --manifest-path /tmp/diesel_testing_main/Cargo.toml
  Time (mean ± σ):      3.085 s ±  0.056 s    [User: 2.896 s, System: 0.175 s]
  Range (min … max):    2.983 s …  3.184 s    10 runs

testing_release is using diesel 2.3.1 from crates.io, testing_patch is using your branch and testing_main is using the main branch. All benchmarks were performed with the postgres backend enabled.

So it become much faster on main, and the patch made it slightly slower again. I think that's acceptable, although we still might want to look for ways to speedup the compilation time further by optimize the output more.

[LucaCappelletti94]

In practice, for my own use cases, I only concretely need the derives on the table markers. I have added them also on the column markers solely for completeness. Maybe for the time being I may remove the derive from the column markers to reduce the amount of code being generated?

I expect that the number of columns is much larger than the number of tables in any given schema.

[LucaCappelletti94]

Closing as resolved by PR #4902