add_column should raise warning about strong_migrations HOT 8 CLOSED

That's a good trick @trevorrjohn. I'll switch to doing that. I didn't think that would work because I thought ignored_columns didn't affect the query ie. it still did SELECT * but ignored the result.

However what seems to happen is it reflects on the table structure and explicitly selects everything but columns matching SELECT table.column_1, table.column_2 ....

Explicitly selecting columns will protect from the prepared statement errors as I believe the reflection and cache preparation happen at the same time. This means adding only one ignored_column will protect all further new column additions to that table. ie. adding two columns you can ignore one OR just have a non-empty ignored_columns array even if that column will never exist.

I'm not sure if this is a good idea but it is possible to add a self.ignored_columns = [:protect_from_expired_cache] to the ActiveRecord::Base and then the error could never occur again (cost could be larger memory footprint for cached statements, not sure what will happen with joins or explicit SQL SELECT * statements) and possibly new columns could be added safely with zero downtime as all selects are explicit.

from strong_migrations.

Hey @trevorrjohn, I haven't seen that error before, and the only mention I can find on Google is this post, so I'm not sure how common it is.

You can always manually add it to your app with a custom check.

from strong_migrations.

We have been switching prepared statements off before the deploy and back on again afterwards. @ankane for more details about the error checkout this Rails issue: rails/rails#12330.

from strong_migrations.

I think adding to ignored_columns is a better solution than switching of prepared statements. I agree I can add it manually, but do think it should be a default in this gem. Happy to submit a PR if you agree.

from strong_migrations.

Hey @aldhsu, that's an interesting idea, but I'm worried about making it the default. It seems like apps that need it can do:

ActiveRecord::Base.ignored_columns = [""]

I'm hesitant to make add_column raise a warning since I haven't personally encountered this (we disabled prepared statements at Instacart when we added pgbouncer, and I don't recall running into this error before then).

from strong_migrations.

Ok. I'm fine with closing for now. We can reopen if more people encounter this issue.

from strong_migrations.

A nice PR is probably coming in Rails 7 that will let use force columns enum rails/rails#41718

Edit: PR has been merged into Rails 7

from strong_migrations.

Hey @ankane I'm wondering if this should be re-opened. We're currently on Rails 6 and this is something we have to work with for our Postgres database. I've reproduced this locally as well:

Reproduction steps

1. Open a rails console.
2. Manually add a new column to a table.
3. In the rails console from step 1, do ActiveRecord::Base.transaction do { .... }.
4. See error.

postgres=# \c development
You are now connected to database "development" as user "daniel.dao".
development=# SELECT * FROM employments;
 id | user_id | employer_name | status | created_at | updated_at
----+---------+---------------+--------+------------+------------
(0 rows)

development=# ALTER TABLE employments ADD new_metric_colum integer;
ALTER TABLE
development=# SELECT * FROM employments;
 id | user_id | employer_name | status | created_at | updated_at | new_metric_colum
----+---------+---------------+--------+------------+------------+------------------

[2] pry(main)> ActiveRecord::Base.transaction { Employment.last }

  TRANSACTION (1.5ms)  BEGIN
  Employment Load (7.5ms)  SELECT "employments".* FROM "employments" ORDER BY "employments"."id" DESC LIMIT $1  [["LIMIT", 1]]
  TRANSACTION (0.3ms)  ROLLBACK
ActiveRecord::PreparedStatementCacheExpired: ERROR:  cached plan must not change result type

from /Users/daniel.dao/.rbenv/versions/3.0.5/lib/ruby/gems/3.0.0/gems/activerecord-6.1.7.6/lib/active_record/connection_adapters/postgresql_adapter.rb:696:in `rescue in exec_cache'
Caused by ActiveRecord::StatementInvalid: PG::FeatureNotSupported: ERROR:  cached plan must not change result type

from /Users/daniel.dao/.rbenv/versions/3.0.5/lib/ruby/gems/3.0.0/gems/activerecord-6.1.7.6/lib/active_record/connection_adapters/postgresql_adapter.rb:687:in `exec_prepared'
Caused by PG::FeatureNotSupported: ERROR:  cached plan must not change result type

from /Users/daniel.dao/.rbenv/versions/3.0.5/lib/ruby/gems/3.0.0/gems/activerecord-6.1.7.6/lib/active_record/connection_adapters/postgresql_adapter.rb:687:in `exec_prepared'

Currently right now this has affected our users during migrations. I'd love for there to be an automated way to prevent developers from accidentally running into this. Ideally by using ignored_columns.

I'm happy to work on a PR as well, but wanted to see if re-opening the issue makes sense to you!

from strong_migrations.