[CalendarServer-changes] [7350] CalendarServer/branches/users/glyph/oracle-nulls/twext/enterprise
source_changes at macosforge.org
source_changes at macosforge.org
Thu Apr 21 09:53:28 PDT 2011
Revision: 7350
http://trac.macosforge.org/projects/calendarserver/changeset/7350
Author: glyph at apple.com
Date: 2011-04-21 09:53:28 -0700 (Thu, 21 Apr 2011)
Log Message:
-----------
substitute '' for None in appropriate places when querying or manipulating Oracle data.
Modified Paths:
--------------
CalendarServer/branches/users/glyph/oracle-nulls/twext/enterprise/dal/syntax.py
CalendarServer/branches/users/glyph/oracle-nulls/twext/enterprise/dal/test/test_sqlsyntax.py
CalendarServer/branches/users/glyph/oracle-nulls/twext/enterprise/test/test_adbapi2.py
Modified: CalendarServer/branches/users/glyph/oracle-nulls/twext/enterprise/dal/syntax.py
===================================================================
--- CalendarServer/branches/users/glyph/oracle-nulls/twext/enterprise/dal/syntax.py 2011-04-21 16:53:19 UTC (rev 7349)
+++ CalendarServer/branches/users/glyph/oracle-nulls/twext/enterprise/dal/syntax.py 2011-04-21 16:53:28 UTC (rev 7350)
@@ -19,7 +19,7 @@
Syntax wrappers and generators for SQL.
"""
-import itertools
+from itertools import count, repeat
from zope.interface import implements
@@ -72,7 +72,7 @@
def __init__(self, dialect):
super(NumericPlaceholder, self).__init__(dialect)
- self._next = itertools.count(1).next
+ self._next = count(1).next
def placeholder(self):
@@ -121,10 +121,37 @@
def _extraVars(self, txn, metadata):
+ """
+ A hook for subclasses to provide additional keyword arguments to the
+ C{bind} call when L{_Statement.on} is executed. Currently this is used
+ only for 'out' parameters to capture results when executing statements
+ that do not normally have a result (L{Insert}, L{Delete}, L{Update}).
+ """
return {}
def _extraResult(self, result, outvars, metadata):
+ """
+ A hook for subclasses to manipulate the results of 'on', after they've
+ been retrieved by the database but before they've been given to
+ application code.
+
+ @param result: a L{Deferred} that will fire with the rows as returned by
+ the database.
+ @type result: C{list} of rows, which are C{list}s or C{tuple}s.
+
+ @param outvars: a dictionary of extra variables returned by
+ C{self._extraVars}.
+
+ @param metadata: information about the connection where the statement
+ was executed.
+
+ @type metadata: L{ConnectionMetadata} (a subclass thereof)
+
+ @return: the result to be returned from L{_Statement.on}.
+
+ @rtype: L{Deferred} firing result rows
+ """
return result
@@ -132,6 +159,19 @@
"""
Execute this statement on a given L{IAsyncTransaction} and return the
resulting L{Deferred}.
+
+ @param txn: the L{IAsyncTransaction} to execute this on.
+
+ @param raiseOnZeroRowCount: the exception to raise if no data was
+ affected or returned by this query.
+
+ @param kw: keyword arguments, mapping names of L{Parameter} objects
+ located somewhere in C{self}
+
+ @return: results from the database.
+
+ @rtype: a L{Deferred} firing a C{list} of records (C{tuple}s or
+ C{list}s)
"""
metadata = self._paramstyles[txn.paramstyle](txn.dialect)
outvars = self._extraVars(txn, metadata)
@@ -139,10 +179,58 @@
fragment = self.toSQL(metadata).bind(**kw)
result = txn.execSQL(fragment.text, fragment.parameters,
raiseOnZeroRowCount)
- return self._extraResult(result, outvars, metadata)
+ result = self._extraResult(result, outvars, metadata)
+ if metadata.dialect == ORACLE_DIALECT and result:
+ result.addCallback(self._fixOracleNulls)
+ return result
+ def _resultColumns(self):
+ """
+ Subclasses must implement this to return a description of the columns
+ expected to be returned. This is a list of L{ColumnSyntax} objects, and
+ possibly other expression syntaxes which will be converted to C{None}.
+ """
+ raise NotImplementedError(
+ "Each statement subclass must describe its result"
+ )
+
+ def _resultShape(self):
+ """
+ Process the result of the subclass's C{_resultColumns}, as described in
+ the docstring above.
+ """
+ for expectation in self._resultColumns():
+ if isinstance(expectation, ColumnSyntax):
+ yield expectation.model
+ else:
+ yield None
+
+
+ def _fixOracleNulls(self, rows):
+ """
+ Oracle treats empty strings as C{NULL}. Fix this by looking at the
+ columns we expect to have returned, and replacing any C{None}s with
+ empty strings in the appropriate position.
+ """
+ newRows = []
+ for row in rows:
+ newRow = []
+ for column, description in zip(row, self._resultShape()):
+ if ((description is not None and
+ # FIXME: "is the python type str" is what I mean; this list
+ # should be more centrally maintained
+ description.type.name in ('varchar', 'text', 'char') and
+ column is None
+ )):
+ column = ''
+ newRow.append(column)
+ newRows.append(newRow)
+ return newRows
+
+
+
class Syntax(object):
"""
Base class for syntactic convenience.
@@ -719,7 +807,11 @@
return result
+ def _resultColumns(self):
+ # FIXME: ALL_COLUMNS
+ return self.columns.columns
+
def _commaJoined(stmts):
first = True
cstatement = SQLFragment()
@@ -783,7 +875,9 @@
Add a dialect-appropriate 'returning' clause to the end of the given SQL
statement.
- @param metadata: describes the database we are generating the statement for.
+ @param metadata: describes the database we are generating the statement
+ for.
+
@type metadata: L{ConnectionMetadata}
@param stmt: the SQL fragment generated without the 'returning' clause
@@ -839,8 +933,16 @@
return result
+ def _resultColumns(self):
+ return self.Return
+
+
class _OracleOutParam(object):
+ """
+ A parameter that will be populated using the cx_Oracle API for host
+ variables.
+ """
implements(IDerivedParameter)
def __init__(self, columnSyntax):
@@ -848,7 +950,6 @@
def preQuery(self, cursor):
- self.columnSyntax
typeMap = {'integer': cx_Oracle.NUMBER,
'text': cx_Oracle.NCLOB,
'varchar': cx_Oracle.STRING,
@@ -993,8 +1094,20 @@
-class Lock(_Statement):
+class _LockingStatement(_Statement):
"""
+ A statement related to lock management, which implicitly has no results.
+ """
+ def _resultColumns(self):
+ """
+ No columns should be expected, so return an infinite iterator of None.
+ """
+ return repeat(None)
+
+
+
+class Lock(_LockingStatement):
+ """
An SQL 'lock' statement.
"""
@@ -1015,7 +1128,7 @@
-class Savepoint(_Statement):
+class Savepoint(_LockingStatement):
"""
An SQL 'savepoint' statement.
"""
@@ -1028,7 +1141,7 @@
return SQLFragment('savepoint %s' % (self.name,))
-class RollbackToSavepoint(_Statement):
+class RollbackToSavepoint(_LockingStatement):
"""
An SQL 'rollback to savepoint' statement.
"""
@@ -1041,7 +1154,7 @@
return SQLFragment('rollback to savepoint %s' % (self.name,))
-class ReleaseSavepoint(_Statement):
+class ReleaseSavepoint(_LockingStatement):
"""
An SQL 'release savepoint' statement.
"""
Modified: CalendarServer/branches/users/glyph/oracle-nulls/twext/enterprise/dal/test/test_sqlsyntax.py
===================================================================
--- CalendarServer/branches/users/glyph/oracle-nulls/twext/enterprise/dal/test/test_sqlsyntax.py 2011-04-21 16:53:19 UTC (rev 7349)
+++ CalendarServer/branches/users/glyph/oracle-nulls/twext/enterprise/dal/test/test_sqlsyntax.py 2011-04-21 16:53:28 UTC (rev 7350)
@@ -34,6 +34,7 @@
from twext.enterprise.adbapi2 import ConnectionPool
from twext.enterprise.test.test_adbapi2 import resultOf
from twext.enterprise.test.test_adbapi2 import FakeThreadHolder
+from twisted.internet.defer import succeed
from twisted.trial.unittest import TestCase
@@ -49,6 +50,25 @@
+class FakeCXOracleModule(object):
+ NUMBER = 'the NUMBER type'
+ STRING = 'a string type (for varchars)'
+ NCLOB = 'the NCLOB type. (for text)'
+ TIMESTAMP = 'for timestamps!'
+
+
+
+class NullTestingOracleTxn(object):
+ """
+ Fake transaction for testing oracle NULL behavior.
+ """
+
+ dialect = ORACLE_DIALECT
+ paramstyle = 'numeric'
+
+ def execSQL(self, text, params, exc):
+ return succeed([[None, None]])
+
class GenerationTests(TestCase):
"""
Tests for syntactic helpers to generate SQL queries.
@@ -63,7 +83,10 @@
create table OTHER (BAR integer,
FOO_BAR integer not null);
create table TEXTUAL (MYTEXT varchar(255));
- create table LEVELS (ACCESS integer, USERNAME varchar(255));
+ create table LEVELS (ACCESS integer,
+ USERNAME varchar(255));
+ create table NULLCHECK (ASTRING varchar(255) not null,
+ ANUMBER integer);
""")
self.schema = SchemaSyntax(s)
@@ -121,7 +144,7 @@
self.assertRaises(ValueError, sampleComparison)
- def test_nullComparison(self):
+ def test_compareWithNULL(self):
"""
Comparing a column with None results in the generation of an 'is null'
or 'is not null' SQL statement.
@@ -510,18 +533,14 @@
)
- def test_insertMultiReturnOnOracleTxn(self):
+ def simulateOracleConnection(self):
"""
- As described in L{test_insertMultiReturnOracle}, Oracle deals with
- 'returning' clauses by using out parameters. However, this is not quite
- enough, as the code needs to actually retrieve the values from the out
- parameters.
+ Create a fake oracle-ish connection pool without using real threads or a
+ real database.
+
+ @return: a 3-tuple of L{IAsyncTransaction}, L{ConnectionPool},
+ L{ConnectionFactory}.
"""
- class FakeCXOracleModule(object):
- NUMBER = 'the NUMBER type'
- STRING = 'a string type (for varchars)'
- NCLOB = 'the NCLOB type. (for text)'
- TIMESTAMP = 'for timestamps!'
self.patch(syntax, 'cx_Oracle', FakeCXOracleModule)
factory = ConnectionFactory()
pool = ConnectionPool(factory.connect, maxConnections=2,
@@ -531,6 +550,17 @@
pool._createHolder = lambda : FakeThreadHolder(self)
pool.startService()
conn = pool.connection()
+ return conn, pool, factory
+
+
+ def test_insertMultiReturnOnOracleTxn(self):
+ """
+ As described in L{test_insertMultiReturnOracle}, Oracle deals with
+ 'returning' clauses by using out parameters. However, this is not quite
+ enough, as the code needs to actually retrieve the values from the out
+ parameters.
+ """
+ conn, pool, factory = self.simulateOracleConnection()
i = Insert({self.schema.FOO.BAR: 40,
self.schema.FOO.BAZ: 50},
Return=(self.schema.FOO.BAR, self.schema.FOO.BAZ))
@@ -828,6 +858,45 @@
)
+ def test_rewriteOracleNULLs_Select(self):
+ """
+ Oracle databases cannot distinguish between the empty string and
+ C{NULL}. When you insert an empty string, C{cx_Oracle} therefore treats
+ it as a C{None} and will return that when you select it back again. We
+ address this in the schema by dropping 'not null' constraints.
+
+ Therefore, when executing a statement which includes a string column,
+ 'on' should rewrite None return values from C{cx_Oracle} to be empty
+ bytestrings, but only for string columns.
+ """
+
+ rows = resultOf(
+ Select([self.schema.NULLCHECK.ASTRING,
+ self.schema.NULLCHECK.ANUMBER],
+ From=self.schema.NULLCHECK).on(NullTestingOracleTxn()))[0]
+
+ self.assertEquals(rows, [['', None]])
+
+
+ def test_rewriteOracleNULLs_Insert(self):
+ """
+ The behavior described in the previous test applies to other statement
+ types as well, specifically those with 'returning' clauses.
+ """
+ conn, pool, factory = self.simulateOracleConnection()
+ # Add 2 cursor variable values so that these will be used by
+ # FakeVariable.getvalue.
+ factory.varvals.extend([None, None])
+ rows = resultOf(
+ Insert({self.schema.NULLCHECK.ASTRING: '',
+ self.schema.NULLCHECK.ANUMBER: None},
+ Return=[self.schema.NULLCHECK.ASTRING,
+ self.schema.NULLCHECK.ANUMBER]
+ ).on(conn))[0]
+
+ self.assertEquals(rows, [['', None]])
+
+
def test_nestedLogicalExpressions(self):
"""
Make sure that logical operator precedence inserts proper parenthesis
Modified: CalendarServer/branches/users/glyph/oracle-nulls/twext/enterprise/test/test_adbapi2.py
===================================================================
--- CalendarServer/branches/users/glyph/oracle-nulls/twext/enterprise/test/test_adbapi2.py 2011-04-21 16:53:19 UTC (rev 7349)
+++ CalendarServer/branches/users/glyph/oracle-nulls/twext/enterprise/test/test_adbapi2.py 2011-04-21 16:53:28 UTC (rev 7350)
@@ -197,6 +197,9 @@
def getvalue(self):
+ vv = self.cursor.connection.parent.varvals
+ if vv:
+ return vv.pop(0)
return self.cursor.variables.index(self) + 300
@@ -211,6 +214,7 @@
self.idcounter = count(1)
self._connectResultQueue = []
self.defaultConnect()
+ self.varvals = []
@property
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.macosforge.org/pipermail/calendarserver-changes/attachments/20110421/5e3273fc/attachment-0001.html>
More information about the calendarserver-changes
mailing list