A bug in PgNumeric from current CVS head.

Adam Buraczewski <adamb-Zr0aIzfuVDxmR6Xm/[email protected]>
Newsgroups gmane.comp.python.db.pypgsql.user
Message-ID <[email protected]>
Hallo everyone,

I've just seen some new changes in PgNumeric code on CVS and tried to
play a bit with it :)  Generally it works nice, I hope pyPgSQL 2.3
would be the point where I start switching my applications to use this
class more heavily.  However, I think I found some bugs in it.  Look
at this:

	>>> from pyPgSQL import PgSQL
	>>> print PgSQL.PgNumeric('0.0000') - PgSQL.PgNumeric('0.0010')
	0.0-10
	>>> print PgSQL.PgNumeric('0.0000') + PgSQL.PgNumeric('-0.0020')
	0.0-20
	>>> print PgSQL.PgNumeric('-0.0010')
	0.0-10

(the same happens when -= and += operators are used).  It looks as the
PgNumeric class has some troubles with displaying negative results of
fractional values (in the range of -1 to 0).  After digging into the
code I found that __fmtNumeric() function does not care of negative
numbers at all and treats minus sign as a digit.  The solution is to
find if the number is negative, remove the '-' sign from the
beginning, format as usual and add the sign at the beginning of the
resulting string.

The other, minor problem is with numbers which have a decimal dot but
don't have any digits after it:

	>>> print PgSQL.PgNumeric('1.')
	Traceback (most recent call last):
  	File "<stdin>", line 1, in ?
  	File "/usr/lib/python2.2/site-packages/pyPgSQL/PgSQL.py", line 1297, in __init__
	    raise ValueError, \
	ValueError: invalid literal for PgNumeric: 1.

I think that there is also another problem with the constructor, which
causes the class to treat minus sign as a digit which counts into
precision of the number.  I think it shouldn't.  Look at this:

	>>> print repr(PgSQL.PgNumeric('0.0010'))
	<PgNumeric instance - precision: 5 scale: 4 value: 0.0010
	>>> print repr(PgSQL.PgNumeric('-0.0010'))
	<PgNumeric instance - precision: 6 scale: 4 value: -0.0010
	>>> print repr(PgSQL.PgNumeric(-0.001))
	<PgNumeric instance - precision: 5 scale: 3 value: -0.001
	>>> print repr(PgSQL.PgNumeric(0.001))
	<PgNumeric instance - precision: 4 scale: 3 value: 0.001

I tried to remove all bugs mentioned above (patch against current CVS
attached), cleaning up the code and adding some checks and comments
during that.  Now it works correctly:

	>>> print repr(PgSQL.PgNumeric('-0.0010'))
	<PgNumeric instance - precision: 5 scale: 4 value: -0.0010
	>>> print PgSQL.PgNumeric('1.')
	1

However I am not sure if I correctly understood the meaning of prec
parameter passed to PgNumeric constructor.  What should happen when
someone calls it with setting to something else than None?  Please,
check my patch before applying.

BTW, I wonder why __repr__ operator of all pyPgSQL classes returns
strings of the form "<something" instead of "<something>" (without
closing '>' sign)?  Is it made on purpose or is it a mistake? :)

Regards,

-- 
Adam Buraczewski <adamb-Zr0aIzfuVDxmR6Xm/[email protected]> * Linux registered user #165585
GCS/TW d- s-:+>+:- a- C+++(++++) UL++++$ P++ L++++ E++ W+ N++ o? K? w--
O M- V- PS+ !PE Y PGP+ t+ 5 X+ R tv- b+ DI? D G++ e+++>++++ h r+>++ y?
PgSQL.py.diff (text/plain, 8.9 KB)
*** PgSQL.py.orig	Sun Dec  1 23:12:30 2002
--- PgSQL.py	Tue Dec  3 19:24:48 2002
***************
*** 1213,1303 ****
  	    else:
  		self.__v = long(value)
  	    self.__p = prec
  	    self.__s = scale
          elif type(value) is FloatType:
              _s = repr(value)
              _d = _s.rfind('.')
              _e = _s.rfind('e')
              _exp = 0
              if (_e >= 0):
                  _exp = int(_s[_e+1:])
                  _s = _s[:_e]
              if _exp > 31:
!                 raise OverflowError, "float too large for PgNumeric"
              if _exp < -31:
