Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

db:fixtures:load raises in Postgres if there are foreign key constraints in a non-public schema #44760

Merged
merged 1 commit into from Jul 19, 2022

Conversation

lxxxvi
Copy link
Contributor

@lxxxvi lxxxvi commented Mar 24, 2022

Hi there 👋

I think I found a tiny issue in a edgecase when loading fixtures bin/rails db:fixtures:load into Postgres.

Steps to reproduce

a) Use repository

The issue can be reproduced using this repository, see its README for details:

b) Manual setup

  1. Create a new rails project
  2. Create a non-public schema and tables and at least one foreign key constraint, for example
CREATE SCHEMA other;

CREATE TABLE IF NOT EXISTS other.authors (
  id          INT       GENERATED ALWAYS AS IDENTITY,
  name        TEXT      NOT NULL,
  PRIMARY KEY(id)
);

CREATE TABLE IF NOT EXISTS other.books (
  id          INT       GENERATED ALWAYS AS IDENTITY,
  author_id   INT       NOT NULL,
  name        TEXT      NOT NULL,
  PRIMARY KEY(id),
  CONSTRAINT fk_authors FOREIGN KEY(author_id)
                        REFERENCES other.authors(id)
);
  1. Create a common (public) Rails model with a fixture
bin/rails g model Cat name:string
  1. Run bin/rails db:fixtures:load

Expected behavior

bin/rails db:fixtures:load should load without an error.

Actual behavior

rails aborted!
Foreign key violations found in your fixture data. Ensure you aren't referring to labels that don't exist on associations.

Tasks: TOP => db:fixtures:load
(See full trace by running task with --trace)

There's only one fixture (cats.yml) and it does not reference anything.

Under the hood I found this problem:

ERROR:  relation "books" does not exist (PG::UndefinedTable)
CONTEXT:  SQL statement "UPDATE pg_constraint SET convalidated=false WHERE conname = 'fk_authors'; ALTER TABLE books VALIDATE CONSTRAINT fk_authors;"

System configuration

Rails version: 7.0.2.3

Ruby version: ruby 3.1.0p0 (2021-12-25 revision fb4df44d16) [arm64-darwin21]

Postgres version: 14.2

@yahonda
Copy link
Member

yahonda commented Jun 14, 2022

Would you add some tests for this change?

@lxxxvi
Copy link
Contributor Author

lxxxvi commented Jun 18, 2022

@yahonda 👍 Let me try to come up with a test.

@lxxxvi lxxxvi force-pushed the fix_rails_issue_db_fixtures_load_raises branch 2 times, most recently from 1d866e3 to 423339f Compare July 8, 2022 14:58
@lxxxvi
Copy link
Contributor Author

lxxxvi commented Jul 8, 2022

@yahonda I think I found a way to test this. We need to manually create another schema with a table and a foreign key, then we can assert that #all_foreign_keys_valid? is (still) true.

Let me know what you think of it.

@yahonda
Copy link
Member

yahonda commented Jul 11, 2022

Creating a schema for the tests is fine as long as these schemas are dropped at the end of the tests.

@lxxxvi
Copy link
Contributor Author

lxxxvi commented Jul 11, 2022

Creating a schema for the tests is fine as long as these schemas are dropped at the end of the tests.

Yes, I did that.

@yahonda
Copy link
Member

yahonda commented Jul 14, 2022

Would you address these CI failures?

  • PostgreSQL CI failures
    It reproduces with this script. It looks like the parrots table has some foreign key(s) and the parent fixture may not be loaded.
ARCONN=postgresql bin/test test/cases/fixtures_test.rb test/cases/adapters/postgresql/referential_integrity_test.rb -n "/^(?:IgnoreFixturesTest#(?:test_ignores_parrots_fixtures)|PostgreSQLReferentialIntegrityTest#(?:test_all_foreign_keys_valid_having_foreign_keys_in_multiple_schemas))$/" --seed 25206

Thanks.

@lxxxvi
Copy link
Contributor Author

lxxxvi commented Jul 14, 2022

@yahonda Thank you for your explanation yesterday, I'm grateful for that. 🙌

Yes, I'll take a look at the failing CI, will let you know when it's done. 👍

@yahonda
Copy link
Member

yahonda commented Jul 15, 2022

It should address PostgreSQL CI failures.

