Skip to content

Conversation

fatkodima
Copy link
Member

Fixes #49591.

While testing changes made in this PR, I identified that quite a few tests are broken for PostgreSQL < 10. So I fixed them as a separate commit.

@kamipo kamipo merged commit 68b119a into rails:main Oct 12, 2023
@fatkodima fatkodima deleted the fix-identity-columns-pg-9 branch October 12, 2023 11:20
kamipo added a commit that referenced this pull request Oct 12, 2023
Fix detecting `IDENTITY` columns for PostgreSQL < 10
@andyatkinson
Copy link
Member

@fatkodima Version 10 of PostgreSQL is no longer supported by PostgreSQL, FWIW

https://www.postgresql.org/support/versioning/

Supported=No
Final Release=November 10, 2022

Do you know what the policy is for Rails on EOL versions of DB adapters?

For new versions of Rails, I'd propose that PostgreSQL 10 is not supported. Maybe Rails version 8 drops PostgreSQL 10 support?

@fatkodima
Copy link
Member Author

I don't think there is any policy regarding dropping older versions of dbms'es. Maybe when maintaining it becomes unnecessary harder?

cc @rafaelfranca @yahonda

@andyatkinson
Copy link
Member

@fatkodima Sounds good.

My recommendation would be Ruby on Rails consider dropping support for version 10 of PostgreSQL. This is based on the open source community distribution guidance above, and what I've seen from major cloud hosting providers like AWS. AWS stopped supporting PostgreSQL version 10 for example:

https://repost.aws/questions/QUph1IFLkkRiyc0pCdTH493Q/announcement-amazon-rds-for-postgresql-10-deprecation

As you probably know, PostgreSQL major versions are released annually since version 10, and there have been 6 newer annual releases since then. The current version is 16 released in September 2023.

I'm sure there are still many Ruby on Rails applications running with PostgreSQL 10 (or even earlier versions) in the wild, so I acknowledge there's a need for advanced communication.

I could put this into an Issue if that's a better way to make a proposal.

The goal to me would be able to remove code and tests that are only for unsupported versions.

Thanks!

@andyatkinson andyatkinson mentioned this pull request Oct 23, 2023
@andyatkinson
Copy link
Member

@fatkodima This was kind of a rabbit hole from finding that this test didn't pass for me locally. If you have any tips let me know, I may be doing something wrong: #49744

@matthewd
Copy link
Member

Maybe when maintaining it becomes unnecessary harder?

Yep. Especially because of how much inertia database servers in particular can have, I like to use a threshold of "unreasonably complicated".

Previously: #41526 (comment)

There are also some edge cases involving protocol-compatible forks/variants/similar, which can also trail the upstream version. We're not going to do work to support them in ways where they differ from upstream, but again, refusing to connect just out of principle seems needlessly punitive.

That said, while I'd hesitate to drag us all the way up to the EOL boundary, it probably does make sense to review what conditionals & branches we can eliminate by at least requiring a higher 9.x, or even >= 10.

@andyatkinson
Copy link
Member

There are also some edge cases involving protocol-compatible forks/variants/similar, which can also trail the upstream version.

Good point. You're referring to things like pgbouncer, Citus extension, AWS Aurora, Neon, YugabyteDB etc. that might be PostgresSQL protocol compatible, but behind on versions, correct?

Even though they aren't officially supported/tested by Active Record, there would need to be some significant value and justification to explicitly break something that's working: agreed.

@jcoleman
Copy link
Contributor

FWIW I think this fails with PG11 also; the restriction was removed in PG12.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rails 7.1.1 breaks Postgres < 10
5 participants