Slaying the HYPOTamus - Mailing list pgsql-hackers

Like some ancient precursor to the modern hypot()enuse the dreaded
ill-tempered HYPOT()amus lurks in the  recesses of geo_ops.c and
geo_decls.h. And many a line of code has been swallowed by its mighty maw.

This patch replaces the HYPOT() macro with a calls to the hypot() function.

The hypot() function has been part of the C standard since C99 (ie 10
years ago), and in most UNIX, IBM and GNU libraries longer than that.
The function is designed not to fail where the current naive macro would
result in overflow. Where available, we might expect to the hypot()
function to take advantage of underlying hardware opcodes. In addition
the function evaluates its arguments only once. The current macro
evaluates its arguments twice.

Currently
  HYPOT(a.x - b.x, a.y - b.y))
becomes:
  sqrt((a.x - b.x)*(a.x - b.x) + (a.y - b.y) * (a.y - b.y))

The patch passes all test.

Patch attached below. (First attempt at using CVS. Hope it's all good)


? GNUmakefile
? config.log
? config.status
? patchfile
? src/Makefile.global
? src/backend/postgres
? src/backend/catalog/postgres.bki
? src/backend/catalog/postgres.description
? src/backend/catalog/postgres.shdescription
? src/backend/snowball/snowball_create.sql
? src/backend/utils/probes.h
? src/backend/utils/mb/conversion_procs/conversion_create.sql
? src/bin/initdb/initdb
? src/bin/pg_config/pg_config
? src/bin/pg_controldata/pg_controldata
? src/bin/pg_ctl/pg_ctl
? src/bin/pg_dump/pg_dump
? src/bin/pg_dump/pg_dumpall
? src/bin/pg_dump/pg_restore
? src/bin/pg_resetxlog/pg_resetxlog
? src/bin/psql/psql
? src/bin/scripts/clusterdb
? src/bin/scripts/createdb
? src/bin/scripts/createlang
? src/bin/scripts/createuser
? src/bin/scripts/dropdb
? src/bin/scripts/droplang
? src/bin/scripts/dropuser
? src/bin/scripts/reindexdb
? src/bin/scripts/vacuumdb
? src/include/pg_config.h
? src/include/stamp-h
? src/interfaces/ecpg/compatlib/exports.list
? src/interfaces/ecpg/compatlib/libecpg_compat.so.3.1
? src/interfaces/ecpg/compatlib/libecpg_compat.so.3.2
? src/interfaces/ecpg/ecpglib/exports.list
? src/interfaces/ecpg/ecpglib/libecpg.so.6.2
? src/interfaces/ecpg/include/ecpg_config.h
? src/interfaces/ecpg/include/stamp-h
? src/interfaces/ecpg/pgtypeslib/exports.list
? src/interfaces/ecpg/pgtypeslib/libpgtypes.so.3.1
? src/interfaces/ecpg/preproc/ecpg
? src/interfaces/libpq/exports.list
? src/interfaces/libpq/libpq.so.5.3
? src/port/pg_config_paths.h
? src/test/regress/log
? src/test/regress/pg_regress
? src/test/regress/results
? src/test/regress/testtablespace
? src/test/regress/tmp_check
? src/test/regress/expected/constraints.out
? src/test/regress/expected/copy.out
? src/test/regress/expected/create_function_1.out
? src/test/regress/expected/create_function_2.out
? src/test/regress/expected/largeobject.out
? src/test/regress/expected/largeobject_1.out
? src/test/regress/expected/misc.out
? src/test/regress/expected/tablespace.out
? src/test/regress/sql/constraints.sql
? src/test/regress/sql/copy.sql
? src/test/regress/sql/create_function_1.sql
? src/test/regress/sql/create_function_2.sql
? src/test/regress/sql/largeobject.sql
? src/test/regress/sql/misc.sql
? src/test/regress/sql/tablespace.sql
? src/timezone/zic
Index: src/backend/utils/adt/geo_ops.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/utils/adt/geo_ops.c,v
retrieving revision 1.103
diff -c -r1.103 geo_ops.c
*** src/backend/utils/adt/geo_ops.c    28 Jul 2009 09:47:59 -0000    1.103
--- src/backend/utils/adt/geo_ops.c    23 Aug 2009 03:44:39 -0000
***************
*** 825,831 ****
      box_cn(&a, box1);
      box_cn(&b, box2);

!     PG_RETURN_FLOAT8(HYPOT(a.x - b.x, a.y - b.y));
  }