!                 raise OverflowError, "float too small for PgNumeric"
              _s = _s[:_d] + _s[_d+1:]
!             if _exp == 0:
!                 _sc = len(_s) - _d
!             elif _exp > 0:
!                 _s = _s + ("0" * (_exp - len(_s) + 1))
                  _sc = 0
              else:
!                 _sc = -_exp + len(_s) - 1
              self.__v = long(_s)
!             self.__p = len(_s)
!             if self.__p < _sc:
!                 self.__p = _sc
!             self.__s = _sc
  	elif type(value) is StringType:
!             _v = value.split()
!             if len(_v) == 0 or len(_v) > 1:
                  raise ValueError, \
                        "invalid literal for PgNumeric: %s" % value
-             _v = _v[0]
  
!             # At this point _v is value with leading and trailing blanks
!             # removed.  Initalize the precision and scale values.  They will be
!             # determined from the input string (value) is prec and scale are
!             # None.
! 
!             _vs = value.rfind('.') # Get the location of the decimal point.
!             			   # It's used to determine the precision and
!             if prec:		   # scale if they aren't passed in, and to
!                 self.__p = prec	   # adjust the input string to match the
!             else:		   # passed in scale.
!                 self.__p = len(value)
!                 if _vs >= 0:
!                     self.__p = self.__p - 1
  
!             # Calculate the scale of the passed in string.  _vs will contain the
!             # calulated scale.
  
!             if _vs >= 0:
!                 _vs = len(value) - _vs - 1
!             else:
!                 _vs = 0
                  
!             if scale:
                  self.__s = scale
!             else:
!                 self.__s = _vs
  
              # Calculate the number of character to add/remove from the end of
              # the input string in order to have it match the given scale.  _sd
              # will contain the number of characters to add (>0) or remove (<0).
! 
!             _sd = self.__s - _vs
! 
              if _sd == 0:
                  pass			# No change to value needed.
              elif _sd > 0:
!                 _v = _v + ('0' * _sd)	# Add needed zeros to the end of value
              else:
!                 _v = _v[:_sd]		# Remove excess digits from the end.
  
!             if self.__s:
!                 _s = _v[:-(self.__s + 1)] + _v[-self.__s:]
              else:
!                 _s = _v
!                 
              try:
                  self.__v = long(_s)
              except:
                  raise ValueError, \
                        "invalid literal for PgNumeric: %s" % value
          elif isinstance(value, PgNumeric):
              # This is used to "cast" a PgNumeric to the specified precision
              # and scale.  It can also make a copy of a PgNumeric.
              self.__v = value.__v
              if scale:
--- 1213,1328 ----
  	    else:
  		self.__v = long(value)
  	    self.__p = prec
  	    self.__s = scale
          elif type(value) is FloatType:
+ 
+ 	    if prec is not None or scale is not None:
+ 		raise TypeError, \
+ 		      "you shouldn't supply precision and scale when value is a " \
+ 		      "float"
+ 
+ 	    # Take the text representation of the float number.
              _s = repr(value)
+ 
+ 	    # Find the sign, mantissa and exponent of the number.
              _d = _s.rfind('.')
              _e = _s.rfind('e')
+ 	    _sign = (_s[0] == '-' and '-') or '+'
+ 
+ 	    # Extract the exponent and leave the mantissa (the minus sign and
+ 	    # the decimal point are left untouched).
              _exp = 0
              if (_e >= 0):
                  _exp = int(_s[_e+1:])
                  _s = _s[:_e]
              if _exp > 31:
!                 raise OverflowError, "float exponent too large for PgNumeric"
              if _exp < -31:
!                 raise OverflowError, "float exponent too small for PgNumeric"
! 
! 	    # Remove the decimal point from the mantissa.
! 	    if _d < 0: _d = len(_s) # Decimal point can be absent from _s.
              _s = _s[:_d] + _s[_d+1:]
! 
! 	    # Convert the number from exponential notation and calculate its
! 	    # scale and precision.
! 	    _p = len(_s) - ((_sign == '-' and 1) or 0)
!             _sc = len(_s) - _d
!             if _exp > _sc:
!                 _s = _s + ("0" * (_exp - _sc))
! 		_p = _p + _exp - _sc
                  _sc = 0
              else:
