Skip to content

Conversation

james-johnston-thumbtack

resolves #1000

Problem

There is no way to compare two models using the equality test when the columns containing the same data have different column names. For example, if the first table has column biz_name and the second table has business_name, there is no way to compare them even if the data is the same.

Solution

The compare_columns now optionally accepts a list of two column names. This allows the column names to be different between base model and other model when comparing two columns.

The body of the macro is also refactored to make it more DRY, and reduce convoluting the different steps and capabilities.

  1. First, we gather a set of numeric column names that we need to round. This is referenced later.

  2. Next, we build two lists of column names that we want to compare: one list per model. If compare_columns was given, we build the lists from that. Otherwise, read column names from the first model, filter the excluded column names, and use the result for both models, as we did before this commit.

  3. Now that we have lists of column names, we can build comma-separated lists for each model. At this point, we do the number rounding expression if the column name is found in the set from step 1.

Note this refactoring also cleans up some weird inconsistencies that resulted from duplication of logic. For example, suppose a model's column name was uppercase, and your database is case-sensitive. If you did not specify a "precision" argument, then you need to provide an upper-case "compare_columns" argument. However, if you did specify a "precision" argument, then the "compare-columns" argument was NOT case-sensitive. It's pretty unexpected to have the case sensitivity of one argument be dependent on a seemingly-unrelated argument.

Checklist

The compare_columns now optionally accepts a list of two column names.
This allows the column names to be different between base model and
other model when comparing two columns.

The body of the macro is also refactored to make it more DRY, and reduce
convoluting the different steps and capabilities.

1.  First, we gather a set of numeric column names that we need to
    round.  This is referenced later.

2.  Next, we build two lists of column names that we want to compare:
    one list per model.  If compare_columns was given, we build the
    lists from that.  Otherwise, read column names from the first model,
    filter the excluded column names, and use the result for both
    models, as we did before this commit.

3.  Now that we have lists of column names, we can build comma-separated
    lists for each model.  At this point, we do the number rounding
    expression if the column name is found in the set from step 1.

Note this refactoring also cleans up some weird inconsistencies that
resulted from duplication of logic.  For example, suppose a model's
column name was uppercase, and your database is case-sensitive.  If you
did not specify a "precision" argument, then you need to provide an
upper-case "compare_columns" argument.  However, if you _did_ specify a
"precision" argument, then the "compare-columns" argument was _NOT_
case-sensitive.  It's pretty unexpected to have the case sensitivity of
one argument be dependent on a seemingly-unrelated argument.
@james-johnston-thumbtack james-johnston-thumbtack force-pushed the james/equality-compare-columns-alternate-names branch from cdec9f2 to 0d60f9a Compare March 25, 2025 09:03
@james-johnston-thumbtack james-johnston-thumbtack marked this pull request as ready for review April 13, 2025 04:31
@james-johnston-thumbtack james-johnston-thumbtack requested a review from a team as a code owner April 13, 2025 04:31
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

Successfully merging this pull request may close these issues.

Allow setting different compare_columns for each table/model in equality generic test

1 participant