Skip to content

Commit

Permalink
Add DisallowFloatTypes rule.
Browse files Browse the repository at this point in the history
  • Loading branch information
erik committed Jan 3, 2019
1 parent b5ab6cd commit 7785305
Show file tree
Hide file tree
Showing 3 changed files with 111 additions and 0 deletions.
7 changes: 7 additions & 0 deletions docs/rules.rst
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,13 @@ DisallowChangeColumnType
:show-inheritance:
:exclude-members: enable

DisallowFloatTypes
------------------------
.. autoclass:: squabble.rules.disallow_float_types.DisallowFloatTypes
:members:
:show-inheritance:
:exclude-members: enable

DisallowRenameEnumValue
-----------------------
.. autoclass:: squabble.rules.disallow_rename_enum_value.DisallowRenameEnumValue
Expand Down
89 changes: 89 additions & 0 deletions squabble/rules/disallow_float_types.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
import pglast

import squabble.rule
from squabble.message import Message
from squabble.rules import BaseRule


def _parse_column_type(typ):
"""
Feed column type name through pglast.
>>> _parse_column_type('real')
'pg_catalog.float4'
>>> _parse_column_type('double precision')
'pg_catalog.float8'
"""
sql = 'CREATE TABLE _(_ {0});'.format(typ)

create_table = pglast.Node(pglast.parse_sql(sql))[0].stmt
type_name = create_table.tableElts[0].typeName

return _type_name_to_string(type_name)


def _type_name_to_string(node):
"""
Return a simple stringified version of a ``pglast.node.TypeName`` node.
Note that this won't be suitable for printing, and ignores type
modifiers (e.g. ``NUMERIC(3,4)`` => ``NUMERIC``).
"""

return '.'.join([
part['String']['str']
for part in node.names._items
])


class DisallowFloatTypes(BaseRule):
"""
Prevent using approximate float point number data types.
In SQL, the types ``FLOAT``, ``REAL``, and ``DOUBLE PRECISION`` are
implemented as IEEE 754 floating point numbers, which will not be
able to perfectly represent all numbers within their ranges.
Often, they'll be "good enough", but when doing aggregates over a
large table, or trying to store very large (or very small)
numbers, errors can be exaggerated.
Most of the time, you'll probably want to used a fixed-point
number, such as ``NUMERIC(3, 4)``.
Configuration ::
{ "DisallowFloatTypes": {} }
"""

_INEXACT_TYPES = set(
_parse_column_type(ty)
for ty in ['real', 'float', 'double', 'double precision']
)

class LossyFloatType(Message):
"""
The types ``FLOAT``, ``REAL``, and ``DOUBLE PRECISION`` are
implemented as IEEE 754 floating point numbers, which by
definition will not be able to perfectly represent all numbers
within their ranges.
This is an issue when performing aggregates over large numbers of
rows, as errors can accumulate.
Instead, using the fixed-precision numeric data types (``NUMERIC``,
``DECIMAL``) are likely the right choice for most cases.
"""
TEMPLATE = 'tried to use a lossy float type instead of fixed precision'
CODE = 1007

def enable(self, root_ctx, _config):
root_ctx.register('ColumnDef', self._check_column_def())

@squabble.rule.node_visitor
def _check_column_def(self, ctx, node):
col_type = _type_name_to_string(node.typeName)

if col_type in self._INEXACT_TYPES:
ctx.report(self.LossyFloatType(), node=node)
15 changes: 15 additions & 0 deletions tests/sql/disallow_float_types.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
-- enable:DisallowFloatTypes
-- >>> {"line": 8, "column": 2, "message_id": "LossyFloatType"}
-- >>> {"line": 9, "column": 2, "message_id": "LossyFloatType"}
-- >>> {"line": 15, "column": 2, "message_id": "LossyFloatType"}

CREATE TABLE foo (
-- should not pass
bar REAL,
baz DOUBLE PRECISION,
-- should pass
quux INT
);

ALTER TABLE foo ADD COLUMN
bar FLOAT;

0 comments on commit 7785305

Please sign in to comment.