PG 10.x Foreign Key Constraints

[cjolly]

Since moving to PG 10, FKs are not getting public. schema name cleaned up:

-ALTER TABLE ONLY dude_logs
-    ADD CONSTRAINT fk_rails_a1ad125ad2 FOREIGN KEY (dude_id) REFERENCES dudes(id);
+ALTER TABLE ONLY public.dude_logs
+    ADD CONSTRAINT fk_rails_a1ad125ad2 FOREIGN KEY (dude_id) REFERENCES public.dudes(id);

Another minor addition with search_path setting at top of file.

+SELECT pg_catalog.set_config('search_path', '', false);

I can certainly give a shot fixing this, but my regex-fu is admittedly a bit rusty, I might just cause us both more work!

[lfittl]

Good catch, let me see if I can get that fixed today

[cjolly]

@lfittl I actually believe this is happening with all previously cleaned public. instances, not just the FKs.

[lfittl]

@cjolly Ah, so I've looked at this again and I don't think we should be cleaning those schema names up.

For additional context: The security change in 10.3 and 9.6.8 is whats causing public. to be added, to ensure any object referenced is the actual one in the public schema and not an injected object in another schema by a less privileged user.

Since that change is in the client tools the problem you are likely running into is that different developers in your team have different Postgres patch versions installed locally. First of all I think the best fix in practice is to have everyone upgrade to the latest patch release locally.

Second, if we make a change in this library, I would actually suggest we adjust it to always add public. (as well as the search path change), so we generate the same output even on old versions.

That is a bit more work though, which I personally can't invest in right now. Patches welcome of course :)

[cjolly]

Aha. That make a lot of sense... as I couldn't really figure out where it was being stripped before in order to attempt a pull request. Makes sense now - cause it wasn't!

Not sure if you've seen it - but while investigating this I ran across https://github.com/rails/rails/pull/28154 - it looks like rails 5.2 has been making some attempts to normalize this file by removing all comment lines, too.

So, I guess we can call this Issue closed, as PG is very intentionally adding that schema namespace. I'll let you decide if you want to actually close it or if you want it open and visible for a while.

Thanks for taking a look!

[lfittl]

Sounds good, I'll close this issue - thanks!

[giovannibonetti]

Would you be interested in a PR to optionally remove the public. prefixes?

if options[:remove_public_prefix] == true
  dump.gsub!(/\bpublic\.(\w+)/, '\1')
  dump.gsub!(/^.+set_config.+search_path.+$/, '')
end

[lfittl]

@giovannibonetti Can you describe the use case for it?

As mentioned upthread, its actually a good thing when the objects in the schema are fully-qualified, as it avoids potential mistakes (and in some rare cases has security implications)

[giovannibonetti]

All right, I've read the discussion in detail now. I understand why the issue is closed. Nevermind.