Inconsistency between Time formats in ROM::Changeset::PipeRegistry and ROM::Plugins::Command::Timestamps

[Aerdayne]

Describe the bug

When timestamps are added using PipeRegistry.add_timestamps (source) current time is NOT converted to UTC.

Command::Timestamps (source), on the other hand, converts the current time to UTC.

This leads to a situation in certain scenarios (for example, when Postgres is the database adapter - Sequel's DateTime type is mapped to Postgres' timestamp without time zone, hence the timezone offset is not being parsed) where equal timestamps differ since one of the timestamps gets converted to UTC beforehand.

I think that both APIs should at least have the same behavior, as in either they both convert to UTC before persisting the timestamps, or both preserve the application timezone.

To Reproduce

require 'rom'

rom = ROM.container(:sql, 'postgres://username:password@0.0.0.0/database') do |conf|
  conf.default.create_table(:users) do
    primary_key :id
    column :data, String, null: false

    column :created_at, DateTime, null: false # Postgres' "timestamp without time zone"
  end

  class Users < ROM::Relation[:sql]
    schema(infer: true)
  end

  conf.register_relation(Users)
end

class UserRepo < ROM::Repository[:users]
  commands :create, use: :timestamps, plugins_options: { timestamps: { timestamps: %i[created_at] } }

  def create_with_changeset(data)
    users
      .changeset(:create, { data: data })
      .map(:add_timestamps)
      .commit
  end

  def find(id)
    root.by_pk(id).one
  end
end

repo = UserRepo.new(rom)

with_command = repo.create(data: 'user #1')
with_changeset = repo.create_with_changeset('user #2')

Time.now # => 2022-01-04 12:51:44.463493 +0300
created_at_from_command = with_command.created_at # => 2022-01-04 09:51:44.451431 +0300
created_at_from_changeset = with_changeset.created_at # => 2022-01-04 12:51:44.460317 +0300

p created_at_from_changeset - created_at_from_command < 0.1 # => false

Expected behavior

created_at_from_command and created_at_from_changeset should be approximately the same, not have a 3-hour difference.

My environment

• Ruby version: 3.0.1
• ROM versions:
  rom-5.2.6
  rom-changeset-5.2.3
  rom-core-5.2.6
  rom-repository-5.2.2
  rom-sql-3.5.0

This is possible to avoid by replacing .map(:add_timestamps) with .map { |tuple| tuple.merge(created_at: Time.now.utc) } as a temporary workaround.

Should I submit a PR?

[solnic]

Thanks for reporting this. I'm afraid this needs to wait until rom 6.0 because it's one of those cases where fixing a bug may break people's code.