pgsql: Revive line type

Revive line type

Change the input/output format to {A,B,C}, to match the internal
representation.

Complete the implementations of line_in, line_out, line_recv, line_send.
Remove comments and error messages about the line type not being
implemented. Add regression tests for existing line operators and
functions.

Reviewed-by: rui hua <365507506hua(at)gmail(dot)com>
Reviewed-by: Álvaro Herrera <alvherre(at)2ndquadrant(dot)com>
Reviewed-by: Jeevan Chalke <jeevan(dot)chalke(at)enterprisedb(dot)com>

Branch
------
master

Details
-------
http://git.postgresql.org/pg/commitdiff/261c7d4b653bc3e44c31fd456d94f292caa50d8f

Modified Files
--------------
doc/src/sgml/datatype.sgml | 42 ++++-
doc/src/sgml/func.sgml | 6 +
src/backend/utils/adt/geo_ops.c | 224 ++++++++++-------------
src/include/catalog/pg_type.h | 3 +-
src/include/utils/geo_decls.h | 7 -
src/test/regress/expected/geometry.out | 3 -
src/test/regress/expected/line.out | 271 ++++++++++++++++++++++++++++
src/test/regress/expected/sanity_check.out | 3 +-
src/test/regress/output/misc.source | 3 +-
src/test/regress/parallel_schedule | 2 +-
src/test/regress/serial_schedule | 1 +
src/test/regress/sql/geometry.sql | 4 -
src/test/regress/sql/line.sql | 87 +++++++++
13 files changed, 503 insertions(+), 153 deletions(-)

On Wed, Oct 9, 2013 at 10:43 PM, Peter Eisentraut <peter_e(at)gmx(dot)net> wrote:
> Revive line type

Kevin just pointed out to me that there are a bunch of buildfarm
failures. I'm looking at the ones that are attributable to shared
memory, but there seem to be some problems with this patch as well.
Check out brolga, for example.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Thu, 2013-10-10 at 10:51 -0400, Robert Haas wrote:
> On Wed, Oct 9, 2013 at 10:43 PM, Peter Eisentraut <peter_e(at)gmx(dot)net> wrote:
> > Revive line type
>
> Kevin just pointed out to me that there are a bunch of buildfarm
> failures. I'm looking at the ones that are attributable to shared
> memory, but there seem to be some problems with this patch as well.
> Check out brolga, for example.

OK, I fixed the obvious issue with the geometry test. The line test is
now failing because of "negative zero", which could be addressed with an
alternative expected file like in the case of geometry, and also because
of a rounding issue. I'm not sure yet whether the latter is just
another platform difference, an unfortunately example case, or an actual
bug.

Hi Peter,

On 2013-10-10 02:43:20 +0000, Peter Eisentraut wrote:
> Revive line type
>
> Change the input/output format to {A,B,C}, to match the internal
> representation.
>
> Complete the implementations of line_in, line_out, line_recv, line_send.
> Remove comments and error messages about the line type not being
> implemented. Add regression tests for existing line operators and
> functions.
>
> Reviewed-by: rui hua <365507506hua(at)gmail(dot)com>
> Reviewed-by: Álvaro Herrera <alvherre(at)2ndquadrant(dot)com>
> Reviewed-by: Jeevan Chalke <jeevan(dot)chalke(at)enterprisedb(dot)com>

That commit missed to update pg_type.h to the changed length of the line
type. Patch attached.

That oversight leads to accesses beyond the length of the tuple in
routines like datumCopy().

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

Andres Freund <andres(at)2ndquadrant(dot)com> writes:
> On 2013-10-10 02:43:20 +0000, Peter Eisentraut wrote:
>> Revive line type

> That commit missed to update pg_type.h to the changed length of the line
> type. Patch attached.

Ouch. Good thing we caught this now.

In principle, fixing this breaks pg_upgrade'ability for anyone who was
using the line type before (presumably by compiling with
-DENABLE_LINE_TYPE). However, we're breaking things for them anyway by
changing the text representation, so that's probably not worth worrying
over.

regards, tom lane

On 2014-05-05 13:24:15 -0400, Tom Lane wrote:
> Andres Freund <andres(at)2ndquadrant(dot)com> writes:
> > On 2013-10-10 02:43:20 +0000, Peter Eisentraut wrote:
> >> Revive line type
>
> > That commit missed to update pg_type.h to the changed length of the line
> > type. Patch attached.
>
> Ouch. Good thing we caught this now.

Yea. The only thing we could have done later is to allocate more memory
and waste the space ondisk :(.

I am really hoping to get pg into a state where it's sufficiently
valgrind clean that we can run a buildfarm animal under valgrind. The
newest release of valgrind helps there - e.g. some float bugs have been
worked out.

> In principle, fixing this breaks pg_upgrade'ability for anyone who was
> using the line type before (presumably by compiling with
> -DENABLE_LINE_TYPE). However, we're breaking things for them anyway by
> changing the text representation, so that's probably not worth worrying
> over.

Agreed

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

On Mon, May 5, 2014 at 01:24:15PM -0400, Tom Lane wrote:
> Andres Freund <andres(at)2ndquadrant(dot)com> writes:
> > On 2013-10-10 02:43:20 +0000, Peter Eisentraut wrote:
> >> Revive line type
>
> > That commit missed to update pg_type.h to the changed length of the line
> > type. Patch attached.
>
> Ouch. Good thing we caught this now.
>
> In principle, fixing this breaks pg_upgrade'ability for anyone who was
> using the line type before (presumably by compiling with
> -DENABLE_LINE_TYPE). However, we're breaking things for them anyway by
> changing the text representation, so that's probably not worth worrying
> over.

Should this be listed in the 9.4 release notes? We didn't have _any_
line type before unless you -D/enabled it? Our release notes say:

Fully-implement the <link
linkend="datatype-line"><type>line</></link> data type (Peter
Eisentraut)

Is this text misleading?

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ Everyone has their own god. +

Bruce Momjian <bruce(at)momjian(dot)us> writes:
> Should this be listed in the 9.4 release notes? We didn't have _any_
> line type before unless you -D/enabled it? Our release notes say:

> Fully-implement the <link
> linkend="datatype-line"><type>line</></link> data type (Peter
> Eisentraut)

> Is this text misleading?

The type existed in the catalogs, but if you tried to do anything with it
you got

regression=# select 'foo'::line;
ERROR: type "line" not yet implemented
LINE 1: select 'foo'::line;
^

... unless, that is, you'd built with -DENABLE_LINE_TYPE. Which is
an option I don't think was ever documented. So I'm not sure whether
we need to cover this in the release notes or not. It seems fairly
unlikely that anybody was doing that, so it's *probably* a waste of
release note space, but ...

regards, tom lane