[postgis-commits] svn - r3643 - in spike/mleslie/parser: liblwgeom regress

postgis-commits at postgis.refractions.net postgis-commits at postgis.refractions.net
Wed Feb 4 21:06:24 PST 2009


Author: mleslie
Date: 2009-02-04 21:06:23 -0800 (Wed, 04 Feb 2009)
New Revision: 3643

Modified:
   spike/mleslie/parser/liblwgeom/lwgparse.c
   spike/mleslie/parser/regress/regress_expected
   spike/mleslie/parser/regress/regress_ogc_expected
Log:
Implemented MCA's suggested change to track the parser location for each tuple so that the location of structural errors can be accurately reported to the user.

Modified: spike/mleslie/parser/liblwgeom/lwgparse.c
===================================================================
--- spike/mleslie/parser/liblwgeom/lwgparse.c	2009-02-05 02:46:56 UTC (rev 3642)
+++ spike/mleslie/parser/liblwgeom/lwgparse.c	2009-02-05 05:06:23 UTC (rev 3643)
@@ -59,6 +59,7 @@
 			int	type;
 			int	num;
 			int	size_here;
+			int parse_location;
 		} nn;
 
 	} uu;
@@ -134,6 +135,17 @@
 };
 
 /* Macro to return the error message and the current position within WKT */ 
+#define LWGEOM_WKT_VALIDATION_ERROR(errcode, parse_location) \
+	do { \
+		if (!parser_ferror_occured) { \
+			parser_ferror_occured = -1 * errcode; \
+			current_lwg_parser_result->message = parser_error_messages[errcode]; \
+			current_lwg_parser_result->errlocation = parse_location; \
+		} \
+	} while (0);
+
+
+/* Macro to return the error message and the current position within WKT */ 
 #define LWGEOM_WKT_PARSER_ERROR(errcode) \
 	do { \
 		if (!parser_ferror_occured) { \
@@ -245,6 +257,11 @@
 	srid=(int)(d_srid+0.1);
 }
 
+/*
+ * Begin alloc / free functions
+ */
+
+
 tuple *
 alloc_tuple(output_func of,size_t size)
 {
@@ -278,6 +295,10 @@
 		the_geom.first = the_geom.last = ret;
 	}
 
+	LWDEBUGF(5, "alloc_tuple %p: parse_location = %d", 
+			ret, lwg_parse_yylloc.last_column);
+ 	ret->uu.nn.parse_location = lwg_parse_yylloc.last_column;
+
 	the_geom.alloc_size += size;
 	return ret;
 }
@@ -300,6 +321,81 @@
 }
 
 void
+alloc_lwgeom(int srid)
+{
+		LWDEBUGF(3, "alloc_lwgeom %d", srid);
+
+	the_geom.srid=srid;
+	the_geom.alloc_size=0;
+	the_geom.stack=NULL;
+	the_geom.ndims=0;
+	the_geom.hasZ=0;
+	the_geom.hasM=0;
+
+	/* Free if used already */
+	if ( the_geom.first ){
+		free_tuple(the_geom.first);
+		the_geom.first=the_geom.last=NULL;
+	}
+
+	if ( srid != -1 ){
+		the_geom.alloc_size+=4;
+	}
+
+	/* Setup up an empty tuple as the stack base */
+	the_geom.stack = alloc_tuple(empty_stack, 0);
+}
+
+void
+alloc_point_2d(double x,double y)
+{
+	tuple* p = alloc_tuple(write_point_2,the_geom.lwgi?8:16);
+	p->uu.points[0] = x;
+	p->uu.points[1] = y;
+
+	LWDEBUGF(3, "alloc_point_2d %f,%f", x, y);
+	LWDEBUGF(5, "  * %p", p);
+	
+	/* keep track of point */
+
+	inc_num();
+	check_dims(2);
+}
+
+void
+alloc_point_3d(double x,double y,double z)
+{
+	tuple* p = alloc_tuple(write_point_3,the_geom.lwgi?12:24);
+	p->uu.points[0] = x;
+	p->uu.points[1] = y;
+	p->uu.points[2] = z;
+
+		LWDEBUGF(3, "alloc_point_3d %f, %f, %f", x, y, z);
+	LWDEBUGF(5, "  * %p", p);
+
+	inc_num();
+	check_dims(3);
+}
+
+void
+alloc_point_4d(double x,double y,double z,double m)
+{
+	tuple* p = alloc_tuple(write_point_4,the_geom.lwgi?16:32);
+	p->uu.points[0] = x;
+	p->uu.points[1] = y;
+	p->uu.points[2] = z;
+	p->uu.points[3] = m;
+
+		LWDEBUGF(3, "alloc_point_4d %f, %f, %f, %f", x, y, z, m);
+		LWDEBUGF(5, "  * %p", p);
+
+	inc_num();
+	check_dims(4);
+}
+
+
+
+void
 inc_num(void)
 {
 	the_geom.stack->uu.nn.num++;
@@ -314,13 +410,14 @@
 	tuple*	p;
 	inc_num();
 
-		LWDEBUGF(3, "alloc_stack_tuple %d, %d", type, size);
+		LWDEBUGF(3, "alloc_stack_tuple: type = %d, size = %d", type, size);
 	
 	p = alloc_tuple(of,size);
 	p->uu.nn.stack_next = the_geom.stack;
 	p->uu.nn.type = type;
 	p->uu.nn.size_here = the_geom.alloc_size;
 	p->uu.nn.num = 0;
+
 	the_geom.stack = p;
 
 	LWDEBUGF(4, "alloc_stack_tuple complete: %p", the_geom.stack);
@@ -334,6 +431,10 @@
 	the_geom.stack = the_geom.stack->uu.nn.stack_next;
 }
 