% git diff 46444af7fd585dd5fbea42615a62d96fbaa25c12...ba3545d3266a74cd2fd3011840e5d076fd816b86
diff --git a/activerecord/test/cases/fixtures_test.rb b/activerecord/test/cases/fixtures_test.rb
index 772f421f2c..e7ffbc54a7 100644
--- a/activerecord/test/cases/fixtures_test.rb
+++ b/activerecord/test/cases/fixtures_test.rb
@@ -1391,7 +1391,7 @@ def test_table_name_is_defined_in_the_model
 end

 class IgnoreFixturesTest < ActiveRecord::TestCase
-  fixtures :other_books, :parrots
+  fixtures :other_books, :parrots, :parrots_pirates, :pirates, :treasures

   # Set to false to blow away fixtures cache and ensure our fixtures are loaded
   # without interfering with other tests that use the same `model_class`.
%

@lxxxvi lxxxvi force-pushed the fix_rails_issue_db_fixtures_load_raises branch from 46444af to 5fdbf5d Compare July 15, 2022 06:33
@lxxxvi
Copy link
Contributor Author

lxxxvi commented Jul 15, 2022

@yahonda
Thank you for your help, I appreciate it! I applied the fixtures fix and rebased main. Let me know if can do anything else.

@yahonda
Copy link
Member

yahonda commented Jul 15, 2022

Once the CI goes green, would you squash your commits?

@lxxxvi
Copy link
Contributor Author

lxxxvi commented Jul 15, 2022

One test failed again. Can it be that there are other places where fixtures are missing?

@yahonda
Copy link
Member

yahonda commented Jul 15, 2022

Here is the way how to find which test triggers the CI failure.

  1. Open the failing job
    https://buildkite.com/rails/rails/builds/88090#01820091-8d39-4197-8b7d-257b29405a55

  2. Run minitest_bisect as follows.

The important thing is use the seed number based on "Run options: --seed 31890"

