On Sat, Jun 09, 2012 at 11:56:45PM -0400, Michael Bayer wrote:
> for _compiler_dispatch, it looks mostly OK - i think "isselect" isn't needed?
> Or if it's meant to accommodate a SELECT that's inside of an UPDATE or
> DELETE, I think you'd need to use a stack based approach so that when you
> exit visit_select() the "isselect" flag is reset. The
> isdelete/isupdate/isinsert flags are meant to be mutually exclusive. I'd
> just peek inside of self.stack, perhaps via the is_subquery() method, or
> something more specific, if you truly need to detect SELECT inside of
> UPDATE/DELETE for that. Looks OK otherwise.
Yes. is_subquery() is just what I needed.
>
> "only()" remains more troubling. I can see the difficulty, it's somewhere
> in between a "hint", and a directive that actually produces a different
> result, which is unlike a hint. But using a new selectable construct adds
> a large amount of complexity that I still see as unwarranted.
>
> Creating a wrapper around a selectable requires that all the Column objects
> be "proxied" into new ones. Column has a ".table" attribute that points to
> the parent table - so you need to do more than just copy over table.columns
> to self._columns - this breaks the Column which you can see if you run a
> regular select against the table after the only():
>
> from sqlalchemy import *
>
> m = MetaData()
> mytable = Table('mytable', m, Column('id', Integer, primary_key=True),
> Column('data', String))
>
> from sqlalchemy.dialects.postgresql import base as postgresql
> mytable_only = postgresql.only(mytable)
> print select([mytable_only.c.id,
> mytable_only.c.data]).compile(dialect=postgresql.dialect())
> print select([mytable.c.id,
> mytable.c.data]).compile(dialect=postgresql.dialect())
>
> output:
>
> SELECT mytable.id, mytable.data
> FROM ONLY mytable
>
> SELECT mytable.id, mytable.data
> FROM ONLY mytable
>
> So at the very least, a construct like "only()" would usually subclass Alias
> so that the column proxying is handled.
>
> Also let's not forget that this feature needs to work with the ORM too. In
> that case, I guess we'd need to use aliased(), something like:
>
> only_of_class = aliased(MyClass, alias=only(MyClass.__table__))
>
> Through all of this, only() as a separate construct still seems unwarranted
> because as far as I can see, only() doesn't represent a new lexical identity
> (which is the main job of a FROM element). That is, you wouldn't have
> something like this:
>
> SELECT * FROM ONLY mytable, mytable WHERE ...?
>
> Assuming I'm understanding ONLY correctly, if you need the same table twice
> in the statement, you still need to use alias(), so not much changes there
> from how things work now (including when using with_hint(), which supports
> aliases). When you say "ONLY mytable", that isn't creating a new lexical
> identity within the statement. You're modifying an existing lexical
> identity, the identity denoted by the Table or Alias. So even though not
> quite a hint, still acts a much more like a hint than a new lexical identity.
>
> The attached patch illustrates some small changes to compiler that lets ONLY
> come right through as a table hint, without the need to change much else.
> Usage is like:
>
> select([mytable.c.id, mytable.c.data]).with_hint(mytable, "ONLY",
> dialect_name="postgresql")
>
> mytable_alias = mytable.alias()
> select([mytable_alias.c.id, mytable_alias.c.data]).with_hint(mytable_alias,
> "ONLY", dialect_name="postgresql")
>
> mytable.delete().with_hint("ONLY", dialect_name="postgresql")
>
> This approach, while stretching the definition of "hint" a bit, makes usage
> of existing functionality and uses an API that is already familiar in other
> use cases, with no dialect import required and no complex table proxying
> going on. We end up making with_hint() a more capable system rather than
> adding an entirely new and narrowly-scoped system elsewhere.
>
> If you approve, we can complete and clean up this approach, write some tests,
> fix up the isselect thing, and all three patches can find their way in.
Well, since you've already done most of the hard work, the hints
approach seems to work perfectly fine :P
I'm attaching two new patches, one for the _compiler_dispatch in
visit_delete/visit_update, and the other for implementing FROM ONLY
using hints. Pretty similar to your patch but with some fixes to make
it work with UPDATE and DELETE as well as proper tests and
documentation.
-Ryan Kelly
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
> On Jun 9, 2012, at 6:51 PM, Ryan Kelly wrote:
>
> > On Sat, May 05, 2012 at 08:00:20PM -0400, Michael Bayer wrote:
> >>
> >> On May 5, 2012, at 7:33 PM, Ryan Kelly wrote:
> >>
> >>> List:
> >>>
> >>> I am currently trying to add support for FROM ONLY to the postgresql
> >>> dialect. FROM ONLY is used when you wish to query a table which has other
> >>> tables that inherit from it, but you ONLY want results from the parent
> >>> table you are referencing. More information can be found here:
> >>> http://www.postgresql.org/docs/current/static/sql-select.html#SQL-FROM
> >>
> >> OK, this looks like a nice patch, though I'm not sure it's consistent with
> >> how we've approached this kind of SQL feature in other cases. "ONLY"
> >> looks
> >> a lot like an optimization hint added to the FROM clause. We already have
> >> an
> >> API for this, called with_hint(). with_hint() is intended to be
> >> supported
> >> by all of INSERT, UPDATE, DELETE in addition to SELECT. I'm not sure
> >> at
> >> the moment what it does with PG right now but it might be a more
> >> appropriate
> >> approach. Take a look and let me know if you have thoughts on that.
> > I had seen that when I started implementing this, but I felt that
> > 'logically'
> > speaking, ONLY was more like an alias than a hint. I tried just now to
> > implement it using hints, but a lot of the code makes assumptions about
> > the location of hints with regards to the table name, i.e., the hint
> > always comes after the table name. ONLY always appears before. And I'm
> > not entirely sure how it would work if the same table is used twice in a
> > statement. ONLY essentially is a different kind of table object. Maybe I'm
> > missing something here.
> >
> >>>
> >>> During the course of implementing this, I found that the visit_delete
> >>> method did not call _compiler_dispatch on the table targeted by the
> >>> delete,
> >>> preventing table aliases in a delete, and preventing my implementation of
> >>> ONLY for delete. I changed this, but the mssql dialect has some strange
> >>> aliasing rules, which needed some TLC to function correctly in the
> >>> presence
> >>> of _compiler_dispatch.
> >>
> >> Also it seems a little awkward that DELETE now defers to generic
> >> compilation
> >> to format the table, but still not INSERT or UPDATE which still hardcode to
> >> preparer.format_table().
> > For update this should probably be changed, and I've attached a patch to
> > do so. I'm not sure how much sense this makes for insert. I don't think you
> > can use anything but the name of the table (schema qualified) in an insert
> > in
> > any DB. I do not believe that hints/aliases/ONLY could ever make sense in
> > this context. Maybe I'm missing something, besides the symmetry aspects of
> > it?
> >
> >>>
> >>> Of course, table aliasing in a delete isn't supported by all dialects, and
> >>> my current implementation doesn't do anything to protect Jimmy[1].
> >>
> >> is aliasing of a table also a different feature need here ? Which DBs
> >> support this ?
> > As far as I know, it works on PostgreSQL and Firebird. It does not work
> > on SQLite, MSSQL, DB2, and Oracle. I have not tried on Sybase.
> >
> >>>
> >>> So there are two patches attached to this email, the first being for
> >>> _compiler_dispatch in visit_delete (compiler_dispatch_deletes.patch) and
> >>> the other for FROM ONLY in postgres (from_only.patch). The second one
> >>> could
> >>> probably be considering more of a work-in-progress and I'm actually
> >>> interested in feedback on whether or not I'm headed in the right direction
> >>> with it. It also depends on the first patch.
> >>
> >>>
> >>> Also, is this the right list?
> >>
> >>
> >> sure, there's also sqlalchemy-devel, though for features/patches you can
> >> also
> >> use trac tickets or bitbucket forks/pull requests....
> >>
> >> thanks for the patches and interest !
> > Welcome. Sorry it took so long to follow up on this, I've been busy.
> >
> > -Ryan Kelly
> >
> >>
> >>
> >>
> >>
> >>
> >>>
> >>> -Ryan Kelly
> >>>
> >>> [1] http://www.globalnerdy.com/2010/05/09/new-programming-jargon/
> >>>
> >>> -- You received this message because you are subscribed to the Google
> >>> Groups "sqlalchemy" group. To post to this group, send email to
> >>> [email protected]. To unsubscribe from this group, send email
> >>> to
> >>> [email protected]. For more options, visit this
> >>> group at http://groups.google.com/group/sqlalchemy?hl=en.
> >>>
> >>> <compiler_dispatch_deletes.patch><from_only.patch>
> >>
> >> -- You received this message because you are subscribed to the Google
> >> Groups
> >> "sqlalchemy" group. To post to this group, send email to
> >> [email protected]. To unsubscribe from this group, send email to
> >> [email protected]. For more options, visit this
> >> group
> >> at http://groups.google.com/group/sqlalchemy?hl=en.
> >>
> >
> > --
> > You received this message because you are subscribed to the Google Groups
> > "sqlalchemy" group.
> > To post to this group, send email to [email protected].
> > To unsubscribe from this group, send email to
> > [email protected].
> > For more options, visit this group at
> > http://groups.google.com/group/sqlalchemy?hl=en.
> >
> > <compiler_dispatch_deletes.patch><from_only.patch>
>
> --
> You received this message because you are subscribed to the Google Groups
> "sqlalchemy" group.
> To post to this group, send email to [email protected].
> To unsubscribe from this group, send email to
> [email protected].
> For more options, visit this group at
> http://groups.google.com/group/sqlalchemy?hl=en.
>
--
You received this message because you are subscribed to the Google Groups
"sqlalchemy" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/sqlalchemy?hl=en.
diff -r ea4bd6b54789 lib/sqlalchemy/dialects/mssql/base.py
--- a/lib/sqlalchemy/dialects/mssql/base.py Fri Jun 08 15:56:58 2012 -0400
+++ b/lib/sqlalchemy/dialects/mssql/base.py Sun Jun 10 12:27:35 2012 -0400
@@ -830,6 +830,10 @@
return super(MSSQLCompiler, self).visit_table(table, **kwargs)
def visit_alias(self, alias, **kwargs):
+ if (self.isupdate and not kwargs.get('mssql_update_from', False)
+ or self.isdelete) and not self.is_subquery():
+ return self.preparer.format_table(alias.original)
+
# translate for schema-qualified table aliases
kwargs['mssql_aliased'] = alias.original
return super(MSSQLCompiler, self).visit_alias(alias, **kwargs)
@@ -951,6 +955,7 @@
well. Otherwise, it is optional. Here, we add it regardless.
"""
+ kw['mssql_update_from'] = True
return "FROM " + ', '.join(
t._compiler_dispatch(self, asfrom=True,
fromhints=from_hints, **kw)
diff -r ea4bd6b54789 lib/sqlalchemy/sql/compiler.py
--- a/lib/sqlalchemy/sql/compiler.py Fri Jun 08 15:56:58 2012 -0400
+++ b/lib/sqlalchemy/sql/compiler.py Sun Jun 10 12:27:35 2012 -0400
@@ -1116,7 +1116,7 @@
MySQL overrides this.
"""
- return self.preparer.format_table(from_table)
+ return from_table._compiler_dispatch(self, asfrom=True, **kw)
def update_from_clause(self, update_stmt,
from_table, extra_froms,
@@ -1423,7 +1423,8 @@
self.stack.append({'from': set([delete_stmt.table])})
self.isdelete = True
- text = "DELETE FROM " + self.preparer.format_table(delete_stmt.table)
+ text = "DELETE FROM "
+ text += delete_stmt.table._compiler_dispatch(self, asfrom=True)
if delete_stmt._hints:
dialect_hints = dict([
@@ -1447,7 +1448,8 @@
delete_stmt, delete_stmt._returning)
if delete_stmt._whereclause is not None:
- text += " WHERE " + self.process(delete_stmt._whereclause)
+ text += " WHERE "
+ text += delete_stmt._whereclause._compiler_dispatch(self)
if self.returning and not self.returning_precedes_values:
text += " " + self.returning_clause(
diff -r ea4bd6b54789 test/sql/test_compiler.py
--- a/test/sql/test_compiler.py Fri Jun 08 15:56:58 2012 -0400
+++ b/test/sql/test_compiler.py Sun Jun 10 12:27:35 2012 -0400
@@ -2866,6 +2866,17 @@
"WHERE mytable.myid = hoho(:hoho_1) AND mytable.name = :param_2 || "
"mytable.name || :param_3")
+ def test_aliased_update(self):
+ talias1 = table1.alias('t1')
+ self.assert_compile(
+ update(talias1, talias1.c.myid == 7),
+ "UPDATE mytable AS t1 SET name=:name WHERE t1.myid = :myid_1",
+ params = {table1.c.name:'fred'})
+ self.assert_compile(
+ update(talias1, table1.c.myid == 7),
+ "UPDATE mytable AS t1 SET name=:name FROM mytable WHERE mytable.myid = :myid_1",
+ params = {table1.c.name:'fred'})
+
def test_correlated_update(self):
# test against a straight text subquery
u = update(table1, values = {
@@ -2948,6 +2959,12 @@
"DELETE FROM mytable WHERE mytable.myid = :myid_1 "
"AND mytable.name = :name_1")
+ def test_aliased_delete(self):
+ talias1 = table1.alias('t1')
+ self.assert_compile(
+ delete(talias1).where(talias1.c.myid == 7),
+ "DELETE FROM mytable AS t1 WHERE t1.myid = :myid_1")
+
def test_correlated_delete(self):
# test a non-correlated WHERE clause
s = select([table2.c.othername], table2.c.otherid == 7)
diff -r a91697d5c523 lib/sqlalchemy/dialects/postgresql/base.py
--- a/lib/sqlalchemy/dialects/postgresql/base.py Sun Jun 10 12:25:18 2012 -0400
+++ b/lib/sqlalchemy/dialects/postgresql/base.py Sun Jun 10 12:25:49 2012 -0400
@@ -114,6 +114,24 @@
where(table.c.name=='foo')
print result.fetchall()
+FROM ONLY ...
+------------------------
+
+The dialect supports PostgreSQL's ONLY keyword for targeting only a particular
+table in an inheritance hierarchy. This can be used to produce the
+``SELECT ... FROM ONLY``, ``UPDATE ONLY ...``, and ``DELETE FROM ONLY ...``
+syntaxes. It uses SQLAlchemy's hints mechanism:
+
+ # SELECT ... FROM ONLY ...
+ result = table.select().with_hint(table, 'ONLY', 'postgresql')
+ print result.fetchall()
+
+ # UPDATE ONLY ...
+ table.update(values=dict(foo='bar')).with_hint('ONLY',
+ dialect_name='postgresql')
+
+ # DELETE FROM ONLY ...
+ table.delete().with_hint('ONLY', dialect_name='postgresql')
.. _postgresql_indexes:
@@ -642,6 +660,16 @@
text += " OFFSET " + self.process(sql.literal(select._offset))
return text
+ def format_from_hint_text(self, sqltext, table, hint):
+ if hint.upper() != 'ONLY':
+ raise CompileError("Unrecognized hint: %r" % hint)
+ return "ONLY " + sqltext
+
+ def format_crud_hint_text(self, sqltext, table, hint):
+ if hint.upper() != 'ONLY':
+ raise CompileError("Unrecognized hint: %r" % hint)
+ return "ONLY " + sqltext
+
def get_select_precolumns(self, select):
if select._distinct is not False:
if select._distinct is True:
diff -r a91697d5c523 lib/sqlalchemy/sql/compiler.py
--- a/lib/sqlalchemy/sql/compiler.py Sun Jun 10 12:25:18 2012 -0400
+++ b/lib/sqlalchemy/sql/compiler.py Sun Jun 10 12:25:49 2012 -0400
@@ -820,9 +820,7 @@
self.preparer.format_alias(alias, alias_name)
if fromhints and alias in fromhints:
- hinttext = self.get_from_hint_text(alias, fromhints[alias])
- if hinttext:
- ret += " " + hinttext
+ ret = self.format_from_hint_text(ret, alias, fromhints[alias])
return ret
else:
@@ -859,6 +857,18 @@
else:
return column
+ def format_from_hint_text(self, sqltext, table, hint):
+ hinttext = self.get_from_hint_text(table, hint)
+ if hinttext:
+ sqltext += " " + hinttext
+ return sqltext
+
+ def format_crud_hint_text(self, sqltext, table, hint):
+ return sqltext + " " + self.get_crud_hint_text(
+ table,
+ hint
+ )
+
def get_select_hint_text(self, byfroms):
return None
@@ -1029,9 +1039,7 @@
else:
ret = self.preparer.quote(table.name, table.quote)
if fromhints and table in fromhints:
- hinttext = self.get_from_hint_text(table, fromhints[table])
- if hinttext:
- ret += " " + hinttext
+ ret = self.format_from_hint_text(ret, table, fromhints[table])
return ret
else:
return ""
@@ -1066,7 +1074,8 @@
if prefixes:
text += " " + " ".join(prefixes)
- text += " INTO " + preparer.format_table(insert_stmt.table)
+ text += " INTO "
+ table_text = preparer.format_table(insert_stmt.table)
if insert_stmt._hints:
dialect_hints = dict([
@@ -1076,11 +1085,14 @@
if dialect in ('*', self.dialect.name)
])
if insert_stmt.table in dialect_hints:
- text += " " + self.get_crud_hint_text(
+ table_text = self.format_crud_hint_text(
+ table_text,
insert_stmt.table,
dialect_hints[insert_stmt.table]
)
+ text += table_text
+
if colparams or not supports_default_values:
text += " (%s)" % ', '.join([preparer.format_column(c[0])
for c in colparams])
@@ -1142,10 +1154,9 @@
colparams = self._get_colparams(update_stmt, extra_froms)
- text = "UPDATE " + self.update_tables_clause(
- update_stmt,
- update_stmt.table,
- extra_froms, **kw)
+ text = "UPDATE "
+ table_text = self.update_tables_clause(update_stmt, update_stmt.table,
+ extra_froms, **kw)
if update_stmt._hints:
dialect_hints = dict([
@@ -1155,13 +1166,16 @@
if dialect in ('*', self.dialect.name)
])
if update_stmt.table in dialect_hints:
- text += " " + self.get_crud_hint_text(
+ table_text = self.format_crud_hint_text(
+ table_text,
update_stmt.table,
dialect_hints[update_stmt.table]
)
else:
dialect_hints = None
+ text += table_text
+
text += ' SET '
if extra_froms and self.render_table_with_column_in_update_from:
text += ', '.join(
@@ -1424,7 +1438,7 @@
self.isdelete = True
text = "DELETE FROM "
- text += delete_stmt.table._compiler_dispatch(self, asfrom=True)
+ table_text = delete_stmt.table._compiler_dispatch(self, asfrom=True)
if delete_stmt._hints:
dialect_hints = dict([
@@ -1434,13 +1448,17 @@
if dialect in ('*', self.dialect.name)
])
if delete_stmt.table in dialect_hints:
- text += " " + self.get_crud_hint_text(
- delete_stmt.table,
+ table_text = self.format_crud_hint_text(
+ table_text,
+ delete_stmt.table,
dialect_hints[delete_stmt.table]
)
+
else:
dialect_hints = None
+ text += table_text
+
if delete_stmt._returning:
self.returning = delete_stmt._returning
if self.returning_precedes_values:
diff -r a91697d5c523 test/dialect/test_postgresql.py
--- a/test/dialect/test_postgresql.py Sun Jun 10 12:25:18 2012 -0400
+++ b/test/dialect/test_postgresql.py Sun Jun 10 12:25:49 2012 -0400
@@ -272,6 +272,51 @@
self.assert_compile(x,
'''SELECT pg_table.col1, pg_table."variadic" FROM pg_table''')
+ def test_from_only(self):
+ m = MetaData()
+ tbl1 = Table('testtbl1', m, Column('id', Integer))
+ tbl2 = Table('testtbl2', m, Column('id', Integer))
+
+ stmt = tbl1.select().with_hint(tbl1, 'ONLY', 'postgresql')
+ expected = 'SELECT testtbl1.id FROM ONLY testtbl1'
+ self.assert_compile(stmt, expected)
+
+ talias1 = tbl1.alias('foo')
+ stmt = talias1.select().with_hint(talias1, 'ONLY', 'postgresql')
+ expected = 'SELECT foo.id FROM ONLY testtbl1 AS foo'
+ self.assert_compile(stmt, expected)
+
+ stmt = select([tbl1, tbl2]).with_hint(tbl1, 'ONLY', 'postgresql')
+ expected = ('SELECT testtbl1.id, testtbl2.id FROM ONLY testtbl1, '
+ 'testtbl2')
+ self.assert_compile(stmt, expected)
+
+ stmt = select([tbl1, tbl2]).with_hint(tbl2, 'ONLY', 'postgresql')
+ expected = ('SELECT testtbl1.id, testtbl2.id FROM testtbl1, ONLY '
+ 'testtbl2')
+ self.assert_compile(stmt, expected)
+
+ stmt = select([tbl1, tbl2])
+ stmt = stmt.with_hint(tbl1, 'ONLY', 'postgresql')
+ stmt = stmt.with_hint(tbl2, 'ONLY', 'postgresql')
+ expected = ('SELECT testtbl1.id, testtbl2.id FROM ONLY testtbl1, '
+ 'ONLY testtbl2')
+ self.assert_compile(stmt, expected)
+
+ stmt = update(tbl1, values=dict(id=1))
+ stmt = stmt.with_hint('ONLY', dialect_name='postgresql')
+ expected = 'UPDATE ONLY testtbl1 SET id=%(id)s'
+ self.assert_compile(stmt, expected)
+
+ stmt = delete(tbl1).with_hint('ONLY', dialect_name='postgresql')
+ expected = 'DELETE FROM ONLY testtbl1'
+ self.assert_compile(stmt, expected)
+
+ tbl3 = Table('testtbl3', m, Column('id', Integer), schema='testschema')
+ stmt = tbl3.select().with_hint(tbl3, 'ONLY', 'postgresql')
+ expected = 'SELECT testschema.testtbl3.id FROM ONLY testschema.testtbl3'
+ self.assert_compile(stmt, expected)
+
class FloatCoercionTest(fixtures.TablesTest, AssertsExecutionResults):
__only_on__ = 'postgresql'