+/*
+ * Begin Check functions
+ */
+
 void check_compoundcurve(void)
 {
 	check_geometry_minpoints();
@@ -407,24 +508,24 @@
 			if(first->uu.points[0] != last->uu.points[0])
 			{
 				LWDEBUG(5, "x value mismatch");
-				LWGEOM_WKT_PARSER_ERROR(PARSER_ERROR_INCONTINUOUS);
+				LWGEOM_WKT_VALIDATION_ERROR(PARSER_ERROR_INCONTINUOUS,last->uu.nn.parse_location);
 			}
 			else if(first->uu.points[1] != last->uu.points[1])
 			{
 				LWDEBUG(5, "y value mismatch");
-				LWGEOM_WKT_PARSER_ERROR(PARSER_ERROR_INCONTINUOUS);
+				LWGEOM_WKT_VALIDATION_ERROR(PARSER_ERROR_INCONTINUOUS,last->uu.nn.parse_location);
 			}
 			else if(the_geom.ndims > 2 &&
 					first->uu.points[2] != last->uu.points[2])
 			{
 				LWDEBUG(5, "z/m value mismatch");
-				LWGEOM_WKT_PARSER_ERROR(PARSER_ERROR_INCONTINUOUS);
+				LWGEOM_WKT_VALIDATION_ERROR(PARSER_ERROR_INCONTINUOUS,last->uu.nn.parse_location);
 			}
 			else if(the_geom.ndims > 3 &&
 					first->uu.points[3] != last->uu.points[3])
 			{
 				LWDEBUG(5, "m value mismatch");
-				LWGEOM_WKT_PARSER_ERROR(PARSER_ERROR_INCONTINUOUS);
+				LWGEOM_WKT_VALIDATION_ERROR(PARSER_ERROR_INCONTINUOUS,last->uu.nn.parse_location);
 			}
 		}
 		for(j = 0; j < mum; j++)
@@ -439,13 +540,18 @@
 void check_circularstring_isodd(void)
 {
 	tuple *tp = the_geom.stack->next;
+	int i, num;
 	
 	LWDEBUG(3, "check_circularstring_isodd");
 	if(tp->uu.nn.num % 2 == 0)
 	{
-		LWDEBUGF(5, "Odd check failed: pointcount = %d", 
-				the_geom.stack->uu.nn.num);
-	   	LWGEOM_WKT_PARSER_ERROR(PARSER_ERROR_ODDPOINTS);
+		num = tp->uu.nn.num;
+		LWDEBUGF(5, "Odd check failed: pointcount = %d", num);
+		for(i = 0; i < num; i++)
+		{
+			tp = tp->next;
+		}
+	   	LWGEOM_WKT_VALIDATION_ERROR(PARSER_ERROR_ODDPOINTS, tp->uu.nn.parse_location);
 	}
 }
 
@@ -484,7 +590,7 @@
 					first->uu.points[0], first->uu.points[1],
 					last->uu.points[0], last->uu.points[1]);
 			LWDEBUGF(5, "First %p, last %p", first, last);
-			LWGEOM_WKT_PARSER_ERROR(PARSER_ERROR_UNCLOSED);
+			LWGEOM_WKT_VALIDATION_ERROR(PARSER_ERROR_UNCLOSED, last->uu.nn.parse_location);
 		}
 		else 
 		{
@@ -536,7 +642,7 @@
 				first->uu.points[0], first->uu.points[1],
 				last->uu.points[0], last->uu.points[1]);
 		LWDEBUGF(5, "First %p, last %p", first, last);