ARCONN=postgresql bundle exec minitest_bisect "test/cases/active_record_schema_test.rb" "test/cases/adapter_prevent_writes_test.rb" "test/cases/adapter_test.rb" "test/cases/aggregations_test.rb" "test/cases/annotate_test.rb" "test/cases/arel/attributes/attribute_test.rb" "test/cases/arel/attributes/math_test.rb" "test/cases/arel/attributes_test.rb" "test/cases/arel/collectors/bind_test.rb" "test/cases/arel/collectors/composite_test.rb" "test/cases/arel/collectors/sql_string_test.rb" "test/cases/arel/collectors/substitute_bind_collector_test.rb" "test/cases/arel/crud_test.rb" "test/cases/arel/delete_manager_test.rb" "test/cases/arel/factory_methods_test.rb" "test/cases/arel/insert_manager_test.rb" "test/cases/arel/nodes/and_test.rb" "test/cases/arel/nodes/as_test.rb" "test/cases/arel/nodes/ascending_test.rb" "test/cases/arel/nodes/bin_test.rb" "test/cases/arel/nodes/binary_test.rb" "test/cases/arel/nodes/bind_param_test.rb" "test/cases/arel/nodes/case_test.rb" "test/cases/arel/nodes/casted_test.rb" "test/cases/arel/nodes/comment_test.rb" "test/cases/arel/nodes/count_test.rb" "test/cases/arel/nodes/delete_statement_test.rb" "test/cases/arel/nodes/descending_test.rb" "test/cases/arel/nodes/distinct_test.rb" "test/cases/arel/nodes/equality_test.rb" "test/cases/arel/nodes/extract_test.rb" "test/cases/arel/nodes/false_test.rb" "test/cases/arel/nodes/filter_test.rb" "test/cases/arel/nodes/grouping_test.rb" "test/cases/arel/nodes/infix_operation_test.rb" "test/cases/arel/nodes/insert_statement_test.rb" "test/cases/arel/nodes/named_function_test.rb" "test/cases/arel/nodes/node_test.rb" "test/cases/arel/nodes/not_test.rb" "test/cases/arel/nodes/or_test.rb" "test/cases/arel/nodes/over_test.rb" "test/cases/arel/nodes/select_core_test.rb" "test/cases/arel/nodes/select_statement_test.rb" "test/cases/arel/nodes/sql_literal_test.rb" "test/cases/arel/nodes/sum_test.rb" "test/cases/arel/nodes/table_alias_test.rb" "test/cases/arel/nodes/true_test.rb" "test/cases/arel/nodes/unary_operation_test.rb" "test/cases/arel/nodes/update_statement_test.rb" "test/cases/arel/nodes/window_test.rb" "test/cases/arel/nodes_test.rb" "test/cases/arel/select_manager_test.rb" "test/cases/arel/table_test.rb" "test/cases/arel/update_manager_test.rb" "test/cases/arel/visitors/dispatch_contamination_test.rb" "test/cases/arel/visitors/dot_test.rb" "test/cases/arel/visitors/mysql_test.rb" "test/cases/arel/visitors/postgres_test.rb" "test/cases/arel/visitors/sqlite_test.rb" "test/cases/arel/visitors/to_sql_test.rb" "test/cases/associations/belongs_to_associations_test.rb" "test/cases/associations/bidirectional_destroy_dependencies_test.rb" "test/cases/associations/callbacks_test.rb" "test/cases/associations/cascaded_eager_loading_test.rb" "test/cases/associations/eager_load_includes_full_sti_class_test.rb" "test/cases/associations/eager_load_nested_include_test.rb" "test/cases/associations/eager_singularization_test.rb" "test/cases/associations/eager_test.rb" "test/cases/associations/extension_test.rb" "test/cases/associations/has_and_belongs_to_many_associations_test.rb" "test/cases/associations/has_many_associations_test.rb" "test/cases/associations/has_many_through_associations_test.rb" "test/cases/associations/has_many_through_disable_joins_associations_test.rb" "test/cases/associations/has_one_associations_test.rb" "test/cases/associations/has_one_through_associations_test.rb" "test/cases/associations/has_one_through_disable_joins_associations_test.rb" "test/cases/associations/inner_join_association_test.rb" "test/cases/associations/inverse_associations_test.rb" "test/cases/associations/join_model_test.rb" "test/cases/associations/left_outer_join_association_test.rb" "test/cases/associations/nested_through_associations_test.rb" "test/cases/associations/required_test.rb" "test/cases/associations_test.rb" "test/cases/asynchronous_queries_test.rb" "test/cases/attribute_methods/read_test.rb" "test/cases/attribute_methods_test.rb" "test/cases/attributes_test.rb" "test/cases/autosave_association_test.rb" "test/cases/base_prevent_writes_test.rb" "test/cases/base_test.rb" "test/cases/batches_test.rb" "test/cases/binary_test.rb" "test/cases/bind_parameter_test.rb" "test/cases/boolean_test.rb" "test/cases/cache_key_test.rb" "test/cases/calculations_test.rb" "test/cases/callbacks_test.rb" "test/cases/clone_test.rb" "test/cases/coders/json_test.rb" "test/cases/coders/yaml_column_test.rb" "test/cases/collection_cache_key_test.rb" "test/cases/column_alias_test.rb" "test/cases/column_definition_test.rb" "test/cases/comment_test.rb" "test/cases/connection_adapters/adapter_leasing_test.rb" "test/cases/connection_adapters/connection_handler_test.rb" "test/cases/connection_adapters/connection_handlers_multi_db_test.rb" "test/cases/connection_adapters/connection_handlers_multi_pool_config_test.rb" "test/cases/connection_adapters/connection_handlers_sharding_db_test.rb" "test/cases/connection_adapters/connection_swapping_nested_test.rb" "test/cases/connection_adapters/merge_and_resolve_default_url_config_test.rb" "test/cases/connection_adapters/mysql_type_lookup_test.rb" "test/cases/connection_adapters/schema_cache_test.rb" "test/cases/connection_adapters/standalone_connection_test.rb" "test/cases/connection_adapters/type_lookup_test.rb" "test/cases/connection_management_test.rb" "test/cases/connection_pool_test.rb" "test/cases/core_test.rb" "test/cases/counter_cache_test.rb" "test/cases/custom_locking_test.rb" "test/cases/database_configurations/hash_config_test.rb" "test/cases/database_configurations/resolver_test.rb" "test/cases/database_configurations_test.rb" "test/cases/database_selector_test.rb" "test/cases/database_statements_test.rb" "test/cases/date_test.rb" "test/cases/date_time_precision_test.rb" "test/cases/date_time_test.rb" "test/cases/defaults_test.rb" "test/cases/delegated_type_test.rb" "test/cases/dirty_test.rb" "test/cases/disconnected_test.rb" "test/cases/dup_test.rb" "test/cases/encryption/cipher/aes256_gcm_test.rb" "test/cases/encryption/cipher_test.rb" "test/cases/encryption/concurrency_test.rb" "test/cases/encryption/config_test.rb" "test/cases/encryption/configurable_test.rb" "test/cases/encryption/contexts_test.rb" "test/cases/encryption/derived_secret_key_provider_test.rb" "test/cases/encryption/deterministic_key_provider_test.rb" "test/cases/encryption/encryptable_record_api_test.rb" "test/cases/encryption/encryptable_record_test.rb" "test/cases/encryption/encrypted_fixtures_test.rb" "test/cases/encryption/encrypting_only_encryptor_test.rb" "test/cases/encryption/encryption_schemes_test.rb" "test/cases/encryption/encryptor_test.rb" "test/cases/encryption/envelope_encryption_key_provider_test.rb" "test/cases/encryption/extended_deterministic_queries_test.rb" "test/cases/encryption/key_generator_test.rb" "test/cases/encryption/key_provider_test.rb" "test/cases/encryption/key_test.rb" "test/cases/encryption/message_serializer_test.rb" "test/cases/encryption/message_test.rb" "test/cases/encryption/null_encryptor_test.rb" "test/cases/encryption/properties_test.rb" "test/cases/encryption/read_only_null_encryptor_test.rb" "test/cases/encryption/scheme_test.rb" "test/cases/encryption/unencrypted_attributes_test.rb" "test/cases/encryption/uniqueness_validations_test.rb" "test/cases/enum_test.rb" "test/cases/errors_test.rb" "test/cases/excluding_test.rb" "test/cases/explain_subscriber_test.rb" "test/cases/explain_test.rb" "test/cases/filter_attributes_test.rb" "test/cases/finder_respond_to_test.rb" "test/cases/finder_test.rb" "test/cases/fixture_set/file_test.rb" "test/cases/fixtures_test.rb" "test/cases/forbidden_attributes_protection_test.rb" "test/cases/habtm_destroy_order_test.rb" "test/cases/hot_compatibility_test.rb" "test/cases/i18n_test.rb" "test/cases/inheritance_test.rb" "test/cases/insert_all_test.rb" "test/cases/instrumentation_test.rb" "test/cases/integration_test.rb" "test/cases/invalid_connection_test.rb" "test/cases/invertible_migration_test.rb" "test/cases/json_attribute_test.rb" "test/cases/json_serialization_test.rb" "test/cases/locking_test.rb" "test/cases/log_subscriber_test.rb" "test/cases/marshal_serialization_test.rb" "test/cases/migration/change_schema_test.rb" "test/cases/migration/change_table_test.rb" "test/cases/migration/check_constraint_test.rb" "test/cases/migration/column_attributes_test.rb" "test/cases/migration/column_positioning_test.rb" "test/cases/migration/columns_test.rb" "test/cases/migration/command_recorder_test.rb" "test/cases/migration/compatibility_test.rb" "test/cases/migration/create_join_table_test.rb" "test/cases/migration/exclusion_constraint_test.rb" "test/cases/migration/foreign_key_test.rb" "test/cases/migration/index_test.rb" "test/cases/migration/logger_test.rb" "test/cases/migration/pending_migrations_test.rb" "test/cases/migration/references_foreign_key_test.rb" "test/cases/migration/references_index_test.rb" "test/cases/migration/references_statements_test.rb" "test/cases/migration/rename_table_test.rb" "test/cases/migration_test.rb" "test/cases/migrator_test.rb" "test/cases/mixin_test.rb" "test/cases/modules_test.rb" "test/cases/multi_db_migrator_test.rb" "test/cases/multiparameter_attributes_test.rb" "test/cases/multiple_db_test.rb" "test/cases/nested_attributes_test.rb" "test/cases/nested_attributes_with_callbacks_test.rb" "test/cases/null_relation_test.rb" "test/cases/numeric_data_test.rb" "test/cases/persistence_test.rb" "test/cases/pooled_connections_test.rb" "test/cases/prepared_statement_status_test.rb" "test/cases/primary_class_test.rb" "test/cases/primary_keys_test.rb" "test/cases/query_cache_test.rb" "test/cases/query_logs_test.rb" "test/cases/quoting_test.rb" "test/cases/readonly_test.rb" "test/cases/reaper_test.rb" "test/cases/reflection_test.rb" "test/cases/relation/and_test.rb" "test/cases/relation/delegation_test.rb" "test/cases/relation/delete_all_test.rb" "test/cases/relation/field_ordered_values_test.rb" "test/cases/relation/load_async_test.rb" "test/cases/relation/merging_test.rb" "test/cases/relation/mutation_test.rb" "test/cases/relation/or_test.rb" "test/cases/relation/predicate_builder_test.rb" "test/cases/relation/record_fetch_warning_test.rb" "test/cases/relation/select_test.rb" "test/cases/relation/structural_compatibility_test.rb" "test/cases/relation/update_all_test.rb" "test/cases/relation/where_chain_test.rb" "test/cases/relation/where_clause_test.rb" "test/cases/relation/where_test.rb" "test/cases/relation/with_test.rb" "test/cases/relation_test.rb" "test/cases/relations_test.rb" "test/cases/reload_models_test.rb" "test/cases/reserved_word_test.rb" "test/cases/result_test.rb" "test/cases/sanitize_test.rb" "test/cases/schema_dumper_test.rb" "test/cases/schema_loading_test.rb" "test/cases/scoping/default_scoping_test.rb" "test/cases/scoping/named_scoping_test.rb" "test/cases/scoping/relation_scoping_test.rb" "test/cases/secure_password_test.rb" "test/cases/secure_token_test.rb" "test/cases/serialization_test.rb" "test/cases/serialized_attribute_test.rb" "test/cases/shard_selector_test.rb" "test/cases/signed_id_test.rb" "test/cases/statement_cache_test.rb" "test/cases/statement_invalid_test.rb" "test/cases/store_test.rb" "test/cases/strict_loading_test.rb" "test/cases/suppressor_test.rb" "test/cases/tasks/database_tasks_test.rb" "test/cases/tasks/mysql_rake_test.rb" "test/cases/tasks/postgresql_rake_test.rb" "test/cases/tasks/sqlite_rake_test.rb" "test/cases/test_databases_test.rb" "test/cases/test_fixtures_test.rb" "test/cases/time_precision_test.rb" "test/cases/timestamp_test.rb" "test/cases/token_for_test.rb" "test/cases/touch_later_test.rb" "test/cases/transaction_callbacks_test.rb" "test/cases/transaction_isolation_test.rb" "test/cases/transactions_test.rb" "test/cases/type/adapter_specific_registry_test.rb" "test/cases/type/date_time_test.rb" "test/cases/type/integer_test.rb" "test/cases/type/string_test.rb" "test/cases/type/time_test.rb" "test/cases/type/type_map_test.rb" "test/cases/type/unsigned_integer_test.rb" "test/cases/type_test.rb" "test/cases/types_test.rb" "test/cases/unconnected_test.rb" "test/cases/unsafe_raw_sql_test.rb" "test/cases/validations/absence_validation_test.rb" "test/cases/validations/association_validation_test.rb" "test/cases/validations/i18n_generate_message_validation_test.rb" "test/cases/validations/i18n_validation_test.rb" "test/cases/validations/length_validation_test.rb" "test/cases/validations/numericality_validation_test.rb" "test/cases/validations/presence_validation_test.rb" "test/cases/validations/uniqueness_validation_test.rb" "test/cases/validations_test.rb" "test/cases/view_test.rb" "test/cases/yaml_serialization_test.rb" "test/cases/adapters/postgresql/active_schema_test.rb" "test/cases/adapters/postgresql/array_test.rb" "test/cases/adapters/postgresql/bind_parameter_test.rb" "test/cases/adapters/postgresql/bit_string_test.rb" "test/cases/adapters/postgresql/bytea_test.rb" "test/cases/adapters/postgresql/case_insensitive_test.rb" "test/cases/adapters/postgresql/change_schema_test.rb" "test/cases/adapters/postgresql/cidr_test.rb" "test/cases/adapters/postgresql/citext_test.rb" "test/cases/adapters/postgresql/collation_test.rb" "test/cases/adapters/postgresql/composite_test.rb" "test/cases/adapters/postgresql/connection_test.rb" "test/cases/adapters/postgresql/create_unlogged_tables_test.rb" "test/cases/adapters/postgresql/datatype_test.rb" "test/cases/adapters/postgresql/date_test.rb" "test/cases/adapters/postgresql/domain_test.rb" "test/cases/adapters/postgresql/enum_test.rb" "test/cases/adapters/postgresql/explain_test.rb" "test/cases/adapters/postgresql/extension_migration_test.rb" "test/cases/adapters/postgresql/foreign_table_test.rb" "test/cases/adapters/postgresql/full_text_test.rb" "test/cases/adapters/postgresql/geometric_test.rb" "test/cases/adapters/postgresql/hstore_test.rb" "test/cases/adapters/postgresql/infinity_test.rb" "test/cases/adapters/postgresql/integer_test.rb" "test/cases/adapters/postgresql/interval_test.rb" "test/cases/adapters/postgresql/invertible_migration_test.rb" "test/cases/adapters/postgresql/json_test.rb" "test/cases/adapters/postgresql/ltree_test.rb" "test/cases/adapters/postgresql/money_test.rb" "test/cases/adapters/postgresql/network_test.rb" "test/cases/adapters/postgresql/numbers_test.rb" "test/cases/adapters/postgresql/optimizer_hints_test.rb" "test/cases/adapters/postgresql/partitions_test.rb" "test/cases/adapters/postgresql/postgresql_adapter_prevent_writes_test.rb" "test/cases/adapters/postgresql/postgresql_adapter_test.rb" "test/cases/adapters/postgresql/prepared_statements_disabled_test.rb" "test/cases/adapters/postgresql/quoting_test.rb" "test/cases/adapters/postgresql/range_test.rb" "test/cases/adapters/postgresql/referential_integrity_test.rb" "test/cases/adapters/postgresql/rename_table_test.rb" "test/cases/adapters/postgresql/schema_authorization_test.rb" "test/cases/adapters/postgresql/schema_test.rb" "test/cases/adapters/postgresql/serial_test.rb" "test/cases/adapters/postgresql/statement_pool_test.rb" "test/cases/adapters/postgresql/timestamp_test.rb" "test/cases/adapters/postgresql/transaction_nested_test.rb" "test/cases/adapters/postgresql/transaction_test.rb" "test/cases/adapters/postgresql/type_lookup_test.rb" "test/cases/adapters/postgresql/utils_test.rb" "test/cases/adapters/postgresql/uuid_test.rb" "test/cases/adapters/postgresql/virtual_column_test.rb" "test/cases/adapters/postgresql/xml_test.rb" --seed 31890

