Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Fixes issue #7914 - PostgreSQL adapter doesn't fetch column defaults when using multiple schemas and domains #7937

Merged
merged 4 commits into from

2 participants

@arturopie

Please, read issue #7914 for details about the bug, and commit messages for details about the fix and changes.

Let me know what branches you want me to backport the changes to.

Thanks,

Arturo

arturopie added some commits
@arturopie arturopie #7914 get default value when type uses schema name
PostgreSQL adapter properly parses default values when using multiple
schemas and domains.

When using domains across schemas, PostgresSQL prefixes the type of the
default value with the name of the schema where that type (or domain) is.

For example, this query:
```
SELECT a.attname, d.adsrc
FROM pg_attribute a LEFT JOIN pg_attrdef d
ON a.attrelid = d.adrelid AND a.attnum = d.adnum
WHERE a.attrelid = "defaults"'::regclass
AND a.attnum > 0 AND NOT a.attisdropped
ORDER BY a.attnum;
```

could return something like "'<default_value>'::pg_catalog.text" or
"(''<default_value>'::pg_catalog.text)::text" for the text columns with
defaults.

I modified the regexp used to parse this value so that it ignores
anything between ':: and \b(?:character varying|bpchar|text), and it
allows to have optional parens like in the above second example.
2da85ed
@arturopie arturopie #7914 Add change of previous commit to CHANGELOG.md 54b3f41
@arturopie arturopie #7914 Using a better way to get the defaults from db.
According to postgreSQL documentation:
(http://www.postgresql.org/docs/8.2/static/catalog-pg-attrdef.html)
we should not be using 'adsrc' field because this field is unaware of
outside changes that could affect the way that default values are
represented. Thus, I changed the queries to use
"pg_get_expr(adbin, adrelid)" instead of the historical "adsrc" field.
40475cf
@arturopie arturopie #7914 Remove code for unsupported postgreSQL version.
Remove parsing of character type default values for 8.1 formatting since
Rails doesn't support postgreSQL 8.1 anymore.

Remove misleading comment unrelated to code.
8fb841b
@rafaelfranca rafaelfranca merged commit ca618d4 into rails:master
@rafaelfranca

Thank you.

Is not easy to backport this fix to 3-2-stable. Could you open a new pull request to it?

@arturopie

Sure, what about 3-1-stable and 3-0-stable?

@rafaelfranca

The only branch that we are supporting is 3-2-stable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Oct 13, 2012
  1. @arturopie

    #7914 get default value when type uses schema name

    arturopie authored arturopie committed
    PostgreSQL adapter properly parses default values when using multiple
    schemas and domains.
    
    When using domains across schemas, PostgresSQL prefixes the type of the
    default value with the name of the schema where that type (or domain) is.
    
    For example, this query:
    ```
    SELECT a.attname, d.adsrc
    FROM pg_attribute a LEFT JOIN pg_attrdef d
    ON a.attrelid = d.adrelid AND a.attnum = d.adnum
    WHERE a.attrelid = "defaults"'::regclass
    AND a.attnum > 0 AND NOT a.attisdropped
    ORDER BY a.attnum;
    ```
    
    could return something like "'<default_value>'::pg_catalog.text" or
    "(''<default_value>'::pg_catalog.text)::text" for the text columns with
    defaults.
    
    I modified the regexp used to parse this value so that it ignores
    anything between ':: and \b(?:character varying|bpchar|text), and it
    allows to have optional parens like in the above second example.
Commits on Oct 14, 2012
  1. @arturopie

    #7914 Add change of previous commit to CHANGELOG.md

    arturopie authored arturopie committed
  2. @arturopie

    #7914 Using a better way to get the defaults from db.

    arturopie authored
    According to postgreSQL documentation:
    (http://www.postgresql.org/docs/8.2/static/catalog-pg-attrdef.html)
    we should not be using 'adsrc' field because this field is unaware of
    outside changes that could affect the way that default values are
    represented. Thus, I changed the queries to use
    "pg_get_expr(adbin, adrelid)" instead of the historical "adsrc" field.
  3. @arturopie

    #7914 Remove code for unsupported postgreSQL version.

    arturopie authored
    Remove parsing of character type default values for 8.1 formatting since
    Rails doesn't support postgreSQL 8.1 anymore.
    
    Remove misleading comment unrelated to code.
This page is out of date. Refresh to see the latest.
View
4 activerecord/CHANGELOG.md
@@ -1,5 +1,9 @@
## Rails 4.0.0 (unreleased) ##
+* PostgreSQL adapter correctly fetches default values when using multiple schemas and domains in a db. Fixes #7914
+
+ *Arturo Pie*
+
* Learn ActiveRecord::QueryMethods#order work with hash arguments
When symbol or hash passed we convert it to Arel::Nodes::Ordering.
View
13 activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb
@@ -280,16 +280,13 @@ def pk_and_sequence_for(table) #:nodoc:
end_sql
if result.nil? or result.empty?
- # If that fails, try parsing the primary key's default value.
- # Support the 7.x and 8.0 nextval('foo'::text) as well as
- # the 8.1+ nextval('foo'::regclass).
result = query(<<-end_sql, 'SCHEMA')[0]
SELECT attr.attname,
CASE
- WHEN split_part(def.adsrc, '''', 2) ~ '.' THEN
- substr(split_part(def.adsrc, '''', 2),
- strpos(split_part(def.adsrc, '''', 2), '.')+1)
- ELSE split_part(def.adsrc, '''', 2)
+ WHEN split_part(pg_get_expr(def.adbin, def.adrelid), '''', 2) ~ '.' THEN
+ substr(split_part(pg_get_expr(def.adbin, def.adrelid), '''', 2),
+ strpos(split_part(pg_get_expr(def.adbin, def.adrelid), '''', 2), '.')+1)
+ ELSE split_part(pg_get_expr(def.adbin, def.adrelid), '''', 2)
END
FROM pg_class t
JOIN pg_attribute attr ON (t.oid = attrelid)
@@ -297,7 +294,7 @@ def pk_and_sequence_for(table) #:nodoc:
JOIN pg_constraint cons ON (conrelid = adrelid AND adnum = conkey[1])
WHERE t.oid = '#{quote_table_name(table)}'::regclass
AND cons.contype = 'p'
- AND def.adsrc ~* 'nextval'
+ AND pg_get_expr(def.adbin, def.adrelid) ~* 'nextval'
end_sql
end
View
8 activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
@@ -78,11 +78,8 @@ def self.extract_value_from_default(default)
when /\A\(?(-?\d+(\.\d*)?\)?)\z/
$1
# Character types
- when /\A'(.*)'::(?:character varying|bpchar|text)\z/m
+ when /\A\(?'(.*)'::.*\b(?:character varying|bpchar|text)\z/m
$1
- # Character types (8.1 formatting)
- when /\AE'(.*)'::(?:character varying|bpchar|text)\z/m
- $1.gsub(/\\(\d\d\d)/) { $1.oct.chr }
# Binary data types
when /\A'(.*)'::bytea\z/m
$1
@@ -763,7 +760,8 @@ def select_raw(sql, name = nil)
# - ::regclass is a function that gives the id for a table name
def column_definitions(table_name) #:nodoc:
exec_query(<<-end_sql, 'SCHEMA').rows
- SELECT a.attname, format_type(a.atttypid, a.atttypmod), d.adsrc, a.attnotnull, a.atttypid, a.atttypmod
+ SELECT a.attname, format_type(a.atttypid, a.atttypmod),
+ pg_get_expr(d.adbin, d.adrelid), a.attnotnull, a.atttypid, a.atttypmod
FROM pg_attribute a LEFT JOIN pg_attrdef d
ON a.attrelid = d.adrelid AND a.attnum = d.adnum
WHERE a.attrelid = '#{quote_table_name(table_name)}'::regclass
View
2  activerecord/test/cases/adapters/postgresql/schema_test.rb
@@ -72,7 +72,7 @@ def teardown
end
def test_schema_names
- assert_equal ["public", "test_schema", "test_schema2"], @connection.schema_names
+ assert_equal ["public", "schema_1", "test_schema", "test_schema2"], @connection.schema_names
end
def test_create_schema
View
40 activerecord/test/cases/defaults_test.rb
@@ -109,3 +109,43 @@ def test_mysql_integer_not_null_defaults
end
end
end
+
+if current_adapter?(:PostgreSQLAdapter)
+ class DefaultsUsingMultipleSchemasAndDomainTest < ActiveSupport::TestCase
+ def setup
+ @connection = ActiveRecord::Base.connection
+
+ @old_search_path = @connection.schema_search_path
+ @connection.schema_search_path = "schema_1, pg_catalog"
+ @connection.create_table "defaults" do |t|
+ t.text "text_col", :default => "some value"
+ t.string "string_col", :default => "some value"
+ end
+ Default.reset_column_information
+ end
+
+ def test_text_defaults_in_new_schema_when_overriding_domain
+ assert_equal "some value", Default.new.text_col, "Default of text column was not correctly parse"
+ end
+
+ def test_string_defaults_in_new_schema_when_overriding_domain
+ assert_equal "some value", Default.new.string_col, "Default of string column was not correctly parse"
+ end
+
+ def test_bpchar_defaults_in_new_schema_when_overriding_domain
+ @connection.execute "ALTER TABLE defaults ADD bpchar_col bpchar DEFAULT 'some value'"
+ Default.reset_column_information
+ assert_equal "some value", Default.new.bpchar_col, "Default of bpchar column was not correctly parse"
+ end
+
+ def test_text_defaults_after_updating_column_default
+ @connection.execute "ALTER TABLE defaults ALTER COLUMN text_col SET DEFAULT 'some text'::schema_1.text"
+ assert_equal "some text", Default.new.text_col, "Default of text column was not correctly parse after updating default using '::text' since postgreSQL will add parens to the default in db"
+ end
+
+ def teardown
+ @connection.schema_search_path = @old_search_path
+ Default.reset_column_information
+ end
+ end
+end
View
9 activerecord/test/schema/postgresql_specific_schema.rb
@@ -12,6 +12,8 @@
execute 'DROP FUNCTION IF EXISTS partitioned_insert_trigger()'
+ execute "DROP SCHEMA IF EXISTS schema_1 CASCADE"
+
%w(accounts_id_seq developers_id_seq projects_id_seq topics_id_seq customers_id_seq orders_id_seq).each do |seq_name|
execute "SELECT setval('#{seq_name}', 100)"
end
@@ -37,7 +39,12 @@
);
_SQL
- execute <<_SQL
+ execute "CREATE SCHEMA schema_1"
+ execute "CREATE DOMAIN schema_1.text AS text"
+ execute "CREATE DOMAIN schema_1.varchar AS varchar"
+ execute "CREATE DOMAIN schema_1.bpchar AS bpchar"
+
+ execute <<_SQL
CREATE TABLE geometrics (
id serial primary key,
a_point point,
Something went wrong with that request. Please try again.