diff options
author | Aaron Patterson <aaron.patterson@gmail.com> | 2011-06-21 10:22:20 -0700 |
---|---|---|
committer | Aaron Patterson <aaron.patterson@gmail.com> | 2011-06-21 10:22:20 -0700 |
commit | ceb5bbbafc642f835a30b2dd967879b1e719a511 (patch) | |
tree | 96958c538f536c234741abe8500e87fe46f77315 | |
parent | e369682ef1c405cab5cfa2f4066b7a0ca74a71fc (diff) | |
parent | 2b3d67fdaf2c78a5447316a90c54c7c9c520c71a (diff) | |
download | rails-ceb5bbbafc642f835a30b2dd967879b1e719a511.tar.gz rails-ceb5bbbafc642f835a30b2dd967879b1e719a511.tar.bz2 rails-ceb5bbbafc642f835a30b2dd967879b1e719a511.zip |
Merge pull request #1632 from tardate/pg_schema_fu
Improving PostgreSQL adapter schema-awareness
4 files changed, 285 insertions, 50 deletions
diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index 1a24ed1375..6d638628df 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -466,10 +466,11 @@ module ActiveRecord # Executes an INSERT query and returns the new record's ID def insert_sql(sql, name = nil, pk = nil, id_value = nil, sequence_name = nil) - # Extract the table from the insert sql. Yuck. - _, table = extract_schema_and_table(sql.split(" ", 4)[2]) - - pk ||= primary_key(table) + unless pk + # Extract the table from the insert sql. Yuck. + table_ref = extract_table_ref_from_insert_sql(sql) + pk = primary_key(table_ref) if table_ref + end if pk select_value("#{sql} RETURNING #{quote_column_name(pk)}") @@ -565,9 +566,9 @@ module ActiveRecord def sql_for_insert(sql, pk, id_value, sequence_name, binds) unless pk - _, table = extract_schema_and_table(sql.split(" ", 4)[2]) - - pk = primary_key(table) + # Extract the table from the insert sql. Yuck. + table_ref = extract_table_ref_from_insert_sql(sql) + pk = primary_key(table_ref) if table_ref end sql = "#{sql} RETURNING #{quote_column_name(pk)}" if pk @@ -665,34 +666,33 @@ module ActiveRecord SQL end + # Returns true if table exists. + # If the schema is not specified as part of +name+ then it will only find tables within + # the current schema search path (regardless of permissions to access tables in other schemas) def table_exists?(name) schema, table = extract_schema_and_table(name.to_s) + return false unless table - binds = [[nil, table.gsub(/(^"|"$)/,'')]] + binds = [[nil, table]] binds << [nil, schema] if schema exec_query(<<-SQL, 'SCHEMA', binds).rows.first[0].to_i > 0 - SELECT COUNT(*) - FROM pg_tables - WHERE tablename = $1 - #{schema ? "AND schemaname = $2" : ''} + SELECT COUNT(*) + FROM pg_class c + LEFT JOIN pg_namespace n ON n.oid = c.relnamespace + WHERE c.relkind in ('v','r') + AND c.relname = $1 + AND n.nspname = #{schema ? '$2' : 'ANY (current_schemas(false))'} SQL end - # Extracts the table and schema name from +name+ - def extract_schema_and_table(name) - schema, table = name.split('.', 2) - - unless table # A table was provided without a schema - table = schema - schema = nil - end - - if name =~ /^"/ # Handle quoted table names - table = name - schema = nil - end - [schema, table] + # Returns true if schema exists. + def schema_exists?(name) + exec_query(<<-SQL, 'SCHEMA', [[nil, name]]).rows.first[0].to_i > 0 + SELECT COUNT(*) + FROM pg_namespace + WHERE nspname = $1 + SQL end # Returns an array of indexes for the given table. @@ -742,6 +742,11 @@ module ActiveRecord query('select current_database()')[0][0] end + # Returns the current schema name. + def current_schema + query('SELECT current_schema', 'SCHEMA')[0][0] + end + # Returns the current database encoding format. def encoding query(<<-end_sql)[0][0] @@ -960,27 +965,27 @@ module ActiveRecord end private - def exec_no_cache(sql, binds) - @connection.async_exec(sql) - end - - def exec_cache(sql, binds) - unless @statements.key? sql - nextkey = "a#{@statements.length + 1}" - @connection.prepare nextkey, sql - @statements[sql] = nextkey + def exec_no_cache(sql, binds) + @connection.async_exec(sql) end - key = @statements[sql] + def exec_cache(sql, binds) + unless @statements.key? sql + nextkey = "a#{@statements.length + 1}" + @connection.prepare nextkey, sql + @statements[sql] = nextkey + end - # Clear the queue - @connection.get_last_result - @connection.send_query_prepared(key, binds.map { |col, val| - type_cast(val, col) - }) - @connection.block - @connection.get_last_result - end + key = @statements[sql] + + # Clear the queue + @connection.get_last_result + @connection.send_query_prepared(key, binds.map { |col, val| + type_cast(val, col) + }) + @connection.block + @connection.get_last_result + end # The internal PostgreSQL identifier of the money data type. MONEY_COLUMN_TYPE_OID = 790 #:nodoc: @@ -1080,9 +1085,29 @@ module ActiveRecord end end - def table_definition - TableDefinition.new(self) - end + # Returns an array of <tt>[schema_name, table_name]</tt> extracted from +name+. + # +schema_name+ is nil if not specified in +name+. + # +schema_name+ and +table_name+ exclude surrounding quotes (regardless of whether provided in +name+) + # +name+ supports the range of schema/table references understood by PostgreSQL, for example: + # + # * <tt>table_name</tt> + # * <tt>"table.name"</tt> + # * <tt>schema_name.table_name</tt> + # * <tt>schema_name."table.name"</tt> + # * <tt>"schema.name"."table name"</tt> + def extract_schema_and_table(name) + table, schema = name.scan(/[^".\s]+|"[^"]*"/)[0..1].collect{|m| m.gsub(/(^"|"$)/,'') }.reverse + [schema, table] + end + + def extract_table_ref_from_insert_sql(sql) + sql[/into\s+([^\(]*).*values\s*\(/i] + $1.strip if $1 + end + + def table_definition + TableDefinition.new(self) + end end end end diff --git a/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb b/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb index 7c49236854..d57794daf8 100644 --- a/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb +++ b/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb @@ -10,6 +10,45 @@ module ActiveRecord @connection.exec_query('create table ex(id serial primary key, number integer, data character varying(255))') end + def test_primary_key + assert_equal 'id', @connection.primary_key('ex') + end + + def test_non_standard_primary_key + @connection.exec_query('drop table if exists ex') + @connection.exec_query('create table ex(data character varying(255) primary key)') + assert_equal 'data', @connection.primary_key('ex') + end + + def test_primary_key_returns_nil_for_no_pk + @connection.exec_query('drop table if exists ex') + @connection.exec_query('create table ex(id integer)') + assert_nil @connection.primary_key('ex') + end + + def test_primary_key_raises_error_if_table_not_found + assert_raises(ActiveRecord::StatementInvalid) do + @connection.primary_key('unobtainium') + end + end + + def test_insert_sql_with_proprietary_returning_clause + id = @connection.insert_sql("insert into ex (number) values(5150)", nil, "number") + assert_equal "5150", id + end + + def test_insert_sql_with_quoted_schema_and_table_name + id = @connection.insert_sql('insert into "public"."ex" (number) values(5150)') + expect = @connection.query('select max(id) from ex').first.first + assert_equal expect, id + end + + def test_insert_sql_with_no_space_after_table_name + id = @connection.insert_sql("insert into ex(number) values(5150)") + expect = @connection.query('select max(id) from ex').first.first + assert_equal expect, id + end + def test_serial_sequence assert_equal 'public.accounts_id_seq', @connection.serial_sequence('accounts', 'id') @@ -35,6 +74,36 @@ module ActiveRecord @connection.default_sequence_name('zomg') end + def test_pk_and_sequence_for + pk, seq = @connection.pk_and_sequence_for('ex') + assert_equal 'id', pk + assert_equal @connection.default_sequence_name('ex', 'id'), seq + end + + def test_pk_and_sequence_for_with_non_standard_primary_key + @connection.exec_query('drop table if exists ex') + @connection.exec_query('create table ex(code serial primary key)') + pk, seq = @connection.pk_and_sequence_for('ex') + assert_equal 'code', pk + assert_equal @connection.default_sequence_name('ex', 'code'), seq + end + + def test_pk_and_sequence_for_returns_nil_if_no_seq + @connection.exec_query('drop table if exists ex') + @connection.exec_query('create table ex(id integer primary key)') + assert_nil @connection.pk_and_sequence_for('ex') + end + + def test_pk_and_sequence_for_returns_nil_if_no_pk + @connection.exec_query('drop table if exists ex') + @connection.exec_query('create table ex(id integer)') + assert_nil @connection.pk_and_sequence_for('ex') + end + + def test_pk_and_sequence_for_returns_nil_if_table_not_found + assert_nil @connection.pk_and_sequence_for('unobtainium') + end + def test_exec_insert_number insert(@connection, 'number' => 10) diff --git a/activerecord/test/cases/adapters/postgresql/schema_test.rb b/activerecord/test/cases/adapters/postgresql/schema_test.rb index a5c3e69af9..27e7b1a1c3 100644 --- a/activerecord/test/cases/adapters/postgresql/schema_test.rb +++ b/activerecord/test/cases/adapters/postgresql/schema_test.rb @@ -20,6 +20,7 @@ class SchemaTest < ActiveRecord::TestCase 'email character varying(50)', 'moment timestamp without time zone default now()' ] + PK_TABLE_NAME = 'table_with_pk' class Thing1 < ActiveRecord::Base set_table_name "test_schema.things" @@ -49,6 +50,7 @@ class SchemaTest < ActiveRecord::TestCase @connection.execute "CREATE INDEX #{INDEX_B_NAME} ON #{SCHEMA2_NAME}.#{TABLE_NAME} USING btree (#{INDEX_B_COLUMN_S2});" @connection.execute "CREATE INDEX #{INDEX_C_NAME} ON #{SCHEMA_NAME}.#{TABLE_NAME} USING gin (#{INDEX_C_COLUMN});" @connection.execute "CREATE INDEX #{INDEX_C_NAME} ON #{SCHEMA2_NAME}.#{TABLE_NAME} USING gin (#{INDEX_C_COLUMN});" + @connection.execute "CREATE TABLE #{SCHEMA_NAME}.#{PK_TABLE_NAME} (id serial primary key)" end def teardown @@ -63,12 +65,30 @@ class SchemaTest < ActiveRecord::TestCase end end + def test_table_exists_when_on_schema_search_path + with_schema_search_path(SCHEMA_NAME) do + assert(@connection.table_exists?(TABLE_NAME), "table should exist and be found") + end + end + + def test_table_exists_when_not_on_schema_search_path + with_schema_search_path('PUBLIC') do + assert(!@connection.table_exists?(TABLE_NAME), "table exists but should not be found") + end + end + def test_table_exists_wrong_schema assert(!@connection.table_exists?("foo.things"), "table should not exist") end - def test_table_exists_quoted_table - assert(@connection.table_exists?('"things.table"'), "table should exist") + def test_table_exists_quoted_names + [ %("#{SCHEMA_NAME}"."#{TABLE_NAME}"), %(#{SCHEMA_NAME}."#{TABLE_NAME}"), %(#{SCHEMA_NAME}."#{TABLE_NAME}")].each do |given| + assert(@connection.table_exists?(given), "table should exist when specified as #{given}") + end + with_schema_search_path(SCHEMA_NAME) do + given = %("#{TABLE_NAME}") + assert(@connection.table_exists?(given), "table should exist when specified as #{given}") + end end def test_with_schema_prefixed_table_name @@ -91,7 +111,6 @@ class SchemaTest < ActiveRecord::TestCase end end - def test_proper_encoding_of_table_name assert_equal '"table_name"', @connection.quote_table_name('table_name') assert_equal '"table.name"', @connection.quote_table_name('"table.name"') @@ -164,6 +183,79 @@ class SchemaTest < ActiveRecord::TestCase ActiveRecord::Base.connection.schema_search_path = "public" end + def test_primary_key_with_schema_specified + [ + %("#{SCHEMA_NAME}"."#{PK_TABLE_NAME}"), + %(#{SCHEMA_NAME}."#{PK_TABLE_NAME}"), + %(#{SCHEMA_NAME}.#{PK_TABLE_NAME}) + ].each do |given| + assert_equal 'id', @connection.primary_key(given), "primary key should be found when table referenced as #{given}" + end + end + + def test_primary_key_assuming_schema_search_path + with_schema_search_path(SCHEMA_NAME) do + assert_equal 'id', @connection.primary_key(PK_TABLE_NAME), "primary key should be found" + end + end + + def test_primary_key_raises_error_if_table_not_found_on_schema_search_path + with_schema_search_path(SCHEMA2_NAME) do + assert_raises(ActiveRecord::StatementInvalid) do + @connection.primary_key(PK_TABLE_NAME) + end + end + end + + def test_pk_and_sequence_for_with_schema_specified + [ + %("#{SCHEMA_NAME}"."#{PK_TABLE_NAME}"), + %(#{SCHEMA_NAME}."#{PK_TABLE_NAME}"), + %(#{SCHEMA_NAME}.#{PK_TABLE_NAME}) + ].each do |given| + pk, seq = @connection.pk_and_sequence_for(given) + assert_equal 'id', pk, "primary key should be found when table referenced as #{given}" + assert_equal "#{SCHEMA_NAME}.#{PK_TABLE_NAME}_id_seq", seq, "sequence name should be found when table referenced as #{given}" + end + end + + def test_extract_schema_and_table + { + %(table_name) => [nil,'table_name'], + %("table.name") => [nil,'table.name'], + %(schema.table_name) => %w{schema table_name}, + %("schema".table_name) => %w{schema table_name}, + %(schema."table_name") => %w{schema table_name}, + %("schema"."table_name") => %w{schema table_name}, + %("even spaces".table) => ['even spaces','table'], + %(schema."table.name") => ['schema', 'table.name'] + }.each do |given,expect| + assert_equal expect, @connection.send(:extract_schema_and_table, given) + end + end + + def test_current_schema + { + %('$user',public) => 'public', + SCHEMA_NAME => SCHEMA_NAME, + %(#{SCHEMA2_NAME},#{SCHEMA_NAME},public) => SCHEMA2_NAME, + %(public,#{SCHEMA2_NAME},#{SCHEMA_NAME}) => 'public' + }.each do |given,expect| + with_schema_search_path(given) { assert_equal expect, @connection.current_schema } + end + end + + def test_schema_exists? + { + 'public' => true, + SCHEMA_NAME => true, + SCHEMA2_NAME => true, + 'darkside' => false + }.each do |given,expect| + assert_equal expect, @connection.schema_exists?(given) + end + end + private def columns(table_name) @connection.send(:column_definitions, table_name).map do |name, type, default| diff --git a/activerecord/test/cases/adapters/postgresql/view_test.rb b/activerecord/test/cases/adapters/postgresql/view_test.rb new file mode 100644 index 0000000000..303ba9245a --- /dev/null +++ b/activerecord/test/cases/adapters/postgresql/view_test.rb @@ -0,0 +1,49 @@ +require "cases/helper" + +class ViewTest < ActiveRecord::TestCase + self.use_transactional_fixtures = false + + SCHEMA_NAME = 'test_schema' + TABLE_NAME = 'things' + VIEW_NAME = 'view_things' + COLUMNS = [ + 'id integer', + 'name character varying(50)', + 'email character varying(50)', + 'moment timestamp without time zone' + ] + + class ThingView < ActiveRecord::Base + set_table_name 'test_schema.view_things' + end + + def setup + @connection = ActiveRecord::Base.connection + @connection.execute "CREATE SCHEMA #{SCHEMA_NAME} CREATE TABLE #{TABLE_NAME} (#{COLUMNS.join(',')})" + @connection.execute "CREATE TABLE #{SCHEMA_NAME}.\"#{TABLE_NAME}.table\" (#{COLUMNS.join(',')})" + @connection.execute "CREATE VIEW #{SCHEMA_NAME}.#{VIEW_NAME} AS SELECT id,name,email,moment FROM #{SCHEMA_NAME}.#{TABLE_NAME}" + end + + def teardown + @connection.execute "DROP SCHEMA #{SCHEMA_NAME} CASCADE" + end + + def test_table_exists + name = ThingView.table_name + assert @connection.table_exists?(name), "'#{name}' table should exist" + end + + def test_column_definitions + assert_nothing_raised do + assert_equal COLUMNS, columns("#{SCHEMA_NAME}.#{VIEW_NAME}") + end + end + + private + def columns(table_name) + @connection.send(:column_definitions, table_name).map do |name, type, default| + "#{name} #{type}" + (default ? " default #{default}" : '') + end + end + +end |