@yahonda
Copy link
Member

yahonda commented Jul 15, 2022

@yahonda
Copy link
Member

yahonda commented Jul 15, 2022

Reproduced the second one https://buildkite.com/rails/rails/builds/88090#01820091-8d96-4e94-8485-debf4a54eaae

needs fixed, It may be better to replace :parrots with something else that does not have foreign keys referred.

% ARCONN=postgresql bin/test test/cases/fixtures_test.rb test/cases/adapters/postgresql/referential_integrity_test.rb -n "/^(?:ActiveSupportSubclassWithFixturesTest#(?:test_foo)|PostgreSQLReferentialIntegrityTest#(?:test_all_foreign_keys_valid_having_foreign_keys_in_multiple_schemas))$/" --seed 49007
Using postgresql
Run options: -n "/^(?:ActiveSupportSubclassWithFixturesTest#(?:test_foo)|PostgreSQLReferentialIntegrityTest#(?:test_all_foreign_keys_valid_having_foreign_keys_in_multiple_schemas))$/" --seed 49007

# Running:

.F

Failure:
PostgreSQLReferentialIntegrityTest#test_all_foreign_keys_valid_having_foreign_keys_in_multiple_schemas [/Users/yahonda/src/github.com/rails/rails/activerecord/test/cases/adapters/postgresql/referential_integrity_test.rb:129]:
Expected false to be truthy.