--- 825,831 ----
      box_cn(&a, box1);
      box_cn(&b, box2);

!     PG_RETURN_FLOAT8(hypot(a.x - b.x, a.y - b.y));
  }


***************
*** 1971,1977 ****
      Point       *pt1 = PG_GETARG_POINT_P(0);
      Point       *pt2 = PG_GETARG_POINT_P(1);

!     PG_RETURN_FLOAT8(HYPOT(pt1->x - pt2->x, pt1->y - pt2->y));
  }

  double
--- 1971,1977 ----
      Point       *pt1 = PG_GETARG_POINT_P(0);
      Point       *pt2 = PG_GETARG_POINT_P(1);

!     PG_RETURN_FLOAT8(hypot(pt1->x - pt2->x, pt1->y - pt2->y));
  }

  double
***************
*** 1979,1987 ****
  {
  #ifdef GEODEBUG
      printf("point_dt- segment (%f,%f),(%f,%f) length is %f\n",
!     pt1->x, pt1->y, pt2->x, pt2->y, HYPOT(pt1->x - pt2->x, pt1->y - pt2->y));
  #endif
!     return HYPOT(pt1->x - pt2->x, pt1->y - pt2->y);
  }

  Datum
--- 1979,1987 ----
  {
  #ifdef GEODEBUG
      printf("point_dt- segment (%f,%f),(%f,%f) length is %f\n",
!     pt1->x, pt1->y, pt2->x, pt2->y, hypot(pt1->x - pt2->x, pt1->y - pt2->y));
  #endif
!     return hypot(pt1->x - pt2->x, pt1->y - pt2->y);
  }

  Datum
***************
*** 2444,2450 ****
  dist_pl_internal(Point *pt, LINE *line)
  {
      return (line->A * pt->x + line->B * pt->y + line->C) /
!         HYPOT(line->A, line->B);
  }

  Datum
--- 2444,2450 ----
  dist_pl_internal(Point *pt, LINE *line)
  {
      return (line->A * pt->x + line->B * pt->y + line->C) /
!         hypot(line->A, line->B);
  }

  Datum
***************
*** 4916,4922 ****
                                             PointPGetDatum(point)));
      result->center.x = p->x;
      result->center.y = p->y;
!     result->radius *= HYPOT(point->x, point->y);

      PG_RETURN_CIRCLE_P(result);
  }
--- 4916,4922 ----
                                             PointPGetDatum(point)));
      result->center.x = p->x;
      result->center.y = p->y;
!     result->radius *= hypot(point->x, point->y);

      PG_RETURN_CIRCLE_P(result);
  }
***************
*** 4936,4942 ****
                                             PointPGetDatum(point)));
      result->center.x = p->x;
      result->center.y = p->y;
!     result->radius /= HYPOT(point->x, point->y);

      PG_RETURN_CIRCLE_P(result);
  }
--- 4936,4942 ----
                                             PointPGetDatum(point)));
      result->center.x = p->x;
      result->center.y = p->y;
!     result->radius /= hypot(point->x, point->y);

      PG_RETURN_CIRCLE_P(result);
  }
Index: src/include/utils/geo_decls.h
===================================================================
RCS file: /projects/cvsroot/pgsql/src/include/utils/geo_decls.h,v
retrieving revision 1.55
diff -c -r1.55 geo_decls.h
*** src/include/utils/geo_decls.h    1 Jan 2009 17:24:02 -0000    1.55
--- src/include/utils/geo_decls.h    23 Aug 2009 03:44:44 -0000
***************
*** 50,56 ****
  #define FPge(A,B)                ((A) >= (B))
  #endif

- #define HYPOT(A, B)                sqrt((A) * (A) + (B) * (B))

  /*---------------------------------------------------------------------
   * Point - (x,y)
--- 50,55 ----