Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add support for PostgreSQL index storage options, and reflection of index storage options and index access methods #179

Merged
merged 3 commits into from Jun 19, 2015
Merged

Add support for PostgreSQL index storage options, and reflection of index storage options and index access methods #179

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
dff8150
Add support for PostgreSQL index storage parameters
hollobon Jun 3, 2015
b03ee45
Add reflection of PostgreSQL index storage options
hollobon Jun 4, 2015
af19435
Add reflection of PostgreSQL index access methods (USING clause)
hollobon Jun 4, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
39 changes: 36 additions & 3 deletions lib/sqlalchemy/dialects/postgresql/base.py
View file
Open in desktop
Expand Up @@ -401,6 +401,15 @@
underlying CREATE INDEX command, so it *must* be a valid index type for your
version of PostgreSQL.

Index Storage Parameters
^^^^^^^^^^^^^^^^^^^^^^^^

PostgreSQL allows storage parameters to be set on indexes. The storage
parameters available depend on the index method used by the index. Storage
parameters can be specified on :class:`.Index` using the ``postgresql_with``
keyword argument::

Index('my_index', my_table.c.data, postgresql_with={"fillfactor": 50})

.. _postgresql_index_concurrently:

Expand Down Expand Up @@ -1592,6 +1601,13 @@ def visit_create_index(self, create):
])
)

withclause = index.dialect_options['postgresql']['with']

if withclause:
text += " WITH (%s)" % (', '.join(
['%s = %s' % storage_parameter
for storage_parameter in withclause.items()]))

whereclause = index.dialect_options["postgresql"]["where"]

if whereclause is not None:
Expand Down Expand Up @@ -1919,6 +1935,7 @@ class PGDialect(default.DefaultDialect):
"where": None,
"ops": {},
"concurrently": False,
"with": {}
}),
(schema.Table, {
"ignore_search_path": False,
Expand Down Expand Up @@ -2607,14 +2624,18 @@ def get_indexes(self, connection, table_name, schema, **kw):
SELECT
i.relname as relname,
ix.indisunique, ix.indexprs, ix.indpred,
a.attname, a.attnum, NULL, ix.indkey%s
a.attname, a.attnum, NULL, ix.indkey%s,
i.reloptions, am.amname
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

eek, < 8.5, does this query work perfectly on PG back to the earliest 8.x versions under all circumstances? the reason the SQL for < 8.5 is here is so the many issues we've had with old versions are no longer impacted by new features in our reflection system.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK it works back to 8.3.23 at least. good enough

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I did check, should be fine. All the 8.x versions are now EOL so not sure it's worth keeping the special cases around that much longer.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unfortunately Amazon Redshift implements an old 8.X version of Postgresql, and the PG dialect is the basis for the redshift dialect so this is still a thing.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed this breaks code with redshift. Also the reloptions column doesn't exist in database PostgreSQL 8.1 but exists only in database PostgreSQL 8.2 and up

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I understand it, Redshift is a fork of PosgreSQL 8.2. That seems fair enough to fix, I'll look at that.

I don't see a reason why SQLAlchemy should be supporting PostgreSQL 8.1 though, given that it's been EOLed and unsupported since November 2010 (unless another of the forks is based off that version).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I now find it's 8.0.2 (http://docs.aws.amazon.com/redshift/latest/dg/c_redshift-and-postgres-sql.html). I'll amend with that in mind.

It would be good to have a policy on this, perhaps to support 8.0.2 and the currently active versions of PostgreSQL (which are 9.0-9.4 at the moment).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK I've created a pull request which should fix this. Tested on PostgreSQL 8.0.2 as I don't have access to a Redshift instance.

FROM
pg_class t
join pg_index ix on t.oid = ix.indrelid
join pg_class i on i.oid = ix.indexrelid
left outer join
pg_attribute a
on t.oid = a.attrelid and %s
left outer join
pg_am am
on i.relam = am.oid
WHERE
t.relkind IN ('r', 'v', 'f', 'm')
and t.oid = :table_oid
Expand All @@ -2634,7 +2655,8 @@ def get_indexes(self, connection, table_name, schema, **kw):
SELECT
i.relname as relname,
ix.indisunique, ix.indexprs, ix.indpred,
a.attname, a.attnum, c.conrelid, ix.indkey::varchar
a.attname, a.attnum, c.conrelid, ix.indkey::varchar,
i.reloptions, am.amname
FROM
pg_class t
join pg_index ix on t.oid = ix.indrelid
Expand All @@ -2647,6 +2669,9 @@ def get_indexes(self, connection, table_name, schema, **kw):
on (ix.indrelid = c.conrelid and
ix.indexrelid = c.conindid and
c.contype in ('p', 'u', 'x'))
left outer join
pg_am am
on i.relam = am.oid
WHERE
t.relkind IN ('r', 'v', 'f', 'm')
and t.oid = :table_oid
Expand All @@ -2663,7 +2688,7 @@ def get_indexes(self, connection, table_name, schema, **kw):

sv_idx_name = None
for row in c.fetchall():
idx_name, unique, expr, prd, col, col_num, conrelid, idx_key = row
idx_name, unique, expr, prd, col, col_num, conrelid, idx_key, options, amname = row

if expr:
if idx_name != sv_idx_name:
Expand All @@ -2689,6 +2714,10 @@ def get_indexes(self, connection, table_name, schema, **kw):
index['unique'] = unique
if conrelid is not None:
index['duplicates_constraint'] = idx_name
if options:
index['options'] = dict([option.split("=") for option in options])
if amname and amname != 'btree':
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

HMmmm. Maybe we want to include that its "btree". I know we don't want it in the CREATE INDEX on the other end all the time though. I'll leave it like this for now.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It may well be fine to have it in the CREATE INDEX; that is what pg_dump does:

> create table test (a integer);
CREATE TABLE
> create index on test(a);
CREATE INDEX
% pg_dump testdumpindex
--
-- PostgreSQL database dump
--
...
CREATE INDEX test_a_idx ON test USING btree (a);

It avoids any ambiguity.

index['amname'] = amname

result = []
for name, idx in indexes.items():
Expand All @@ -2699,6 +2728,10 @@ def get_indexes(self, connection, table_name, schema, **kw):
}
if 'duplicates_constraint' in idx:
entry['duplicates_constraint'] = idx['duplicates_constraint']
if 'options' in idx:
entry.setdefault('dialect_options', {})["postgresql_with"] = idx['options']
if 'amname' in idx:
entry.setdefault('dialect_options', {})["postgresql_using"] = idx['amname']
result.append(entry)
return result

Expand Down
24 changes: 24 additions & 0 deletions test/dialect/postgresql/test_compiler.py
View file
Open in desktop
Expand Up @@ -369,6 +369,30 @@ def test_create_index_with_using(self):
'USING hash (data)',
dialect=postgresql.dialect())