bin/test test/cases/adapters/postgresql/referential_integrity_test.rb:108



Finished in 0.081365s, 24.5806 runs/s, 36.8709 assertions/s.
2 runs, 3 assertions, 1 failures, 0 errors, 0 skips
%

@yahonda
Copy link
Member

yahonda commented Jul 15, 2022

Reproduced https://buildkite.com/rails/rails/builds/88090#01820091-8d39-4197-8b7d-257b29405a55

fixtures = ActiveRecord::FixtureSet.create_fixtures(FIXTURES_ROOT, "parrots")

also needs fixed, It may be better to replace :parrots with something else that does not have foreign keys referred.

$ ARCONN=postgresql bin/test test/cases/fixtures_test.rb test/cases/adapters/postgresql/referential_integrity_test.rb -n "/^(?:FixturesTest#(?:test_create_fixtures)|PostgreSQLReferentialIntegrityTest#(?:test_all_foreign_keys_valid_having_foreign_keys_in_multiple_schemas))$/" --seed 64716
Using postgresql
Run options: -n "/^(?:FixturesTest#(?:test_create_fixtures)|PostgreSQLReferentialIntegrityTest#(?:test_all_foreign_keys_valid_having_foreign_keys_in_multiple_schemas))$/" --seed 64716

# Running:

