teoljungberg / fx

Versioned database functions and triggers for Rails
MIT License
763 stars 77 forks source link

Schema dump fails in postgis #34

Closed maheshm closed 4 years ago

maheshm commented 5 years ago

https://github.com/teoljungberg/fx/blob/53f8b3fb651e7574f98f6ff3685361a13de0981c/lib/fx/adapters/postgres/functions.rb#L11

This throws the following error if you have aggregate functions (Postgis has a lot of these):

ActiveRecord::StatementInvalid: PG::WrongObjectType: ERROR:  "array_accum" is an aggregate function
:           SELECT
              pp.proname AS name,
              pg_get_functiondef(pp.oid) AS definition
          FROM pg_proc pp
          JOIN pg_namespace pn
              ON pn.oid = pp.pronamespace
          LEFT JOIN pg_depend pd
              ON pd.objid = pp.oid AND pd.deptype = 'e'
          WHERE pn.nspname = 'public' AND pd.objid IS NULL
          ORDER BY pp.oid;

Stacktrace if that helps:

/bundle/gems/activerecord-5.1.6.2/lib/active_record/connection_adapters/postgresql/database_statements.rb:73:in `async_exec'
/bundle/gems/activerecord-5.1.6.2/lib/active_record/connection_adapters/postgresql/database_statements.rb:73:in `block (2 levels) in execute'
/bundle/gems/activesupport-5.1.6.2/lib/active_support/dependencies/interlock.rb:46:in `block in permit_concurrent_loads'
/bundle/gems/activesupport-5.1.6.2/lib/active_support/concurrency/share_lock.rb:185:in `yield_shares'
/bundle/gems/activesupport-5.1.6.2/lib/active_support/dependencies/interlock.rb:45:in `permit_concurrent_loads'
/bundle/gems/activerecord-5.1.6.2/lib/active_record/connection_adapters/postgresql/database_statements.rb:72:in `block in execute'
/bundle/gems/activerecord-5.1.6.2/lib/active_record/connection_adapters/abstract_adapter.rb:613:in `block (2 levels) in log'
/bundle/gems/activerecord-5.1.6.2/lib/active_record/connection_adapters/abstract_adapter.rb:612:in `block in log'
/bundle/gems/activesupport-5.1.6.2/lib/active_support/notifications/instrumenter.rb:21:in `instrument'
/bundle/gems/activerecord-5.1.6.2/lib/active_record/connection_adapters/abstract_adapter.rb:604:in `log'
/bundle/gems/activerecord-5.1.6.2/lib/active_record/connection_adapters/postgresql/database_statements.rb:71:in `execute'
/bundle/gems/fx-0.5.0/lib/fx/adapters/postgres/functions.rb:47:in `functions_from_postgres'
/bundle/gems/fx-0.5.0/lib/fx/adapters/postgres/functions.rb:39:in `all'
/bundle/gems/fx-0.5.0/lib/fx/adapters/postgres/functions.rb:28:in `all'
/bundle/gems/fx-0.5.0/lib/fx/adapters/postgres.rb:50:in `functions'
/bundle/gems/fx-0.5.0/lib/fx/schema_dumper/function.rb:25:in `dumpable_functions_in_database'
/bundle/gems/fx-0.5.0/lib/fx/schema_dumper/function.rb:13:in `functions'
/bundle/gems/fx-0.5.0/lib/fx/schema_dumper/function.rb:9:in `tables'
/bundle/gems/fx-0.5.0/lib/fx/schema_dumper/trigger.rb:8:in `tables'
/bundle/gems/scenic-1.5.1/lib/scenic/schema_dumper.rb:7:in `tables'
/bundle/gems/activerecord-5.1.6.2/lib/active_record/schema_dumper.rb:37:in `dump'
/bundle/gems/activerecord-5.1.6.2/lib/active_record/schema_dumper.rb:21:in `dump'
/bundle/gems/activerecord-5.1.6.2/lib/active_record/railties/databases.rake:241:in `block (4 levels) in <top (required)>'
/bundle/gems/activerecord-5.1.6.2/lib/active_record/railties/databases.rake:240:in `open'
/bundle/gems/activerecord-5.1.6.2/lib/active_record/railties/databases.rake:240:in `block (3 levels) in <top (required)>'
/bundle/gems/activerecord-5.1.6.2/lib/active_record/railties/databases.rake:66:in `block (2 levels) in <top (required)>'
/bundle/gems/activerecord-5.1.6.2/lib/active_record/railties/databases.rake:59:in `block (2 levels) in <top (required)>'
/bundle/gems/railties-5.1.6.2/lib/rails/commands/rake/rake_command.rb:21:in `block in perform'
/bundle/gems/railties-5.1.6.2/lib/rails/commands/rake/rake_command.rb:18:in `perform'
/bundle/gems/railties-5.1.6.2/lib/rails/command.rb:46:in `invoke'
/bundle/gems/railties-5.1.6.2/lib/rails/commands.rb:16:in `<top (required)>'
bin/rails:4:in `require'
bin/rails:4:in `<main>'

Basically pg_get_functiondef() fails for aggregate functions.

maheshm commented 5 years ago
WHERE pn.nspname = 'public' AND pd.objid IS null and pp.prosrc != 'aggregate_dummy'

This seems to work for me, but not sure how this will behave in non-postgis DB.

maheshm commented 5 years ago

It will be great if someone can help me test the above on normal Postgres and I can raise a PR with the above change

vladimirtemnikov commented 5 years ago
WHERE pn.nspname = 'public' AND pd.objid IS NULL AND pp.prokind != 'a'
teoljungberg commented 5 years ago

I’d happily welcome a PR that fixes the error

vladimirtemnikov commented 5 years ago

@teoljungberg just created one :)

vladimirtemnikov commented 5 years ago

So it's ready for various versions of PG now.

bradrobertson commented 5 years ago

So, we don't have postgis, but we do have aggregate functions, and I can confirm the current query doesn't work, but adding AND pp.prokind != 'a' seems to dump it fine. What's the general testing protocol for a change like this?

bradrobertson commented 5 years ago

Another option btw would be AND pp.prokind = 'f' https://www.postgresql.org/docs/11/catalog-pg-proc.html

According to this:

f for a normal function, p for a procedure, a for an aggregate function, or w for a window function

However I'm not sure the scope of this gem (if it should dump things other than normal functions)

bradrobertson commented 5 years ago

Sorry for the multiple messages. It should also be noted that pg_proc.prokind is only available on Postgres 11+

teoljungberg commented 5 years ago

@bradrobertson The opened PR should resolve the issue on postgres 10 and 11. I hope.

Can you shed any light on this @vladimirtemnikov ?

vladimirtemnikov commented 5 years ago

@teoljungberg I'm sure it's the same problem which my PR resolves. I'll try to finish PR soon.

teoljungberg commented 4 years ago

This seems to be fixed by #50.