-		LWGEOM_WKT_PARSER_ERROR(PARSER_ERROR_UNCLOSED);
+		LWGEOM_WKT_VALIDATION_ERROR(PARSER_ERROR_UNCLOSED, last->uu.nn.parse_location);
 	}
 	else
 	{
@@ -576,7 +682,7 @@
 					first->uu.points[0], first->uu.points[1], 
 					last->uu.points[0], last->uu.points[1]);
 			LWDEBUGF(5, "First %p, last %p", first, last);
-			LWGEOM_WKT_PARSER_ERROR(PARSER_ERROR_UNCLOSED);
+			LWGEOM_WKT_VALIDATION_ERROR(PARSER_ERROR_UNCLOSED, last->uu.nn.parse_location);
 		}
 		else
 		{
@@ -634,18 +740,21 @@
 		tp = tp->next;
 		mum = tp->uu.nn.num;
 
+		/* Skip the point tuples */
+		for(j = 0; j < mum; j++)
+		{
+			tp = tp->next;
+		}
+
 		if(mum < minpoints)
 		{
 			LWDEBUGF(5, "Minpoint check failed: needed %d, got %d",
 					minpoints, mum);
-			LWGEOM_WKT_PARSER_ERROR(PARSER_ERROR_MOREPOINTS);
+			LWDEBUGF(5, "tuple = %p; parse_location = %d; parser reported column = %d",
+					tp, tp->uu.nn.parse_location, lwg_parse_yylloc.last_column);
+			LWGEOM_WKT_VALIDATION_ERROR(PARSER_ERROR_MOREPOINTS, tp->uu.nn.parse_location);
 		}
 
-		/* Skip the point tuples */
-		for(j = 0; j < mum; j++)
-		{
-			tp = tp->next;
-		}
 	}
 }
 
@@ -691,7 +800,7 @@
 				{
 					LWDEBUGF(5, "Minpoint check failed: needed %d, got %d",
 							minpoints, count);
-					LWGEOM_WKT_PARSER_ERROR(PARSER_ERROR_MOREPOINTS);
+					LWGEOM_WKT_VALIDATION_ERROR(PARSER_ERROR_MOREPOINTS, tp->uu.nn.parse_location);
 				}
 				break;
 			case LINETYPE:
@@ -706,7 +815,7 @@
 				{
 					LWDEBUGF(5, "Minpoint check failed: needed %d, got %d",
 							minpoints, count);
-					LWGEOM_WKT_PARSER_ERROR(PARSER_ERROR_MOREPOINTS);
+					LWGEOM_WKT_VALIDATION_ERROR(PARSER_ERROR_MOREPOINTS, tp->uu.nn.parse_location);
 				}
 				break;
 		}
@@ -756,7 +865,7 @@
 	{
 		LWDEBUGF(5, "Minpoint check failed: needed %d, got %d",
 				minpoints, count);
-		LWGEOM_WKT_PARSER_ERROR(PARSER_ERROR_MOREPOINTS);
+		LWGEOM_WKT_VALIDATION_ERROR(PARSER_ERROR_MOREPOINTS, tp->uu.nn.parse_location);
 	}
 	LWDEBUG(4, "check_compoundcurve_minpoints complete");
 }
@@ -765,13 +874,19 @@
 check_linestring_minpoints(int minpoints)
 {
 	tuple *tp = the_geom.stack->next; /* Current counting tuple */
+	int i, num;
 
 	LWDEBUG(3, "check_linestring_minpoints");
 	if(tp->uu.nn.num < minpoints)
 	{
+		num = tp->uu.nn.num;
+		for(i = 0; i < num; i++)
+		{
+			tp = tp->next;
+		}
 		LWDEBUGF(5, "Minpoint check failed: needed %d, got %d",
 				minpoints, tp->uu.nn.num);
-		LWGEOM_WKT_PARSER_ERROR(PARSER_ERROR_MOREPOINTS);
+		LWGEOM_WKT_VALIDATION_ERROR(PARSER_ERROR_MOREPOINTS, tp->uu.nn.parse_location);
 	}
 }
 
@@ -891,34 +1006,7 @@
 {
 	/* Do nothing but provide an empty base for the geometry stack */
 }
-
 void