.F

Failure:
PostgreSQLReferentialIntegrityTest#test_all_foreign_keys_valid_having_foreign_keys_in_multiple_schemas [/home/yahonda/src/github.com/rails/rails/activerecord/test/cases/adapters/postgresql/referential_integrity_test.rb:129]:
Expected false to be truthy.


bin/test test/cases/adapters/postgresql/referential_integrity_test.rb:108



Finished in 0.243139s, 8.2258 runs/s, 16.4515 assertions/s.
2 runs, 4 assertions, 1 failures, 0 errors, 0 skips
$

@lxxxvi
Copy link
Contributor Author

lxxxvi commented Jul 15, 2022

@yahonda

Firstly, thank you for showing me how to track down the issues using minitest_bisect. I copy/pasted your example, but on my machine I could not reproduce it, it said Reproduction run passed? Aborting. I'll try to figure out why that could be.

Secondly, I addressed the two issues that you reproduced. I replaced the usage of parrots with admin/users.

Lastly, could there be more/other places that makes the test #test_all_foreign_keys_valid_having_foreign_keys_in_multiple_schemas fail? Would it make sense to add an assert at the beginning of that test just to ensure that the initial state is "clean"? Something like:

def test_all_foreign_keys_valid_having_foreign_keys_in_multiple_schemas
  assert @connection.all_foreign_keys_valid?, "Foreign keys should be valid before this test"

  # ...
