Permalink
Browse files

On PostgreSQL, if no :schema option is provided for #tables, #table_e…

…xists?, or #schema, assume all schemas except the default non-public ones (Fixes #305)

This changes Sequel's behavior on PostgreSQL for the Database
methods #tables, #table_exists?, or #schema to assume any/all
schemas except the default non-public ones (pg_catalog, pg_toast,
pg_temp_1, pg_toast_temp_1, and information_schema).  Before,
didn't assume any particular schema, which led to bad results if
you used a table name that was the same as a table name in the
information schema.
  • Loading branch information...
1 parent d7f3265 commit b33a367a3c0c4fc8c3a9c53429b442e77d9f42c7 @jeremyevans committed Jun 21, 2010
Showing with 43 additions and 7 deletions.
  1. +2 −0 CHANGELOG
  2. +16 −3 lib/sequel/adapters/shared/postgres.rb
  3. +25 −4 spec/adapters/postgres_spec.rb
View
@@ -1,5 +1,7 @@
=== HEAD
+* On PostgreSQL, if no :schema option is provided for #tables, #table_exists?, or #schema, assume all schemas except the default non-public ones (jeremyevans) (#305)
+
* Cache prepared statements when using the native sqlite driver, improving performance (jeremyevans)
* Add a Tree plugin for treating model objects as being part of a tree (jeremyevans, mwlang)
@@ -157,6 +157,7 @@ def sequence(schema, table)
# Methods shared by Database instances that connect to PostgreSQL.
module DatabaseMethods
+ EXCLUDE_SCHEMAS = %w[pg_catalog pg_toast pg_temp_1 pg_toast_temp_1 information_schema]
PREPARED_ARG_PLACEHOLDER = LiteralString.new('$').freeze
RE_CURRVAL_ERROR = /currval of sequence "(.*)" is not yet defined in this session|relation "(.*)" does not exist/.freeze
SYSTEM_TABLE_REGEXP = /^pg|sql/.freeze
@@ -362,13 +363,14 @@ def table_exists?(table, opts={})
# * :schema - The schema to search (default_schema by default)
# * :server - The server to use
def tables(opts={})
- ds = metadata_dataset.from(:pg_class).filter(:relkind=>'r').select(:relname).exclude(SQL::StringExpression.like(:relname, SYSTEM_TABLE_REGEXP)).server(opts[:server]).join(:pg_namespace, :oid=>:relnamespace, :nspname=>(opts[:schema]||default_schema||'public').to_s)
+ ds = metadata_dataset.from(:pg_class).filter(:relkind=>'r').select(:relname).exclude(SQL::StringExpression.like(:relname, SYSTEM_TABLE_REGEXP)).server(opts[:server]).join(:pg_namespace, :oid=>:relnamespace)
+ ds = filter_schema(ds, opts)
m = output_identifier_meth
block_given? ? yield(ds) : ds.map{|r| m.call(r[:relname])}
end
private
-
+
# SQL statement to create database function.
def create_function_sql(name, definition, opts={})
args = opts[:args]
@@ -427,6 +429,16 @@ def drop_trigger_sql(table, name, opts={})
"DROP TRIGGER#{' IF EXISTS' if opts[:if_exists]} #{name} ON #{quote_schema_table(table)}#{' CASCADE' if opts[:cascade]}"
end
+ # If opts includes a :schema option, or a default schema is used, restrict the dataset to
+ # that schema. Otherwise, just exclude the default PostgreSQL schemas except for public.
+ def filter_schema(ds, opts)
+ if schema = opts[:schema] || default_schema
+ ds.filter(:pg_namespace__nspname=>schema.to_s)
+ else
+ ds.exclude(:pg_namespace__nspname=>EXCLUDE_SCHEMAS)
+ end
+ end
+
# PostgreSQL folds unquoted identifiers to lowercase, so it shouldn't need to upcase identifiers on input.
def identifier_input_method_default
nil
@@ -523,13 +535,14 @@ def schema_parse_table(table_name, opts)
from(:pg_class).
join(:pg_attribute, :attrelid=>:oid).
join(:pg_type, :oid=>:atttypid).
+ join(:pg_namespace, :oid=>:pg_class__relnamespace).
left_outer_join(:pg_attrdef, :adrelid=>:pg_class__oid, :adnum=>:pg_attribute__attnum).
left_outer_join(:pg_index, :indrelid=>:pg_class__oid, :indisprimary=>true).
filter(:pg_attribute__attisdropped=>false).
filter{|o| o.pg_attribute__attnum > 0}.
filter(:pg_class__relname=>m2.call(table_name)).
order(:pg_attribute__attnum)
- ds.join!(:pg_namespace, :oid=>:pg_class__relnamespace, :nspname=>(opts[:schema] || default_schema).to_s) if opts[:schema] || default_schema
+ ds = filter_schema(ds, opts)
ds.map do |row|
row[:default] = nil if blank_object?(row[:default])
row[:type] = schema_column_type(row[:db_type])
@@ -557,19 +557,40 @@ def logger.method_missing(m, msg)
POSTGRES_DB.drop_table(:schema_test.qualify(:schema_test))
end
- specify "#tables should include only tables in the public schema if no schema is given" do
+ specify "#tables should not include tables in a default non-public schema" do
POSTGRES_DB.create_table(:schema_test__schema_test){integer :i}
- POSTGRES_DB.tables.should_not include(:schema_test)
+ POSTGRES_DB.tables.should include(:schema_test)
+ POSTGRES_DB.tables.should_not include(:pg_am)
+ POSTGRES_DB.tables.should_not include(:domain_udt_usage)
end
specify "#tables should return tables in the schema provided by the :schema argument" do
POSTGRES_DB.create_table(:schema_test__schema_test){integer :i}
POSTGRES_DB.tables(:schema=>:schema_test).should == [:schema_test]
end
- specify "#table_exists? should assume the public schema if no schema is provided" do
+ specify "#schema should not include columns from tables in a default non-public schema" do
+ POSTGRES_DB.create_table(:schema_test__domains){integer :i}
+ sch = POSTGRES_DB.schema(:domains)
+ cs = sch.map{|x| x.first}
+ cs.should include(:i)
+ cs.should_not include(:data_type)
+ end
+
+ specify "#schema should only include columns from the table in the given :schema argument" do
+ POSTGRES_DB.create_table!(:domains){integer :d}
+ POSTGRES_DB.create_table(:schema_test__domains){integer :i}
+ sch = POSTGRES_DB.schema(:domains, :schema=>:schema_test)
+ cs = sch.map{|x| x.first}
+ cs.should include(:i)
+ cs.should_not include(:d)
+ POSTGRES_DB.drop_table(:domains)
+ end
+
+ specify "#table_exists? should not include tables from the default non-public schemas" do
POSTGRES_DB.create_table(:schema_test__schema_test){integer :i}
- POSTGRES_DB.table_exists?(:schema_test).should == false
+ POSTGRES_DB.table_exists?(:schema_test).should == true
+ POSTGRES_DB.table_exists?(:domain_udt_usage).should == false
end
specify "#table_exists? should see if the table is in a given schema" do

0 comments on commit b33a367

Please sign in to comment.