Skip to content

Conversation

@aidanharan
Copy link
Contributor

@aidanharan aidanharan commented Apr 15, 2021

Previously in #875 the column deduplication method was updated to handle default boolean values. The change did not handle other non-string classes such as Time/Date/Integer/etc. This is the cause of the NoMethodError: undefined method '-@' errors in the main CI build:

https://pipelines.actions.githubusercontent.com/OGQvFf2eDbGUprKdJ5qsq3TsAFLfKWDWAJCyTHn8ess0fbvp0X/_apis/pipelines/1/runs/67/signedlogcontent/6?urlExpires=2021-04-15T12%3A48%3A19.5876504Z&urlSigningMethod=HMACV1&urlSignature=BboOo2lvNJlxvLAzbOHeep1jlPH%2FtSuW75%2BiBiJvq1s%3D

Updated the ActiveRecord::ConnectionAdapters::SQLServerColumn#deduplicated method to create a frozen copy of any object that is not a String.

Before
https://github.com/rails-sqlserver/activerecord-sqlserver-adapter/runs/2354091308
7133 runs, 18455 assertions, 26 failures, 145 errors, 37 skips

After
https://github.com/rails-sqlserver/activerecord-sqlserver-adapter/pull/880/checks?check_run_id=2354573868
7133 runs, 20119 assertions, 25 failures, 80 errors, 37 skips

@aidanharan aidanharan force-pushed the deduplicate-date-value branch from 1dfb90f to 686a21d Compare April 15, 2021 15:26
@aidanharan aidanharan marked this pull request as ready for review April 15, 2021 15:39
@aidanharan aidanharan marked this pull request as draft April 15, 2021 15:47
@aidanharan aidanharan force-pushed the deduplicate-date-value branch from 686a21d to a67c311 Compare April 15, 2021 15:59
@aidanharan aidanharan marked this pull request as ready for review April 15, 2021 16:08
@wpolicarpo wpolicarpo merged commit c98e72f into rails-sqlserver:main Apr 15, 2021
lavika pushed a commit to lavika/activerecord-sqlserver-adapter that referenced this pull request Sep 26, 2023
Co-authored-by: Aidan Haran <aharan@fusioneer.com>
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.

2 participants