Adopt active_record.postgresql_adapter_decode_dates = true (currently overridden to false)

[philayres]

Background

As part of the Rails 8 upgrade (#1015), advancing config.load_defaults to 7.2 adopts the Rails 7.2 default:

• active_record.postgresql_adapter_decode_dates = true

This causes date-typed columns returned via raw SQL (connection.exec_query, select_value, etc.) to be decoded as Ruby Date objects instead of String. Previously such values came back as strings.

Why this is temporarily overridden

Test coverage for raw-SQL date decoding is incomplete. There are direct raw-SQL paths that bypass the ActiveRecord type layer, e.g.:

• app/models/admin/migration_generator.rb (connection.exec_query)
• app/models/full_text_search/tsvector_writer.rb (conn.exec_query)
• app/models/nfs_store/manage/archived_file.rb (connection.exec_query)

In addition, report and dynamic-model SQL is configuration-driven, so the test suite cannot see all consumers of raw date values. To keep behaviour attributable during the Rails 8 upgrade, we have explicitly pinned the old behaviour:

# config/application.rb
config.active_record.postgresql_adapter_decode_dates = false

Goal

Remove the override and adopt the Rails 7.2 default (= true) once we have verified all raw-SQL date consumers behave correctly with Date objects.

Acceptance criteria

• Audit every raw-SQL execution path (exec_query, select_value, select_all, select_rows, find_by_sql) that can return a date-typed column.
• Add specs asserting the returned Ruby type / correct handling for those paths.
• Review configuration-driven report/dynamic-model SQL for date-string assumptions.
• Remove the postgresql_adapter_decode_dates = false override from config/application.rb.
• Full RSpec suite green with the default enabled.

Refs #1015

[philayres]

Issue #1295 — postgresql_adapter_decode_dates audit findings

Change made

config/application.rb: removed the config.active_record.postgresql_adapter_decode_dates = false
override entirely, adopting the Rails 7.2 default (true) set by config.load_defaults 7.2.

Audit of every raw-SQL execution path

Grepped the whole codebase for exec_query, select_all, select_rows, select_value,
find_by_sql, and connection.execute (the setting is applied at the PG connection's
type_map_for_results, so execute is affected exactly the same as exec_query/select_all
unless a caller installs its own type map — see below):

• app/models/admin/migration_generator.rb — tables_and_views, column_comments,
  views_referencing_table, schemas_in_db, foreign_keys, data_dic, and the three exec_query
  calls (primary-key lookup, single view-definition lookup, and a LIKE-based multi-view lookup).
  All select only text/varchar/name/oid columns (schema/table/column names, comments, SQL
  definitions) — no date-typed column is ever selected by any of these queries.
• app/models/full_text_search/tsvector_writer.rb — both exec_query calls are
  INSERT/UPDATE statements; the returned ActiveRecord::Result is discarded.
• app/models/nfs_store/manage/archived_file.rb — exec_query (in remove_duplicates) is an
  UPDATE, not a delete despite the method name: it relabels duplicate rows' file_name to a
  "duplicates-<timestamp>" marker rather than deleting them. Result discarded either way.
• app/models/dynamic/data_dictionary.rb, app/models/dynamic/external_id_implementer.rb
  — raw connection.execute calls are all UPDATE/INSERT statements; results discarded.
• app/models/dynamic/version_handler.rb#all_versions_query — select * from #{history_table_name} where history_table_name is the definition class's own history
  table (dynamic_models_history, activity_logs_history, external_identifiers_history,
  config_libraries_history, message_templates_history — config/metadata tables, no date
  columns). The raw hash is passed to self.class.new(res.to_h), which re-casts every value
  through the model's normal AR attribute assignment regardless of what type PG handed back —
  ActiveRecord::Type::Date#cast accepts both String and Date inputs, so this path is safe
  even in the hypothetical case of a date column appearing here in the future. Confirmed via
  the existing extensive spec/models/dynamic/version_handler_spec.rb (and several other
  specs exercising all_versions/all_versions_query), all green.
• app/models/reports/runner.rb#run (config-driven report SQL) — uses
  connection.execute(sql) but then explicitly overrides results.type_map with
  PG::BasicTypeMapForResults.new(connection.raw_connection) immediately afterwards. This
  installs the pg gem's own type map on that specific result object, which already decodes
  date columns as Ruby Date (and always has, independent of Rails' decode_dates setting).
  Report SQL results were never affected by this Rails setting at all — any report that
  selects a date column has always received a Ruby Date object. Downstream rendering
  (ReportResults::ReportsCommonResultCell#view_content → html_escape cell_content) calls
  .to_s implicitly either way, so this was never a risk.
• Remaining exec_query/select_all/select_value/find_by_sql matches are in spec-only code
  (test support helpers under spec/support/, plus a couple of integration/unit specs directly —
  spec/models/option_configs/dynamic_model_options_spec.rb and
  spec/system/apps/play_ipa/play_ipa_process_spec.rb), and only ever select
  count/id/extra_log_type/text columns — no dates.

Conclusion: no application code path currently returns a raw date-typed column value through
an un-cast raw-SQL result. The override could be removed with no code changes.

New spec

Added spec/models/admin/migration_generator_decode_dates_spec.rb, which creates a real table
with a date column and confirms:

1. A raw ::date literal decodes as a Ruby Date via Admin::MigrationGenerator.connection.execute.
2. A SELECT * against a table with a date column, consumed as a plain hash (the pattern used
   by data_dic/column_comments/etc.), also decodes as Date — locking in the behavior for
   any raw-SQL consumer added in future.

(See the follow-up comment below for corrections made to this spec after review.)

Targeted spec validation

Ran the new spec plus existing regression coverage for every audited consumer: 59 examples,
0 failures.

• migration_generator_decode_dates_spec.rb, migration_generator_schema_search_spec.rb,
  dynamic/version_handler_spec.rb, report_spec.rb — 16 examples, 0 failures.
• full_text_search/tsvector_writer_spec.rb, dynamic/versioned_field_list_spec.rb,
  option_configs/dynamic_model_options_spec.rb (includes a raw exec_query against a history
  table), report_options_spec.rb, external_identifier_spec.rb — 43 examples, 0 failures.

Conclusion

config.active_record.postgresql_adapter_decode_dates adopting the Rails 7.2 default (true)
is safe. No application code changes were required beyond removing the override. One new
regression spec added to lock in raw-SQL date-decoding behavior for future consumers.

---

Edit: corrected two inaccuracies caught on review: the archived_file.rb call is an UPDATE,
not a DELETE (method name remove_duplicates is misleading — it relabels rows rather than
removing them), and migration_generator.rb has three exec_query calls, not two. Also
clarified that not all "remaining" raw-SQL matches are in spec/support/. No change to the
overall conclusion.

[philayres]

Follow-up to the audit findings above (post-review corrections)

An independent review (Claude Opus 5, via subagent) verified this change against the actual
Rails/pg source and re-read every call site. Recommendation: approve with follow-ups. No
change to the core conclusion (safe to adopt the Rails 7.2 default), but several corrections and
strengthenings applied, now pushed to postgresql-decode-dates-1295:

Corrections to the record

• Admin::MigrationGenerator does not select "only text/varchar/oid" everywhere.
  data_dic (app/models/admin/migration_generator.rb, SELECT * FROM #{basename}_datadic)
  runs against an admin/project-created table whose schema this repo doesn't control. It's safe
  only because its one consumer (ReportResults::ReportsTableHeaderCell#data_dictionary_view)
  reads exclusively text columns (variable_name, domain, field_attributes, field_note).
• Reports::Runner#result_tables (line ~128, Report.connection.execute(clean_sql)) is
  actually governed by this Rails setting — it does not get the PG::BasicTypeMapForResults
  override (that's only applied to the main results object). It's safe only because it selects
  pg_class.relname (a name type, never a date), not because it's exempt from the setting.
• Two previously-unnamed config-driven SQL surfaces confirmed safe and added to the record:
  ActivityLog::Perspectives::Runner (report-backed perspectives only ever read r['id'].to_i,
  and the order-by path is integer-sanitized), and Admin::MigrationGenerator.data_dic (above).
• Schema evidence for the Dynamic::VersionHandler claim: independently confirmed via
  db/structure.sql that none of activity_log_history, config_library_history,
  dynamic_model_history, external_identifier_history, message_template_history has any
  date-typed column (all character varying/integer/boolean/timestamp).

Spec corrections (spec/models/admin/migration_generator_decode_dates_spec.rb)

• Moved the test table from Admin::MigrationGenerator.current_search_paths.first (which
  resolved to ml_app, the core app schema) to the repo's dedicated dynamic_test schema for
  test-only tables.
• Added Admin::MigrationGenerator.tables_and_views_reset! to after :all, avoiding a dropped
  table lingering in the class-level memo for later specs in the same process.
• Added coverage for the exec_query/ActiveRecord::Result path (the one actually used by
  Admin::MigrationGenerator's and FullTextSearch::TsvectorWriter's raw-SQL call sites) —
  previously only connection.execute was covered.
• Added a Reports::Runner#run example asserting a date-selecting report SQL yields a Ruby
  Date, locking in the type-map-override behavior against future refactors.
• Added an explicit assertion that ActiveRecord::ConnectionAdapters::PostgreSQLAdapter.decode_dates
  is true.
• Removed dead setup (unused create_admin, no-op ON CONFLICT DO NOTHING).

Re-ran the new spec plus existing regression coverage together (to confirm no memo-pollution
interference): 27 examples, 0 failures. (Note: this remains a targeted validation, not a full
suite run — the issue's final acceptance criterion of a full green suite is still outstanding.)