def test_create_index_with_with(self):
m = MetaData()
tbl = Table('testtbl', m, Column('data', String))

idx1 = Index('test_idx1', tbl.c.data)
idx2 = Index('test_idx2', tbl.c.data, postgresql_with={"fillfactor": 50})
idx3 = Index('test_idx3', tbl.c.data, postgresql_using="gist",
postgresql_with={"buffering": "off"})

self.assert_compile(schema.CreateIndex(idx1),
'CREATE INDEX test_idx1 ON testtbl '
'(data)',
dialect=postgresql.dialect())
self.assert_compile(schema.CreateIndex(idx2),
'CREATE INDEX test_idx2 ON testtbl '
'(data) '
'WITH (fillfactor = 50)',
dialect=postgresql.dialect())
self.assert_compile(schema.CreateIndex(idx3),
'CREATE INDEX test_idx3 ON testtbl '
'USING gist (data) '
'WITH (buffering = off)',
dialect=postgresql.dialect())

def test_create_index_expr_gets_parens(self):
m = MetaData()
tbl = Table('testtbl', m, Column('x', Integer), Column('y', Integer))
Expand Down
39 changes: 39 additions & 0 deletions test/dialect/postgresql/test_reflection.py
View file
Open in desktop
Expand Up @@ -12,6 +12,7 @@
from sqlalchemy import exc
import sqlalchemy as sa
from sqlalchemy.dialects.postgresql import base as postgresql
from sqlalchemy.dialects.postgresql import ARRAY


class ForeignTableReflectionTest(fixtures.TablesTest, AssertsExecutionResults):
Expand Down Expand Up @@ -672,6 +673,44 @@ def test_index_reflection_modified(self):
eq_(ind, [{'unique': False, 'column_names': ['y'], 'name': 'idx1'}])
conn.close()

@testing.provide_metadata
def test_index_reflection_with_storage_options(self):
"""reflect indexes with storage options set"""

metadata = self.metadata

t1 = Table('t', metadata,
Column('id', Integer, primary_key=True),
Column('x', Integer)
)
metadata.create_all()
conn = testing.db.connect().execution_options(autocommit=True)
conn.execute("CREATE INDEX idx1 ON t (x) WITH (fillfactor = 50)")

ind = testing.db.dialect.get_indexes(conn, "t", None)
eq_(ind, [{'unique': False, 'column_names': ['x'], 'name': 'idx1',
'dialect_options': {"postgresql_with": {"fillfactor": "50"}}}])
conn.close()

@testing.provide_metadata
def test_index_reflection_with_access_method(self):
"""reflect indexes with storage options set"""

metadata = self.metadata

t1 = Table('t', metadata,
Column('id', Integer, primary_key=True),
Column('x', ARRAY(Integer))
)
metadata.create_all()
conn = testing.db.connect().execution_options(autocommit=True)
conn.execute("CREATE INDEX idx1 ON t USING gin (x)")

ind = testing.db.dialect.get_indexes(conn, "t", None)
eq_(ind, [{'unique': False, 'column_names': ['x'], 'name': 'idx1',
'dialect_options': {'postgresql_using': 'gin'}}])
conn.close()

@testing.provide_metadata
def test_foreign_key_option_inspection(self):
metadata = self.metadata
Expand Down