Thread: 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@gmail.com>
Reviewed-by: Álvaro Herrera <alvherre@2ndquadrant.com>
Reviewed-by: Jeevan Chalke <jeevan.chalke@enterprisedb.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(-)
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@gmail.com>
> Reviewed-by: Álvaro Herrera <alvherre@2ndquadrant.com>
> Reviewed-by: Jeevan Chalke <jeevan.chalke@enterprisedb.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
Attachment
Andres Freund <andres@2ndquadrant.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@2ndquadrant.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@2ndquadrant.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@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ Everyone has their own god. +
Bruce Momjian <bruce@momjian.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