chore(tests): Add tests to the column denormalization flow

[Vitor-Avila]

SUMMARY

Following up on #26199, this PR adds some tests around the column denormalization logic.

ADDITIONAL INFORMATION

• Has associated issue:
• Required feature flags:
• Changes UI
• Includes DB Migration (follow approval process in SIP-59)
  • Migration is atomic, supports rollback & is backwards-compatible
  • Confirm DB migration upgrade and downgrade tested
  • Runtime estimates and downtime expectations provided
• Introduces new feature or API
• Removes existing feature or API

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (05d7060) 69.19% compared to head (62bb4b5) 69.10%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #26220      +/-   ##
==========================================
- Coverage   69.19%   69.10%   -0.09%     
==========================================
  Files        1945     1945              
  Lines       75929    75929              
  Branches     8453     8453              
==========================================
- Hits        52538    52473      -65     
- Misses      21207    21272      +65     
  Partials     2184     2184              

Flag	Coverage Δ
hive	?
mysql	78.10% <ø> (+0.02%)	⬆️
postgres	78.20% <ø> (+<0.01%)	⬆️
presto	53.64% <ø> (ø)
python	82.71% <ø> (-0.18%)	⬇️
sqlite	?
unit	55.81% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[villebro]

Great tests, thanks for adding these @Vitor-Avila 👍 As there seems to be general consensus that the inverse is the better name here (denormalize_columns as opposed to norrmalize_columns), do you think it would be helpful to update the UI, table column, and the method in BaseEngineSpec? This would provide a more intuitive devex for future devs who have to maintain this logic. If needed I can probably take on this task, as I feel guilty for having introduced the original name..

[Vitor-Avila]

Great tests, thanks for adding these @Vitor-Avila 👍 As there seems to be general consensus that the inverse is the better name here (denormalize_columns as opposed to norrmalize_columns), do you think it would be helpful to update the UI, table column, and the method in BaseEngineSpec? This would provide a more intuitive devex for future devs who have to maintain this logic. If needed I can probably take on this task, as I feel guilty for having introduced the original name..

hey @villebro! I'm not 100% sure on the most accurate name to represent this feature, so I'd leave this decision up for you. I think the confusing part to me was that we were introducing a new feature to handle the column casing, but then we can "enable the disablement" via the UI. I think it would make more sense to me that the checkbox in the UI would be used to enable the feature, which would then be disabled for older datasets, but enabled by default for new ones. I understand the decision process here (the checkbox was introduced after the feature, so it made sense to keep it disabled when we were doing it since it was already live).

Let me know your thoughts -- happy to help as much as I can.

[michael-s-molina]