Use `SET CONSTRAINTS` for `disable_referential_integrity` without superuser privileges (take 2) #27636

mtsmfm · 2017-01-10T17:32:58Z

Re-create #21233

eeac615 was reverted (127509c) because it breaks tests.

ref: 72c1557

context: #5523

Other Information

We must use authors fixture with author_addresses because of its foreign key constraint.
Tests require PostgreSQL >= 9.4.2 because it had a bug about ALTER CONSTRAINTS and fixed in 9.4.2.

rails-bot · 2017-01-10T17:33:01Z

r? @chancancode

(@rails-bot has picked a reviewer for you, use r? to override)

yahonda · 2017-02-07T14:18:22Z

Let me add some comment for this pull request.

There was a pull request #21233 which had merged and reverted back due to regressions. I believe this pull request addresses regression by loading necessary :author_address fixtures.

mtsmfm · 2017-03-15T00:47:23Z

Sorry, test is failing after rebase 😢
I'll investigate and fix it.

mtsmfm · 2017-03-15T15:27:29Z

Sorry, test is failing after rebase
I'll investigate and fix it.

https://travis-ci.org/rails/rails/jobs/210515444

1) Failure:
AssociationProxyTest#test_save_on_parent_saves_children [/home/travis/build/rails/rails/activerecord/test/cases/associations_test.rb:185]:
Expected: 1
  Actual: 2

I found the cause.
It is not related to this PR.
#28426 will fix it.

ericboehs · 2017-03-20T19:54:19Z

It looks like the tests are passing and this pull request is ready for another review?

I'm hopeful this will get merged for 5.1 as it is needed for Heroku CI to work. <3

