Permalink
Browse files

Merge pull request #1632 from tardate/pg_schema_fu

Improving PostgreSQL adapter schema-awareness
  • Loading branch information...
2 parents e369682 + 2b3d67f commit ceb5bbbafc642f835a30b2dd967879b1e719a511 @tenderlove tenderlove committed Jun 21, 2011
View
119 activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
@@ -466,10 +466,11 @@ def select_rows(sql, name = nil)
# 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 @@ def exec_delete(sql, name = 'SQL', binds = [])
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 @@ def tables(name = nil)
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 @@ def current_database
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 @@ def translate_exception(exception, message)
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 @@ def extract_pg_identifier_from_name(name)
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
View
69 activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb
@@ -10,6 +10,45 @@ def setup
@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 @@ def test_default_sequence_name_bad_table
@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)
View
98 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 @@ def setup
@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 @@ def test_table_exists?
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 @@ def test_with_schema_search_path
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 @@ def test_with_uppercase_index_name
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|
View
49 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

0 comments on commit ceb5bbb

Please sign in to comment.