end

@yahonda
Copy link
Member

yahonda commented Jul 19, 2022

Thanks for the update.

Firstly, thank you for showing me how to track down the issues using minitest_bisect. I copy/pasted your example, but on my machine I could not reproduce it, it said Reproduction run passed? Aborting. I'll try to figure out why that could be.

There may be some environment specific differences, you do not need to reproduce them because all of them have been reproduced in my environment and your commit addresses them.

Secondly, I addressed the two issues that you reproduced. I replaced the usage of parrots with admin/users.

Taking a look at activerecord_unittest database and searching for tables that are not referenced by other tables and whose corresponding fixtures exists but no other good fixtures are found so admin_users should be fine.

Lastly, could there be more/other places that makes the test #test_all_foreign_keys_valid_having_foreign_keys_in_multiple_schemas fail? Would it make sense to add an assert at the beginning of that test just to ensure that the initial state is "clean"? Something like:

git grep may help you to find the similar cases.

% git grep -n 'fixtures :' |grep :parrots
test/cases/associations_test.rb:1055:  fixtures :pirates, :parrots
test/cases/fixtures_test.rb:1174:  fixtures :parrots, :parrots_pirates, :pirates, :treasures, :mateys, :ships, :computers,
test/cases/fixtures_test.rb:1394:  fixtures :other_books, :parrots, :parrots_pirates, :pirates, :treasures
%

I think this line needs to add :parrots_pirates, :pirates, :treasures fixtures.

fixtures :pirates, :parrots

Here are steps to reproduce.

% ARCONN=postgresql bin/test test/cases/associations_test.rb test/cases/adapters/postgresql/referential_integrity_test.rb --seed 34646 -n "/^(?:WithAnnotationsTest#(?:test_has_many_through_with_annotation_includes_a_query_comment_when_eager_loading)|PostgreSQLReferentialIntegrityTest#(?:test_all_foreign_keys_valid_having_foreign_keys_in_multiple_schemas))$/"
Using postgresql
Run options: --seed 34646 -n "/^(?:WithAnnotationsTest#(?:test_has_many_through_with_annotation_includes_a_query_comment_when_eager_loading)|PostgreSQLReferentialIntegrityTest#(?:test_all_foreign_keys_valid_having_foreign_keys_in_multiple_schemas))$/"

# Running:

.F

Failure:
PostgreSQLReferentialIntegrityTest#test_all_foreign_keys_valid_having_foreign_keys_in_multiple_schemas [/Users/yahonda/src/github.com/rails/rails/activerecord/test/cases/adapters/postgresql/referential_integrity_test.rb:129]:
Expected false to be truthy.


bin/test test/cases/adapters/postgresql/referential_integrity_test.rb:108



