Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Don't use a schema by default on postgresql

Before, the shared PostgreSQL adapter had a default schema of public.
This was added to fix some issues when a user had a tables with the
same name in multiple schemas.  This commit changes it so that a
default schema can still be specified, but removes the built-in
default of public.

Internally, this changes some constants in AdapterMethods from
strings to procs that take schemas and tables, and changes the
SQL they produce to not check for a specific schema unless a
schema is provided.

This commit removes the private Database instance methods
primary_key_for_table and primary_key_sequence_from_table, and
changes the API to the public Database instance method
primary_key, and adds the public Database instance method
primary_key_sequence.  It also fixes a performance issue when
a table does not have a primary key, where it would query for
the primary key every time, instead of using the cached negative
lookup.

This commit also adds a couple commented out lines to the postgres
and integration tests.  Because the code paths for Postgres 8.2 and
later and 8.1 and earlier are very different when it comes to
inserting records, testing both is a good idea whenever changes are
made to this area of the code.
  • Loading branch information...
commit 2ce435a2730d9cab6e56521763faf8f01972bd8f 1 parent 6ca4bfb
@jeremyevans authored
View
73 lib/sequel_core/adapters/shared/postgres.rb
@@ -53,7 +53,7 @@ module AdapterMethods
attr_writer :db
SELECT_CURRVAL = "SELECT currval('%s')".freeze
- SELECT_CUSTOM_SEQUENCE = <<-end_sql
+ SELECT_CUSTOM_SEQUENCE = proc do |schema, table| <<-end_sql
SELECT '"' || name.nspname || '"."' || CASE
WHEN split_part(def.adsrc, '''', 2) ~ '.' THEN
substr(split_part(def.adsrc, '''', 2),
@@ -67,10 +67,11 @@ module AdapterMethods
JOIN pg_constraint cons ON (conrelid = adrelid AND adnum = conkey[1])
WHERE cons.contype = 'p'
AND def.adsrc ~* 'nextval'
- AND name.nspname = '%s'
- AND t.relname = '%s'
+ #{"AND name.nspname = '#{schema}'" if schema}
+ AND t.relname = '#{table}'
end_sql
- SELECT_PK = <<-end_sql
+ end
+ SELECT_PK = proc do |schema, table| <<-end_sql
SELECT pg_attribute.attname
FROM pg_class, pg_attribute, pg_index, pg_namespace
WHERE pg_class.oid = pg_attribute.attrelid
@@ -78,10 +79,11 @@ module AdapterMethods
AND pg_class.oid = pg_index.indrelid
AND pg_index.indkey[0] = pg_attribute.attnum
AND pg_index.indisprimary = 't'
- AND pg_namespace.nspname = '%s'
- AND pg_class.relname = '%s'
+ #{"AND pg_namespace.nspname = '#{schema}'" if schema}
+ AND pg_class.relname = '#{table}'
end_sql
- SELECT_SERIAL_SEQUENCE = <<-end_sql
+ end
+ SELECT_SERIAL_SEQUENCE = proc do |schema, table| <<-end_sql
SELECT '"' || name.nspname || '"."' || seq.relname || '"'
FROM pg_class seq, pg_attribute attr, pg_depend dep,
pg_namespace name, pg_constraint cons
@@ -93,9 +95,10 @@ module AdapterMethods
AND attr.attrelid = cons.conrelid
AND attr.attnum = cons.conkey[1]
AND cons.contype = 'p'
- AND name.nspname = '%s'
- AND seq.relname = '%s'
+ #{"AND name.nspname = '#{schema}'" if schema}
+ AND seq.relname = '#{table}'
end_sql
+ end
# Depth of the current transaction on this connection, used
# to implement multi-level transactions with savepoints.
@@ -132,7 +135,7 @@ def last_insert_id(sequence)
# Get the primary key for the given table.
def primary_key(schema, table)
- sql = SELECT_PK % [schema, table]
+ sql = SELECT_PK[schema, table]
@db.log_info(sql)
execute(sql) do |r|
return single_value(r)
@@ -141,14 +144,14 @@ def primary_key(schema, table)
# Get the primary key and sequence for the given table.
def sequence(schema, table)
- sql = SELECT_SERIAL_SEQUENCE % [schema, table]
+ sql = SELECT_SERIAL_SEQUENCE[schema, table]
@db.log_info(sql)
execute(sql) do |r|
seq = single_value(r)
return seq if seq
end
- sql = SELECT_CUSTOM_SEQUENCE % [schema, table]
+ sql = SELECT_CUSTOM_SEQUENCE[schema, table]
@db.log_info(sql)
execute(sql) do |r|
return single_value(r)
@@ -336,8 +339,25 @@ def locks
end
# Return primary key for the given table.
- def primary_key(table, server=nil)
- synchronize(server){|conn| primary_key_for_table(conn, table)}
+ def primary_key(table, opts={})
+ quoted_table = quote_schema_table(table)
+ return @primary_keys[quoted_table] if @primary_keys.include?(quoted_table)
+ @primary_keys[quoted_table] = if conn = opts[:conn]
+ conn.primary_key(*schema_and_table(table))
+ else
+ synchronize(opts[:server]){|con| con.primary_key(*schema_and_table(table))}
+ end
+ end
+
+ # Return the sequence providing the default for the primary key for the given table.
+ def primary_key_sequence(table, opts={})
+ quoted_table = quote_schema_table(table)
+ return @primary_key_sequences[quoted_table] if @primary_key_sequences.include?(quoted_table)
+ @primary_key_sequences[quoted_table] = if conn = opts[:conn]
+ conn.sequence(*schema_and_table(table))
+ else
+ synchronize(opts[:server]){|con| con.sequence(*schema_and_table(table))}
+ end
end
# SQL DDL statement for renaming a table. PostgreSQL doesn't allow you to change a table's schema in
@@ -436,12 +456,7 @@ def transaction(server=nil)
end
private
-
- # The default schema to use if none is specified (default: public)
- def default_schema_default
- :public
- end
-
+
# PostgreSQL folds unquoted identifiers to lowercase, so it shouldn't need to upcase identifiers on input.
def identifier_input_method_default
nil
@@ -461,12 +476,12 @@ def identifier_output_method_default
def insert_result(conn, table, values)
case values
when Hash
- return nil unless pk = primary_key_for_table(conn, table)
+ return nil unless pk = primary_key(table, :conn=>conn)
if pk and pkv = values[pk.to_sym]
pkv
else
begin
- if seq = primary_key_sequence_for_table(conn, table)
+ if seq = primary_key_sequence(table, :conn=>conn)
conn.last_insert_id(seq)
end
rescue Exception => e
@@ -486,20 +501,6 @@ def prepared_arg_placeholder
PREPARED_ARG_PLACEHOLDER
end
- # Returns primary key for the given table. This information is
- # cached, and if the primary key for a table is changed, the
- # @primary_keys instance variable should be reset manually.
- def primary_key_for_table(conn, table)
- @primary_keys[quote_schema_table(table)] ||= conn.primary_key(*schema_and_table(table))
- end
-
- # Returns primary key for the given table. This information is
- # cached, and if the primary key for a table is changed, the
- # @primary_keys instance variable should be reset manually.
- def primary_key_sequence_for_table(conn, table)
- @primary_key_sequences[quote_schema_table(table)] ||= conn.sequence(*schema_and_table(table))
- end
-
# The dataset used for parsing table schemas, using the pg_* system catalogs.
def schema_parse_table(table_name, opts)
ds2 = dataset
View
40 spec/adapters/postgres_spec.rb
@@ -4,7 +4,7 @@
POSTGRES_URL = 'postgres://postgres:postgres@localhost:5432/reality_spec' unless defined? POSTGRES_URL
POSTGRES_DB = Sequel.connect(ENV['SEQUEL_PG_SPEC_DB']||POSTGRES_URL)
end
-
+#POSTGRES_DB.instance_variable_set(:@server_version, 80100)
POSTGRES_DB.create_table! :test do
text :name
integer :value, :index => true
@@ -347,8 +347,8 @@ def POSTGRES_DB.ret_commit
full_text_index [:title, :body]
end
POSTGRES_DB.create_table_sql_list(:posts, *g.create_info).should == [
- "CREATE TABLE public.posts (title text, body text)",
- "CREATE INDEX posts_title_body_index ON public.posts USING gin (to_tsvector('simple', (COALESCE(title, '') || ' ' || COALESCE(body, ''))))"
+ "CREATE TABLE posts (title text, body text)",
+ "CREATE INDEX posts_title_body_index ON posts USING gin (to_tsvector('simple', (COALESCE(title, '') || ' ' || COALESCE(body, ''))))"
]
end
@@ -359,8 +359,8 @@ def POSTGRES_DB.ret_commit
full_text_index [:title, :body], :language => 'french'
end
POSTGRES_DB.create_table_sql_list(:posts, *g.create_info).should == [
- "CREATE TABLE public.posts (title text, body text)",
- "CREATE INDEX posts_title_body_index ON public.posts USING gin (to_tsvector('french', (COALESCE(title, '') || ' ' || COALESCE(body, ''))))"
+ "CREATE TABLE posts (title text, body text)",
+ "CREATE INDEX posts_title_body_index ON posts USING gin (to_tsvector('french', (COALESCE(title, '') || ' ' || COALESCE(body, ''))))"
]
end
@@ -381,8 +381,8 @@ def POSTGRES_DB.ret_commit
spatial_index [:geom]
end
POSTGRES_DB.create_table_sql_list(:posts, *g.create_info).should == [
- "CREATE TABLE public.posts (geom geometry)",
- "CREATE INDEX posts_geom_index ON public.posts USING gist (geom)"
+ "CREATE TABLE posts (geom geometry)",
+ "CREATE INDEX posts_geom_index ON posts USING gist (geom)"
]
end
@@ -392,8 +392,8 @@ def POSTGRES_DB.ret_commit
index :title, :type => 'hash'
end
POSTGRES_DB.create_table_sql_list(:posts, *g.create_info).should == [
- "CREATE TABLE public.posts (title varchar(5))",
- "CREATE INDEX posts_title_index ON public.posts USING hash (title)"
+ "CREATE TABLE posts (title varchar(5))",
+ "CREATE INDEX posts_title_index ON posts USING hash (title)"
]
end
@@ -403,8 +403,8 @@ def POSTGRES_DB.ret_commit
index :title, :type => 'hash', :unique => true
end
POSTGRES_DB.create_table_sql_list(:posts, *g.create_info).should == [
- "CREATE TABLE public.posts (title varchar(5))",
- "CREATE UNIQUE INDEX posts_title_index ON public.posts USING hash (title)"
+ "CREATE TABLE posts (title varchar(5))",
+ "CREATE UNIQUE INDEX posts_title_index ON posts USING hash (title)"
]
end
@@ -414,8 +414,8 @@ def POSTGRES_DB.ret_commit
index :title, :where => {:something => 5}
end
POSTGRES_DB.create_table_sql_list(:posts, *g.create_info).should == [
- "CREATE TABLE public.posts (title varchar(5))",
- "CREATE INDEX posts_title_index ON public.posts (title) WHERE (something = 5)"
+ "CREATE TABLE posts (title varchar(5))",
+ "CREATE INDEX posts_title_index ON posts (title) WHERE (something = 5)"
]
end
@@ -425,8 +425,8 @@ def POSTGRES_DB.ret_commit
index :title, :where => {:something => 5}
end
POSTGRES_DB.create_table_sql_list(Sequel::SQL::Identifier.new(:posts__test), *g.create_info).should == [
- "CREATE TABLE public.posts__test (title varchar(5))",
- "CREATE INDEX posts__test_title_index ON public.posts__test (title) WHERE (something = 5)"
+ "CREATE TABLE posts__test (title varchar(5))",
+ "CREATE INDEX posts__test_title_index ON posts__test (title) WHERE (something = 5)"
]
end
end
@@ -557,18 +557,18 @@ def POSTGRES_DB.ret_commit
specify "should be able to get primary keys for tables in a given schema" do
POSTGRES_DB.create_table(:schema_test__schema_test){primary_key :i}
- POSTGRES_DB.synchronize{|c| POSTGRES_DB.send(:primary_key_for_table, c, :schema_test__schema_test).should == 'i'}
+ POSTGRES_DB.primary_key(:schema_test__schema_test).should == 'i'
end
specify "should be able to get serial sequences for tables in a given schema" do
POSTGRES_DB.create_table(:schema_test__schema_test){primary_key :i}
- POSTGRES_DB.synchronize{|c| POSTGRES_DB.send(:primary_key_sequence_for_table, c, :schema_test__schema_test).should == '"schema_test"."schema_test_i_seq"'}
+ POSTGRES_DB.primary_key_sequence(:schema_test__schema_test).should == '"schema_test"."schema_test_i_seq"'
end
specify "should be able to get custom sequences for tables in a given schema" do
POSTGRES_DB << "CREATE SEQUENCE schema_test.kseq"
POSTGRES_DB.create_table(:schema_test__schema_test){integer :j; primary_key :k, :type=>:integer, :default=>"nextval('schema_test.kseq'::regclass)".lit}
- POSTGRES_DB.synchronize{|c| POSTGRES_DB.send(:primary_key_sequence_for_table, c, :schema_test__schema_test).should == '"schema_test"."kseq"'}
+ POSTGRES_DB.primary_key_sequence(:schema_test__schema_test).should == '"schema_test"."kseq"'
end
specify "#default_schema= should change the default schema used from public" do
@@ -576,8 +576,8 @@ def POSTGRES_DB.ret_commit
POSTGRES_DB.default_schema = :schema_test
POSTGRES_DB.table_exists?(:schema_test).should == true
POSTGRES_DB.tables.should == [:schema_test]
- POSTGRES_DB.synchronize{|c| POSTGRES_DB.send(:primary_key_for_table, c, :schema_test).should == 'i'}
- POSTGRES_DB.synchronize{|c| POSTGRES_DB.send(:primary_key_sequence_for_table, c, :schema_test).should == '"schema_test"."schema_test_i_seq"'}
+ POSTGRES_DB.primary_key(:schema_test__schema_test).should == 'i'
+ POSTGRES_DB.primary_key_sequence(:schema_test__schema_test).should == '"schema_test"."schema_test_i_seq"'
end
end
View
1  spec/integration/spec_helper.rb
@@ -27,6 +27,7 @@ def stop_logging
unless defined?(INTEGRATION_DB)
url = defined?(INTEGRATION_URL) ? INTEGRATION_URL : ENV['SEQUEL_INTEGRATION_URL']
INTEGRATION_DB = Sequel.connect(url)
+ #INTEGRATION_DB.instance_variable_set(:@server_version, 80100)
end
class Spec::Example::ExampleGroup
def sqls_should_be(*args)
Please sign in to comment.
Something went wrong with that request. Please try again.