-alloc_lwgeom(int srid)
-{
-		LWDEBUGF(3, "alloc_lwgeom %d", srid);
-
-	the_geom.srid=srid;
-	the_geom.alloc_size=0;
-	the_geom.stack=NULL;
-	the_geom.ndims=0;
-	the_geom.hasZ=0;
-	the_geom.hasM=0;
-
-	/* Free if used already */
-	if ( the_geom.first ){
-		free_tuple(the_geom.first);
-		the_geom.first=the_geom.last=NULL;
-	}
-
-	if ( srid != -1 ){
-		the_geom.alloc_size+=4;
-	}
-
-	/* Setup up an empty tuple as the stack base */
-	the_geom.stack = alloc_tuple(empty_stack, 0);
-}
-
-void
 write_point_2(tuple* this,output_state* out)
 {
 	WRITE_DOUBLES(out,this->uu.points,2);
@@ -953,55 +1041,7 @@
 {
 	WRITE_INT4_REAL_MULTIPLE(out,this->uu.points,4);
 }
-
 void
-alloc_point_2d(double x,double y)
-{
-	tuple* p = alloc_tuple(write_point_2,the_geom.lwgi?8:16);
-	p->uu.points[0] = x;
-	p->uu.points[1] = y;
-
-	LWDEBUGF(3, "alloc_point_2d %f,%f", x, y);
-	LWDEBUGF(5, "  * %p", p);
-	
-	/* keep track of point */
-
-	inc_num();
-	check_dims(2);
-}
-
-void
-alloc_point_3d(double x,double y,double z)
-{
-	tuple* p = alloc_tuple(write_point_3,the_geom.lwgi?12:24);
-	p->uu.points[0] = x;
-	p->uu.points[1] = y;
-	p->uu.points[2] = z;
-
-		LWDEBUGF(3, "alloc_point_3d %f, %f, %f", x, y, z);
-	LWDEBUGF(5, "  * %p", p);
-
-	inc_num();
-	check_dims(3);
-}
-
-void
-alloc_point_4d(double x,double y,double z,double m)
-{
-	tuple* p = alloc_tuple(write_point_4,the_geom.lwgi?16:32);
-	p->uu.points[0] = x;
-	p->uu.points[1] = y;
-	p->uu.points[2] = z;
-	p->uu.points[3] = m;
-
-		LWDEBUGF(3, "alloc_point_4d %f, %f, %f, %f", x, y, z, m);
-		LWDEBUGF(5, "  * %p", p);
-
-	inc_num();
-	check_dims(4);
-}
-
-void
 write_type(tuple* this,output_state* out)
 {
 	uchar type=0;

Modified: spike/mleslie/parser/regress/regress_expected
===================================================================
--- spike/mleslie/parser/regress/regress_expected	2009-02-05 02:46:56 UTC (rev 3642)
+++ spike/mleslie/parser/regress/regress_expected	2009-02-05 05:06:23 UTC (rev 3643)
@@ -60,9 +60,9 @@
 ERROR:  parse error - invalid geometry
 HINT:  "MULTIPOINT()" <-- parse error at position 12 within geometry
 ERROR:  geometry contains non-closed rings
-HINT:  "POLYGON( (0 0, 10 0, 10 10, 0 10) )" <-- parse error at position 35 within geometry
+HINT:  "POLYGON( (0 0, 10 0, 10 10, 0 10)" <-- parse error at position 33 within geometry
 ERROR:  geometry contains non-closed rings
-HINT:  "...0, 0 10, 0 0),(5 5, 7 5, 7 7 , 5 7) )" <-- parse error at position 62 within geometry
+HINT:  "... 10, 0 10, 0 0),(5 5, 7 5, 7 7 , 5 7)" <-- parse error at position 60 within geometry
 ERROR:  parse error - invalid geometry
 HINT:  "...INESTRING((0 0, 1 1),(0 0, 1 1, 2 2,)" <-- parse error at position 43 within geometry
 ERROR:  parse error - invalid geometry

Modified: spike/mleslie/parser/regress/regress_ogc_expected
===================================================================
--- spike/mleslie/parser/regress/regress_ogc_expected	2009-02-05 02:46:56 UTC (rev 3642)
+++ spike/mleslie/parser/regress/regress_ogc_expected	2009-02-05 05:06:23 UTC (rev 3643)
@@ -73,7 +73,7 @@
 linemerge149|LINESTRING(-5 -5,0 0,1 1,4 4)
 intersects|f
 ERROR:  geometry requires more points
-HINT:  "POLYGON((0 0, 1 1, 0 0))" <-- parse error at position 24 within geometry
+HINT:  "POLYGON((0 0, 1 1, 0 0)" <-- parse error at position 23 within geometry
 buffer|POLYGON((1 0,0.707107 -0.707107,0 -1,-0.707107 -0.707107,-1 0,-0.707107 0.707107,0 1,0.707107 0.707107,1 0))
 geomunion|MULTIPOINT(0 0,1 1)
 unite_garray|t



More information about the postgis-commits mailing list