Edit: Annnd RC1 just landed. :(

mtsmfm · 2017-03-21T15:22:40Z

@rafaelfranca @chancancode Could you review this pr?

rafaelfranca

Sorry for the delay. I just reviewed it. Could you made the change and rebase?

rafaelfranca · 2017-03-22T23:46:47Z

activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb

@@ -314,6 +314,12 @@ def supports_pgcrypto_uuid?
        postgresql_version >= 90400
      end

+      # PostgreSQL 9.4 introduces ALTER TABLE ... ALTER CONSTRAINT but it has a bug and fixed in 9.4.2
+      # https://www.postgresql.org/docs/9.4/static/release-9-4-2.html


Could you move this comment to inside the method? Otherwise it will show as documentation of it.

Moved and force pushed!

BTW, I followed the same way as supports_ranges? so it may not be intended.
I created another PR #28543.
Please close the PR if the comment is intended 😅

Because this comment is not document for `supports_ranges?` ref: rails#27636 (comment)

mtsmfm · 2017-03-26T08:11:49Z

https://travis-ci.org/rails/rails/builds/214205098

ap, ac test is failed.
It isn't related to this pr.

I'll rebase and force push to restart test.

…eruser privileges (take 2) Re-create rails#21233 eeac615 was reverted (127509c) because it breaks tests. ---------------- ref: 72c1557 - We must use `authors` fixture with `author_addresses` because of its foreign key constraint. - Tests require PostgreSQL >= 9.4.2 because it had a bug about `ALTER CONSTRAINTS` and fixed in 9.4.2.

mtsmfm · 2017-03-26T09:25:38Z

#27636 (comment)

@rafaelfranca I moved the comment and force pushed 😃

ericboehs · 2017-03-27T21:26:50Z

@rafaelfranca Forgive my ignorance here, but are changes to master now targeting 5.1.1? Instead of 5.1.0?

rafaelfranca · 2017-03-27T21:30:50Z

5.2.0 actually. But I'll backport it to 5.1.0

…ithout-superuser-privilege-take-2 Use `SET CONSTRAINTS` for `disable_referential_integrity` without superuser privileges (take 2)

rafaelfranca · 2017-03-27T21:31:19Z

Backported in 1ccd40e

ericboehs · 2017-03-27T21:32:19Z

That's great news! Thank you kindly.

Related rails#28532, rails#27636

…egrity-without-superuser-privilege-take-2" This reverts commit 1ccd40e. Fixes #28888 Referential actions other than the NO ACTION check cannot be deferred, even if the constraint is declared deferrable.

rafaelfranca · 2017-04-26T20:38:52Z

Reverted again in 2812694. Seems like referential actions other than the NO ACTION check cannot be deferred, even if the constraint is declared deferrable.

…egrity-without-superuser-privilege-take-2" This reverts commit c1faca6, reversing changes made to 8c658a0. See #27636 (comment)

`:authors` has a foreign key to `:author_addresses`. If only `:authors` fixture loaded into the database which supports foreign key and checks the existing data when enabling foreien keys like Oracle, it raises the following error `ORA-02298: cannot validate (ARUNIT.FK_RAILS_94423A17A3) - parent keys not found` It is because there is no parent data exists in `author_addresses` table. Here are how other database with foreign key support works: - MySQL does not check the existing data when enabling foreign key by `foreign_key_checks=1` https://dev.mysql.com/doc/refman/5.7/en/server-system-variables.html#sysvar_foreign_key_checks > Setting foreign_key_checks to 1 does not trigger a scan of the existing table data. Therefore, rows added to the table while foreign_key_checks=0 will not be verified for consistency. - PostgreSQL database itself has a feature to check existing data when enabling foreign key and discussed at rails#27636, which is reverted.

ASing0 · 2022-11-09T17:41:11Z

If I understand this issue and fix correctly, you added a parameter so you can run tests with foreign keys, that normally require superuser privileges, without those privileges. If this is correct, (and I hope it is because I'm trying to run Rails tests on Azure Postgres, which does not allow superuser), would you kindly tell me where this parameter is and what to change it to?

felixwatts · 2023-10-20T09:29:24Z

Same question here

zzak · 2023-10-20T22:04:15Z

@felixwatts @ASing0 This is just a suggestion, but if you've hit a bug running the rails test suite, it's preferable to open a new ticket with a reproducible test case than comment asking for help on a 5 year old PR. Alternatively, you can ask as many questions you want on the discuss. 🙇

rails-bot assigned chancancode Jan 10, 2017

maclover7 added activerecord PostgreSQL labels Jan 10, 2017

mtsmfm force-pushed the disable-referential-integrity-without-superuser-privilege-take-2 branch from 656edb7 to 8aba178 Compare March 13, 2017 10:50

mtsmfm force-pushed the disable-referential-integrity-without-superuser-privilege-take-2 branch from 8aba178 to 1829d0d Compare March 15, 2017 15:23

mtsmfm force-pushed the disable-referential-integrity-without-superuser-privilege-take-2 branch 2 times, most recently from 7f9d22d to f4ada34 Compare March 16, 2017 08:44

maclover7 added the needs feedback label Mar 21, 2017

rafaelfranca requested changes Mar 22, 2017

View reviewed changes

mtsmfm force-pushed the disable-referential-integrity-without-superuser-privilege-take-2 branch from f4ada34 to b410c79 Compare March 23, 2017 11:21

mtsmfm added a commit to mtsmfm/rails that referenced this pull request Mar 23, 2017

Move comment to inside the method [ci skip]

6971702

Because this comment is not document for `supports_ranges?` ref: rails#27636 (comment)

mtsmfm mentioned this pull request Mar 23, 2017

Move comment to inside the method [ci skip] #28543

Merged

mtsmfm and others added 2 commits March 26, 2017 17:12

Load :author_addresses

ef6391d

mtsmfm force-pushed the disable-referential-integrity-without-superuser-privilege-take-2 branch from b410c79 to ef6391d Compare March 26, 2017 08:12

rafaelfranca approved these changes Mar 27, 2017

View reviewed changes

rafaelfranca merged commit c1faca6 into rails:master Mar 27, 2017

rafaelfranca added a commit that referenced this pull request Mar 27, 2017

Merge pull request #27636 from mtsmfm/disable-referential-integrity-w…

1ccd40e

…ithout-superuser-privilege-take-2 Use `SET CONSTRAINTS` for `disable_referential_integrity` without superuser privileges (take 2)

maclover7 removed the needs feedback label Mar 27, 2017

y-yagi added a commit to y-yagi/rails that referenced this pull request Apr 3, 2017

CI against Ruby 2.4.1

f2e3f59

Related rails#28532, rails#27636

y-yagi mentioned this pull request Apr 3, 2017

CI against Ruby 2.4.1 #28649

Merged

AndrewSwerlick mentioned this pull request Apr 26, 2017

5.1.0.rc2 Does not properly handle fixtures on postgres where the tables have a foreign key with on_delete restrict or cascade. #28888

Closed

rafaelfranca added a commit that referenced this pull request Apr 26, 2017

Revert "Merge pull request #27636 from mtsmfm/disable-referential-int…

eac6f36

…egrity-without-superuser-privilege-take-2" This reverts commit c1faca6, reversing changes made to 8c658a0. See #27636 (comment)

david-a-wheeler mentioned this pull request May 26, 2018

Having trouble with test fixture with blind_index ankane/blind_index#2

Closed

Use SET CONSTRAINTS for disable_referential_integrity without superuser privileges (take 2) #27636

Use SET CONSTRAINTS for disable_referential_integrity without superuser privileges (take 2) #27636

Conversation

mtsmfm commented Jan 10, 2017

Other Information

Uh oh!

rails-bot commented Jan 10, 2017

Uh oh!

yahonda commented Feb 7, 2017

Uh oh!

mtsmfm commented Mar 15, 2017

Uh oh!

mtsmfm commented Mar 15, 2017

Uh oh!

ericboehs commented Mar 20, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mtsmfm commented Mar 21, 2017

Uh oh!

rafaelfranca left a comment

Choose a reason for hiding this comment

Uh oh!

rafaelfranca Mar 22, 2017

Choose a reason for hiding this comment

Uh oh!

mtsmfm Mar 23, 2017

Choose a reason for hiding this comment

Uh oh!

mtsmfm commented Mar 26, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mtsmfm commented Mar 26, 2017

Uh oh!

ericboehs commented Mar 27, 2017

Uh oh!

rafaelfranca commented Mar 27, 2017

Uh oh!

rafaelfranca commented Mar 27, 2017

Uh oh!

ericboehs commented Mar 27, 2017

Uh oh!

rafaelfranca commented Apr 26, 2017

Uh oh!

ASing0 commented Nov 9, 2022

Uh oh!

felixwatts commented Oct 20, 2023

Uh oh!

zzak commented Oct 20, 2023

Uh oh!

Uh oh!

Use `SET CONSTRAINTS` for `disable_referential_integrity` without superuser privileges (take 2) #27636

Use `SET CONSTRAINTS` for `disable_referential_integrity` without superuser privileges (take 2) #27636

ericboehs commented Mar 20, 2017 •

edited

Loading

mtsmfm commented Mar 26, 2017 •

edited

Loading