Finished in 0.154817s, 12.9185 runs/s, 45.2147 assertions/s.
2 runs, 7 assertions, 1 failures, 0 errors, 0 skips
%

No need to add assertions to make sure the initial state is "clean".
Regardless which test causes foreign key violation, test_all_foreign_keys_valid_having_foreign_keys_in_multiple_schemas find them that is fine.

@yahonda
Copy link
Member

yahonda commented Jul 19, 2022

I think this line needs to add :parrots_pirates, :pirates, :treasures fixtures.

I mean to say this change.

% git diff
diff --git a/activerecord/test/cases/associations_test.rb b/activerecord/test/cases/associations_test.rb
index 19e0bf9e0b..9a98485c84 100644
--- a/activerecord/test/cases/associations_test.rb
+++ b/activerecord/test/cases/associations_test.rb
@@ -1052,7 +1052,7 @@ def test_included_module_overwrites_association_methods
 end

 class WithAnnotationsTest < ActiveRecord::TestCase
-  fixtures :pirates, :parrots
+  fixtures :pirates, :parrots, :parrots_pirates, :pirates, :treasures

   def test_belongs_to_with_annotation_includes_a_query_comment
     pirate = SpacePirate.where.not(parrot_id: nil).first
%

@lxxxvi lxxxvi force-pushed the fix_rails_issue_db_fixtures_load_raises branch from fe144b2 to caea305 Compare July 19, 2022 12:20
@lxxxvi
Copy link
Contributor Author

lxxxvi commented Jul 19, 2022

@yahonda

Thank you once again. 🙂
I pushed the changes you suggested. Let me know if I can do more.

@yahonda
Copy link
Member

yahonda commented Jul 19, 2022

Only one possible concern is using admin_users fixtures whose model has namespace Admin::User. I've asked feedbacks from other committers. Please give me some more time to have conclusions.

@lxxxvi
Copy link
Contributor Author

lxxxvi commented Jul 19, 2022

No rush.

In order to keep the changes to a minimum I looked for fixtures having

  • a column :name
  • no references to other models

There were only a few candidates:

  • admin/users.yml
  • funny_jokes.yml
  • organizations.yml
  • trees.yml (has a self reference, but that's fine, I guess)

My second choice would have been organizations.yml. I'm looking forward to hearing back from you (and other commiters).

@yahonda
Copy link
Member

yahonda commented Jul 19, 2022

I was not aware of organizations.yml that looks good to me.
Would you replace admin/users with organizations?

@lxxxvi lxxxvi force-pushed the fix_rails_issue_db_fixtures_load_raises branch 2 times, most recently from c3def4b to 8ce0d47 Compare July 19, 2022 15:19
@lxxxvi
Copy link
Contributor Author

lxxxvi commented Jul 19, 2022

👍 I replaced admin/users with organizations. I had to require "models/organization" to make the two affected tests work, I hope that's fine.

@yahonda
Copy link
Member

yahonda commented Jul 19, 2022

Looks good. Would you squash commits?

…gres

In Postgres there was an issue when calling `ActiveRecord::ConnectionAdapters::PostgreSQL::ReferentialIntegrity#all_foreign_keys_valid?` if there were multiple schemas with foreign key constraints, because the ALTER TABLE statement did not include the schema.

This commit adds the necessary schema prefix

```
ALTER TABLE [schema].[table_name] VALIDATE CONSTRAINT [constraint]
             ^^^^^^
```
@lxxxvi lxxxvi force-pushed the fix_rails_issue_db_fixtures_load_raises branch from 8ce0d47 to e3bb10d Compare July 19, 2022 16:03
@lxxxvi
Copy link
Contributor Author

lxxxvi commented Jul 19, 2022

👍 Squashed into 1 commit.

@yahonda yahonda merged commit f07d080 into rails:main Jul 19, 2022
@yahonda
Copy link
Member

yahonda commented Jul 19, 2022

Great work. Thanks.

@lxxxvi
Copy link
Contributor Author

lxxxvi commented Jul 19, 2022

I thank you. I learned a lot.

yahonda added a commit to yahonda/rails that referenced this pull request Jul 19, 2022
…s_load_raises

`db:fixtures:load` raises in Postgres if there are foreign key constraints in a non-`public` schema
@yahonda
Copy link
Member

yahonda commented Jul 19, 2022

Backported to 7-0-stable branch via aed8afa

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.

None yet

2 participants