!                 _sc = _sc - _exp
! 
              self.__v = long(_s)
!             self.__p = _p
! 	    self.__s = _sc
! 
  	elif type(value) is StringType:
! 
! 	    if (prec is not None and prec <= 0) or (scale is not None and scale < 0):
! 		raise TypeError, \
! 		      "you shouldn't supply negative precision or scale values" \
! 
! 	    # Remove blanks and check if the rest of the string is reasonable.
!             _s = value.strip()
!             if len(_s) == 0 or len(filter(lambda c: c.isspace(), _s)) > 0:
                  raise ValueError, \
                        "invalid literal for PgNumeric: %s" % value
  
! 	    # Check the sign of the number and find the position of the decimal
! 	    # point.
! 	    _sign = (_s[0] == '-' and '-') or '+'
! 	    _d = _s.rfind('.')
  
! 	    # Remove the decimal point.
! 	    if _d < 0: _d = len(_s) # Decimal point can be absent from _s.
!             _s = _s[:_d] + _s[_d+1:]
  
!             # Calculate the precision and scale of the passed in string.
! 	    _p = len(_s) - ((_sign == '-' and 1) or 0)
!             _sc = len(_s) - _d
                  
! 	    # Check if the number has at least one digit ;)
! 	    if _p == 0:
!                 raise ValueError, \
!                       "invalid literal for PgNumeric: %s" % value
! 
! 	    # Initalize the scale value.
!             if scale is not None:
                  self.__s = scale
! 	    else:
! 	        self.__s = _sc
  
              # Calculate the number of character to add/remove from the end of
              # the input string in order to have it match the given scale.  _sd
              # will contain the number of characters to add (>0) or remove (<0).
!             _sd = self.__s - _sc
              if _sd == 0:
                  pass			# No change to value needed.
              elif _sd > 0:
!                 _s = _s + ('0' * _sd)	# Add needed zeros to the end of value
              else:
!                 _s = _s[:_sd]		# Remove excess digits from the end.
! 	    _p = _p + _sd
  
! 	    # Initalize the prec value.  It cannot be done earlier, because
! 	    # precision changes when the number is adjusted to reflect passed
! 	    # scale parameter.
!             if prec is not None:
!                 self.__p = prec
              else:
!                 self.__p = _p
! 
              try:
                  self.__v = long(_s)
              except:
                  raise ValueError, \
                        "invalid literal for PgNumeric: %s" % value
+ 
          elif isinstance(value, PgNumeric):
              # This is used to "cast" a PgNumeric to the specified precision
              # and scale.  It can also make a copy of a PgNumeric.
              self.__v = value.__v
              if scale:
***************
*** 1337,1354 ****
  	else:
  	    _v = str(value)
  	if _v[-1:] == 'L':
  	    _v = _v[:-1]
  
! 	# Check to see if the numeric is less than zero and fix string if so.
  	if len(_v) <= self.__s:
  	    _v = ("0" * (self.__s - len(_v) + 1)) + _v
  
  	if self.__s:
! 	    _s = "%s.%s" % (_v[:-(self.__s)], _v[-(self.__s):])
  	else:
! 	    _s = "%s" % _v
  	return _s
  
      def __repr__(self):
  	return "<PgNumeric instance - precision: %d scale: %d value: %s" % \
  	       (self.__p, self.__s, self.__fmtNumeric())
--- 1362,1387 ----
  	else:
  	    _v = str(value)
  	if _v[-1:] == 'L':
  	    _v = _v[:-1]
  
! 	# Check if the numeric is negative, store this information and fix the
! 	# string appropriately.
! 	if _v[0] == '-':
! 	    _sign = '-'
! 	    _v = _v[1:]
! 	else:
! 	    _sign = ''
! 
! 	# Check to see if the numeric is less than one and fix string if so.
  	if len(_v) <= self.__s:
  	    _v = ("0" * (self.__s - len(_v) + 1)) + _v
  
  	if self.__s:
! 	    _s = "%s%s.%s" % (_sign, _v[:-(self.__s)], _v[-(self.__s):])
  	else:
! 	    _s = "%s%s" % (_sign, _v)
  	return _s
  
      def __repr__(self):
  	return "<PgNumeric instance - precision: %d scale: %d value: %s" % \
  	       (self.__p, self.__s, self.__fmtNumeric())
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.