From: Mike Bayer Date: Fri, 30 Sep 2016 14:09:56 +0000 (-0400) Subject: Add "eager_parenthesis" late-compilation rule, use w/ PG JSON/HSTORE X-Git-Tag: rel_1_1_0~12 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=333414fe94941a6a58e7d8e45042548eb2d58119;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git Add "eager_parenthesis" late-compilation rule, use w/ PG JSON/HSTORE Added compiler-level flags used by Postgresql to place additional parenthesis than would normally be generated by precedence rules around operations involving JSON, HSTORE indexing operators as well as within their operands since it has been observed that Postgresql's precedence rules for at least the HSTORE indexing operator is not consistent between 9.4 and 9.5. Fixes: #3806 Change-Id: I5899677b330595264543b055abd54f3c76bfabf2 --- diff --git a/doc/build/changelog/changelog_11.rst b/doc/build/changelog/changelog_11.rst index af21717079..c2dd2d8483 100644 --- a/doc/build/changelog/changelog_11.rst +++ b/doc/build/changelog/changelog_11.rst @@ -40,6 +40,17 @@ those of the "excluded" namespace would not be table-qualified in the WHERE clauses in the statement. + .. change:: + :tags: bug, sql, postgresql + :tickets: 3806 + + Added compiler-level flags used by Postgresql to place additional + parenthesis than would normally be generated by precedence rules + around operations involving JSON, HSTORE indexing operators as well as + within their operands since it has been observed that Postgresql's + precedence rules for at least the HSTORE indexing operator is not + consistent between 9.4 and 9.5. + .. change:: :tags: bug, sql, mysql :tickets: 3803 diff --git a/lib/sqlalchemy/dialects/postgresql/base.py b/lib/sqlalchemy/dialects/postgresql/base.py index a9f11aae01..bde855fbe6 100644 --- a/lib/sqlalchemy/dialects/postgresql/base.py +++ b/lib/sqlalchemy/dialects/postgresql/base.py @@ -1270,11 +1270,13 @@ class PGCompiler(compiler.SQLCompiler): ) def visit_json_getitem_op_binary(self, binary, operator, **kw): + kw['eager_grouping'] = True return self._generate_generic_binary( binary, " -> ", **kw ) def visit_json_path_getitem_op_binary(self, binary, operator, **kw): + kw['eager_grouping'] = True return self._generate_generic_binary( binary, " #> ", **kw ) diff --git a/lib/sqlalchemy/dialects/postgresql/hstore.py b/lib/sqlalchemy/dialects/postgresql/hstore.py index 67923fe39e..d3ff30efbe 100644 --- a/lib/sqlalchemy/dialects/postgresql/hstore.py +++ b/lib/sqlalchemy/dialects/postgresql/hstore.py @@ -16,29 +16,36 @@ from ... import util __all__ = ('HSTORE', 'hstore') +idx_precedence = operators._PRECEDENCE[operators.json_getitem_op] GETITEM = operators.custom_op( - "->", precedence=15, natural_self_precedent=True, + "->", precedence=idx_precedence, natural_self_precedent=True, + eager_grouping=True ) HAS_KEY = operators.custom_op( - "?", precedence=15, natural_self_precedent=True + "?", precedence=idx_precedence, natural_self_precedent=True, + eager_grouping=True ) HAS_ALL = operators.custom_op( - "?&", precedence=15, natural_self_precedent=True + "?&", precedence=idx_precedence, natural_self_precedent=True, + eager_grouping=True ) HAS_ANY = operators.custom_op( - "?|", precedence=15, natural_self_precedent=True + "?|", precedence=idx_precedence, natural_self_precedent=True, + eager_grouping=True ) CONTAINS = operators.custom_op( - "@>", precedence=15, natural_self_precedent=True + "@>", precedence=idx_precedence, natural_self_precedent=True, + eager_grouping=True ) CONTAINED_BY = operators.custom_op( - "<@", precedence=15, natural_self_precedent=True + "<@", precedence=idx_precedence, natural_self_precedent=True, + eager_grouping=True ) diff --git a/lib/sqlalchemy/dialects/postgresql/json.py b/lib/sqlalchemy/dialects/postgresql/json.py index 05c4d014d3..821018471c 100644 --- a/lib/sqlalchemy/dialects/postgresql/json.py +++ b/lib/sqlalchemy/dialects/postgresql/json.py @@ -17,33 +17,42 @@ from ... import util __all__ = ('JSON', 'JSONB') +idx_precedence = operators._PRECEDENCE[operators.json_getitem_op] + ASTEXT = operators.custom_op( - "->>", precedence=15, natural_self_precedent=True, + "->>", precedence=idx_precedence, natural_self_precedent=True, + eager_grouping=True ) JSONPATH_ASTEXT = operators.custom_op( - "#>>", precedence=15, natural_self_precedent=True, + "#>>", precedence=idx_precedence, natural_self_precedent=True, + eager_grouping=True ) HAS_KEY = operators.custom_op( - "?", precedence=15, natural_self_precedent=True + "?", precedence=idx_precedence, natural_self_precedent=True, + eager_grouping=True ) HAS_ALL = operators.custom_op( - "?&", precedence=15, natural_self_precedent=True + "?&", precedence=idx_precedence, natural_self_precedent=True, + eager_grouping=True ) HAS_ANY = operators.custom_op( - "?|", precedence=15, natural_self_precedent=True + "?|", precedence=idx_precedence, natural_self_precedent=True, + eager_grouping=True ) CONTAINS = operators.custom_op( - "@>", precedence=15, natural_self_precedent=True + "@>", precedence=idx_precedence, natural_self_precedent=True, + eager_grouping=True ) CONTAINED_BY = operators.custom_op( - "<@", precedence=15, natural_self_precedent=True + "<@", precedence=idx_precedence, natural_self_precedent=True, + eager_grouping=True ) diff --git a/lib/sqlalchemy/sql/compiler.py b/lib/sqlalchemy/sql/compiler.py index a2dbcee5c3..6527eb8c65 100644 --- a/lib/sqlalchemy/sql/compiler.py +++ b/lib/sqlalchemy/sql/compiler.py @@ -994,7 +994,9 @@ class SQLCompiler(Compiled): return "NOT %s" % self.visit_binary( binary, override_operator=operators.match_op) - def visit_binary(self, binary, override_operator=None, **kw): + def visit_binary(self, binary, override_operator=None, + eager_grouping=False, **kw): + # don't allow "? = ?" to render if self.ansi_bind_rules and \ isinstance(binary.left, elements.BindParameter) and \ @@ -1014,6 +1016,7 @@ class SQLCompiler(Compiled): return self._generate_generic_binary(binary, opstring, **kw) def visit_custom_op_binary(self, element, operator, **kw): + kw['eager_grouping'] = operator.eager_grouping return self._generate_generic_binary( element, " " + operator.opstring + " ", **kw) @@ -1025,10 +1028,21 @@ class SQLCompiler(Compiled): return self._generate_generic_unary_modifier( element, " " + operator.opstring, **kw) - def _generate_generic_binary(self, binary, opstring, **kw): - return binary.left._compiler_dispatch(self, **kw) + \ + def _generate_generic_binary( + self, binary, opstring, eager_grouping=False, **kw): + + _in_binary = kw.get('_in_binary', False) + + kw['_in_binary'] = True + text = binary.left._compiler_dispatch( + self, eager_grouping=eager_grouping, **kw) + \ opstring + \ - binary.right._compiler_dispatch(self, **kw) + binary.right._compiler_dispatch( + self, eager_grouping=eager_grouping, **kw) + + if _in_binary and eager_grouping: + text = "(%s)" % text + return text def _generate_generic_unary_operator(self, unary, opstring, **kw): return opstring + unary.element._compiler_dispatch(self, **kw) @@ -2215,6 +2229,12 @@ class StrSQLCompiler(SQLCompiler): self.process(binary.right, **kw) ) + def visit_json_getitem_op_binary(self, binary, operator, **kw): + return self.visit_getitem_binary(binary, operator, **kw) + + def visit_json_path_getitem_op_binary(self, binary, operator, **kw): + return self.visit_getitem_binary(binary, operator, **kw) + def returning_clause(self, stmt, returning_cols): columns = [ self._label_select_column(None, c, True, False, {}) diff --git a/lib/sqlalchemy/sql/operators.py b/lib/sqlalchemy/sql/operators.py index 1426066804..69eee28abe 100644 --- a/lib/sqlalchemy/sql/operators.py +++ b/lib/sqlalchemy/sql/operators.py @@ -215,11 +215,12 @@ class custom_op(object): def __init__( self, opstring, precedence=0, is_comparison=False, - natural_self_precedent=False): + natural_self_precedent=False, eager_grouping=False): self.opstring = opstring self.precedence = precedence self.is_comparison = is_comparison self.natural_self_precedent = natural_self_precedent + self.eager_grouping = eager_grouping def __eq__(self, other): return isinstance(other, custom_op) and \ @@ -935,9 +936,10 @@ _PRECEDENCE = { from_: 15, any_op: 15, all_op: 15, + getitem: 15, json_getitem_op: 15, json_path_getitem_op: 15, - getitem: 15, + mul: 8, truediv: 8, div: 8, @@ -985,6 +987,7 @@ _PRECEDENCE = { as_: -1, exists: 0, + _asbool: -10, _smallest: _smallest, _largest: _largest diff --git a/test/dialect/postgresql/test_types.py b/test/dialect/postgresql/test_types.py index 6bcc4cf9ad..b611c4a9df 100644 --- a/test/dialect/postgresql/test_types.py +++ b/test/dialect/postgresql/test_types.py @@ -1816,7 +1816,7 @@ class HStoreTest(AssertsCompiledSQL, fixtures.TestBase): def test_where_getitem(self): self._test_where( self.hashcol['bar'] == None, - "test_table.hash -> %(hash_1)s IS NULL" + "(test_table.hash -> %(hash_1)s) IS NULL" ) def test_cols_get(self): @@ -1902,6 +1902,12 @@ class HStoreTest(AssertsCompiledSQL, fixtures.TestBase): "(test_table.hash || test_table.hash) -> %(param_1)s AS anon_1" ) + def test_cols_against_is(self): + self._test_cols( + self.hashcol['foo'] != None, + "(test_table.hash -> %(hash_1)s) IS NOT NULL AS anon_1" + ) + def test_cols_keys(self): self._test_cols( # hide from 2to3 @@ -2436,13 +2442,13 @@ class JSONTest(AssertsCompiledSQL, fixtures.TestBase): def test_where_getitem(self): self._test_where( self.jsoncol['bar'] == None, - "test_table.test_column -> %(test_column_1)s IS NULL" + "(test_table.test_column -> %(test_column_1)s) IS NULL" ) def test_where_path(self): self._test_where( self.jsoncol[("foo", 1)] == None, - "test_table.test_column #> %(test_column_1)s IS NULL" + "(test_table.test_column #> %(test_column_1)s) IS NULL" ) def test_path_typing(self): @@ -2481,27 +2487,27 @@ class JSONTest(AssertsCompiledSQL, fixtures.TestBase): def test_where_getitem_as_text(self): self._test_where( self.jsoncol['bar'].astext == None, - "test_table.test_column ->> %(test_column_1)s IS NULL" + "(test_table.test_column ->> %(test_column_1)s) IS NULL" ) def test_where_getitem_astext_cast(self): self._test_where( self.jsoncol['bar'].astext.cast(Integer) == 5, - "CAST(test_table.test_column ->> %(test_column_1)s AS INTEGER) " + "CAST((test_table.test_column ->> %(test_column_1)s) AS INTEGER) " "= %(param_1)s" ) def test_where_getitem_json_cast(self): self._test_where( self.jsoncol['bar'].cast(Integer) == 5, - "CAST(test_table.test_column -> %(test_column_1)s AS INTEGER) " + "CAST((test_table.test_column -> %(test_column_1)s) AS INTEGER) " "= %(param_1)s" ) def test_where_path_as_text(self): self._test_where( self.jsoncol[("foo", 1)].astext == None, - "test_table.test_column #>> %(test_column_1)s IS NULL" + "(test_table.test_column #>> %(test_column_1)s) IS NULL" ) def test_cols_get(self): @@ -2781,6 +2787,13 @@ class JSONRoundTripTest(fixtures.TablesTest): ).first() eq_(result, ({'k1': 'r3v1', 'k2': 'r3v2'},)) + result = engine.execute( + select([data_table.c.data]).where( + data_table.c.data['k1'].astext.cast(String) == 'r3v1' + ) + ).first() + eq_(result, ({'k1': 'r3v1', 'k2': 'r3v2'},)) + def _test_fixed_round_trip(self, engine): s = select([ cast( diff --git a/test/sql/test_operators.py b/test/sql/test_operators.py index 99f8a10ca3..bbc912ffd7 100644 --- a/test/sql/test_operators.py +++ b/test/sql/test_operators.py @@ -670,13 +670,13 @@ class JSONIndexOpTest(fixtures.TestBase, testing.AssertsCompiledSQL): def visit_json_getitem_op_binary(self, binary, operator, **kw): return self._generate_generic_binary( - binary, " -> ", **kw + binary, " -> ", eager_grouping=True, **kw ) def visit_json_path_getitem_op_binary( self, binary, operator, **kw): return self._generate_generic_binary( - binary, " #> ", **kw + binary, " #> ", eager_grouping=True, **kw ) def visit_getitem_binary(self, binary, operator, **kw): @@ -748,12 +748,37 @@ class JSONIndexOpTest(fixtures.TestBase, testing.AssertsCompiledSQL): checkparams={} ) + def test_getindex_sqlexpr_right_grouping(self): + + col = Column('x', self.MyType()) + col2 = Column('y', Integer()) + self.assert_compile( col[col2 + 8], "x -> (y + :y_1)", checkparams={'y_1': 8} ) + def test_getindex_sqlexpr_left_grouping(self): + + col = Column('x', self.MyType()) + + self.assert_compile( + col[8] != None, + "(x -> :x_1) IS NOT NULL" + ) + + def test_getindex_sqlexpr_both_grouping(self): + + col = Column('x', self.MyType()) + col2 = Column('y', Integer()) + + self.assert_compile( + col[col2 + 8] != None, + "(x -> (y + :y_1)) IS NOT NULL", + checkparams={'y_1': 8} + ) + def test_override_operators(self): special_index_op = operators.custom_op('$$>')