replace unaligned access with copy

spatial/include/spatial/core/geometry/geometry.hpp

@@ -104,8 +104,7 @@ class Point {
 	}
 	string ToString() const;
 	bool IsEmpty() const;
-	Vertex &GetVertex();
-	const Vertex &GetVertex() const;
+	Vertex GetVertex() const;
 
 	const VertexVector &Vertices() const {
 		return vertices;

spatial/include/spatial/core/geometry/vertex_vector.hpp

@@ -66,39 +66,18 @@ class VertexVector {
 public:
 	uint32_t count;
 	uint32_t capacity;
-	Vertex *data;
+	data_ptr_t data;
 
-	explicit VertexVector(Vertex *data, uint32_t count, uint32_t capacity)
+	explicit VertexVector(data_ptr_t data, uint32_t count, uint32_t capacity)
 	    : count(count), capacity(capacity), data(data) {
 	}
 
 	// Create a VertexVector from an already existing buffer
-	static VertexVector FromBuffer(Vertex *buffer, uint32_t count) {
+	static VertexVector FromBuffer(data_ptr_t buffer, uint32_t count) {
 		VertexVector array(buffer, count, count);
 		return array;
 	}
 
-	inline Vertex &operator[](uint32_t index) const {
-		D_ASSERT(index < count);
-		return data[index];
-	}
-
-	inline Vertex *begin() {
-		return data;
-	}
-
-	inline Vertex *end() {
-		return data + count;
-	}
-
-	const Vertex *begin() const {
-		return data;
-	}
-
-	const Vertex *end() const {
-		return data + count;
-	}
-
 	inline uint32_t Count() const {
 		return count;
 	}
@@ -109,7 +88,18 @@ class VertexVector {
 
 	inline void Add(const Vertex &v) {
 		D_ASSERT(count < capacity);
-		data[count++] = v;
+		Store<Vertex>(v, data + count * sizeof(Vertex));
+		count++;
+	}
+
+	inline void Set(uint32_t index, const Vertex &v) const {
+		D_ASSERT(index < count);
+		Store<Vertex>(v, data + index * sizeof(Vertex));
+	}
+
+	inline Vertex Get(uint32_t index) const {
+		D_ASSERT(index < count);
+		return Load<Vertex>(data + index * sizeof(Vertex));
 	}
 
 	// Returns the number of bytes that this VertexVector requires to be serialized
@@ -120,10 +110,6 @@ class VertexVector {
 	void Serialize(Cursor &cursor) const;
 	void SerializeAndUpdateBounds(Cursor &cursor, BoundingBox &bbox) const;
 
-	inline Vertex *Data() {
-		return data;
-	}
-
 	double Length() const;
 	double SignedArea() const;
 	double Area() const;

spatial/src/spatial/core/functions/cast/geometry_cast.cpp

@@ -46,7 +46,7 @@ static bool GeometryToPoint2DCast(Vector &source, Vector &result, idx_t count, C
 		if (point.IsEmpty()) {
 			throw CastException("Cannot cast empty point GEOMETRY to POINT_2D");
 		}
-		auto &vertex = point.GetVertex();
+		auto vertex = point.GetVertex();
 		return POINT_TYPE {vertex.x, vertex.y};
 	});
 	return true;
@@ -103,8 +103,8 @@ static bool GeometryToLineString2DCast(Vector &source, Vector &result, idx_t cou
 		ListVector::Reserve(result, total_coords);
 
 		for (idx_t i = 0; i < line_size; i++) {
-			x_data[entry.offset + i] = line.Vertices()[i].x;
-			y_data[entry.offset + i] = line.Vertices()[i].y;
+			x_data[entry.offset + i] = line.Vertices().Get(i).x;
+			y_data[entry.offset + i] = line.Vertices().Get(i).y;
 		}
 		return entry;
 	});
@@ -182,8 +182,8 @@ static bool GeometryToPolygon2DCast(Vector &source, Vector &result, idx_t count,
 			ring_entries[total_rings + ring_idx] = ring_entry;
 
 			for (idx_t j = 0; j < ring_size; j++) {
-				x_data[ring_entry.offset + j] = ring.data[j].x;
-				y_data[ring_entry.offset + j] = ring.data[j].y;
+				x_data[ring_entry.offset + j] = ring.Get(j).x;
+				y_data[ring_entry.offset + j] = ring.Get(j).y;
 			}
 			total_coords += ring_size;
 		}
@@ -217,16 +217,13 @@ static bool Box2DToGeometryCast(Vector &source, Vector &result, idx_t count, Cas
 
 		auto geom = lstate.factory.CreatePolygon(1, &capacity);
 		auto &shell = geom.Ring(0);
-		shell.data[0].x = minx;
-		shell.data[0].y = miny;
-		shell.data[1].x = maxx;
-		shell.data[1].y = miny;
-		shell.data[2].x = maxx;
-		shell.data[2].y = maxy;
-		shell.data[3].x = minx;
-		shell.data[3].y = maxy;
-		shell.data[4].x = minx;
-		shell.data[4].y = miny;
+
+		shell.Set(0, Vertex(minx, miny));
+		shell.Set(1, Vertex(maxx, miny));
+		shell.Set(2, Vertex(maxx, maxy));
+		shell.Set(3, Vertex(minx, maxy));
+		shell.Set(4, Vertex(minx, miny));
+
 		shell.count = 5;
 		return lstate.factory.Serialize(result, Geometry(geom));
 	});

spatial/src/spatial/core/functions/scalar/st_asgeojson.cpp

@@ -60,7 +60,7 @@ class JSONAllocator {
 static void VerticesToGeoJSON(const VertexVector &vertices, yyjson_mut_doc *doc, yyjson_mut_val *arr) {
 	// TODO: If the vertexvector is empty, do we null, add an empty array or a pair of NaN?
 	for (uint32_t i = 0; i < vertices.count; i++) {
-		auto &vertex = vertices[i];
+		auto vertex = vertices.Get(i);
 		auto coord = yyjson_mut_arr(doc);
 		yyjson_mut_arr_add_real(doc, coord, vertex.x);
 		yyjson_mut_arr_add_real(doc, coord, vertex.y);
@@ -75,7 +75,7 @@ static void ToGeoJSON(const Point &point, yyjson_mut_doc *doc, yyjson_mut_val *o
 	yyjson_mut_obj_add_val(doc, obj, "coordinates", coords);
 
 	if (!point.IsEmpty()) {
-		auto &vertex = point.GetVertex();
+		auto vertex = point.GetVertex();
 		yyjson_mut_arr_add_real(doc, coords, vertex.x);
 		yyjson_mut_arr_add_real(doc, coords, vertex.y);
 	}

spatial/src/spatial/core/functions/scalar/st_endpoint.cpp

@@ -80,7 +80,7 @@ static void GeometryEndPointFunction(DataChunk &args, ExpressionState &state, Ve
 			    return string_t();
 		    }
 
-		    auto &point = line.Vertices()[point_count - 1];
+		    auto point = line.Vertices().Get(point_count - 1);
 		    return lstate.factory.Serialize(result, Geometry(lstate.factory.CreatePoint(point.x, point.y)));
 	    });
 }

spatial/src/spatial/core/functions/scalar/st_exteriorring.cpp

@@ -114,7 +114,7 @@ static void GeometryExteriorRingFunction(DataChunk &args, ExpressionState &state
 
 		    auto line = lstate.factory.CreateLineString(num_points);
 		    for (uint32_t i = 0; i < num_points; i++) {
-			    line.Vertices().Add(shell[i]);
+			    line.Vertices().Add(shell.Get(i));
 		    }
 		    return lstate.factory.Serialize(result, Geometry(line));
 	    });

spatial/src/spatial/core/functions/scalar/st_flipcoordinates.cpp

@@ -155,7 +155,9 @@ static void BoxFlipCoordinatesFunction(DataChunk &args, ExpressionState &state,
 //------------------------------------------------------------------------------
 static void FlipVertexVector(VertexVector &vertices) {
 	for (idx_t i = 0; i < vertices.count; i++) {
-		std::swap(vertices[i].x, vertices[i].y);
+		auto vertex = vertices.Get(i);
+		std::swap(vertex.x, vertex.y);
+		vertices.Set(i, vertex);
 	}
 }
 static void FlipGeometry(Point &point) {

spatial/src/spatial/core/functions/scalar/st_makepolygon.cpp

@@ -35,7 +35,7 @@ static void MakePolygonFromRingsFunction(DataChunk &args, ExpressionState &state
 		    }
 
 		    auto &shell_verts = shell.Vertices();
-		    if (shell_verts[0] != shell_verts[shell_vert_count - 1]) {
+		    if (!shell_verts.IsClosed()) {
 			    throw InvalidInputException(
 			        "ST_MakePolygon shell must be closed (first and last vertex must be equal)");
 		    }
@@ -70,7 +70,7 @@ static void MakePolygonFromRingsFunction(DataChunk &args, ExpressionState &state
 			    }
 
 			    auto &ring_verts = hole.Vertices();
-			    if (ring_verts[0] != ring_verts[hole_count - 1]) {
+			    if (!ring_verts.IsClosed()) {
 				    throw InvalidInputException(StringUtil::Format(
 				        "ST_MakePolygon hole #%lu must be closed (first and last vertex must be equal)", hole_idx + 1));
 			    }
@@ -84,8 +84,9 @@ static void MakePolygonFromRingsFunction(DataChunk &args, ExpressionState &state
 		    for (idx_t ring_idx = 0; ring_idx < rings.size(); ring_idx++) {
 			    auto &new_ring = rings[ring_idx];
 			    auto &poly_ring = polygon.Ring(ring_idx);
-			    for (auto &v : new_ring.Vertices()) {
-				    poly_ring.Add(v);
+
+			    for(auto i = 0; i < new_ring.Vertices().Count(); i++) {
+				    poly_ring.Add(new_ring.Vertices().Get(i));
 			    }
 		    }
 
@@ -111,13 +112,13 @@ static void MakePolygonFromShellFunction(DataChunk &args, ExpressionState &state
 		}
 
 		auto &line_verts = line.Vertices();
-		if (line_verts[0] != line_verts[line_count - 1]) {
+		if (!line_verts.IsClosed()) {
 			throw InvalidInputException("ST_MakePolygon shell must be closed (first and last vertex must be equal)");
 		}
 
 		auto polygon = lstate.factory.CreatePolygon(1, &line_count);
-		for (auto &v : line_verts) {
-			polygon.Shell().Add(v);
+		for (uint32_t i = 0; i < line_count; i++) {
+			polygon.Shell().Add(line_verts.Get(i));
 		}
 
 		return lstate.factory.Serialize(result, Geometry(polygon));

spatial/src/spatial/core/functions/scalar/st_perimeter.cpp

@@ -76,8 +76,8 @@ static double PolygonPerimeter(const Polygon &poly) {
 	double perimeter = 0;
 	for (auto &ring : poly.Rings()) {
 		for (uint32_t i = 0; i < ring.Count() - 1; i++) {
-			auto &v1 = ring[i];
-			auto &v2 = ring[i + 1];
+			auto v1 = ring.Get(i);
+			auto v2 = ring.Get(i + 1);
 			perimeter += std::sqrt(std::pow(v1.x - v2.x, 2) + std::pow(v1.y - v2.y, 2));
 		}
 	}

spatial/src/spatial/core/functions/scalar/st_pointn.cpp

@@ -90,7 +90,7 @@ static void GeometryPointNFunction(DataChunk &args, ExpressionState &state, Vect
 		    }
 
 		    auto actual_index = index < 0 ? point_count + index : index - 1;
-		    auto &point = line.Vertices()[actual_index];
+		    auto point = line.Vertices().Get(actual_index);
 		    return lstate.factory.Serialize(result, Geometry(lstate.factory.CreatePoint(point.x, point.y)));
 	    });
 }

spatial/src/spatial/core/functions/scalar/st_startpoint.cpp

@@ -80,7 +80,7 @@ static void GeometryStartPointFunction(DataChunk &args, ExpressionState &state,
 			    return string_t();
 		    }
 
-		    auto &point = line.Vertices()[0];
+		    auto point = line.Vertices().Get(0);
 		    return lstate.factory.Serialize(result, Geometry(lstate.factory.CreatePoint(point.x, point.y)));
 	    });
 }

spatial/src/spatial/core/geometry/geometry.cpp

@@ -33,28 +33,22 @@ string Point::ToString() const {
 	if (IsEmpty()) {
 		return "POINT EMPTY";
 	}
-	auto &vert = vertices[0];
+	auto vert = vertices.Get(0);
 	if (std::isnan(vert.x) && std::isnan(vert.y)) {
 		// This is a special case for WKB. WKB does not support empty points,
 		// and instead writes a point with NaN coordinates. We therefore need to
 		// check for this case and return POINT EMPTY instead to round-trip safely
 		return "POINT EMPTY";
 	}
-	auto x = vertices[0].x;
-	auto y = vertices[0].y;
-	return StringUtil::Format("POINT (%s)", Utils::format_coord(x, y));
+	return StringUtil::Format("POINT (%s)", Utils::format_coord(vert.x, vert.y));
 }
 
 bool Point::IsEmpty() const {
 	return vertices.Count() == 0;
 }
 
-Vertex &Point::GetVertex() {
-	return vertices[0];
-}
-
-const Vertex &Point::GetVertex() const {
-	return vertices[0];
+Vertex Point::GetVertex() const {
+	return vertices.Get(0);
 }
 
 Point::operator Geometry() const {
@@ -85,8 +79,8 @@ string LineString::ToString() const {
 
 	string result = "LINESTRING (";
 	for (uint32_t i = 0; i < vertices.Count(); i++) {
-		auto x = vertices[i].x;
-		auto y = vertices[i].y;
+		auto x = vertices.Get(i).x;
+		auto y = vertices.Get(i).y;
 		result += Utils::format_coord(x, y);
 		if (i < vertices.Count() - 1) {
 			result += ", ";
@@ -150,8 +144,8 @@ string Polygon::ToString() const {
 	for (uint32_t i = 0; i < num_rings; i++) {
 		result += "(";
 		for (uint32_t j = 0; j < rings[i].Count(); j++) {
-			auto x = rings[i][j].x;
-			auto y = rings[i][j].y;
+			auto x = rings[i].Get(j).x;
+			auto y = rings[i].Get(j).y;
 			result += Utils::format_coord(x, y);
 			if (j < rings[i].Count() - 1) {
 				result += ", ";
@@ -187,7 +181,7 @@ string MultiPoint::ToString() const {
 		if (points[i].IsEmpty()) {
 			str += "EMPTY";
 		} else {
-			auto &vert = points[i].GetVertex();
+			auto vert = points[i].GetVertex();
 			str += Utils::format_coord(vert.x, vert.y);
 		}
 		if (i < num_points - 1) {
@@ -249,7 +243,8 @@ string MultiLineString::ToString() const {
 		}
 		str += "(";
 		bool first_vert = true;
-		for (auto &vert : line.Vertices()) {
+		for (uint32_t i = 0; i < line.Vertices().Count(); i++) {
+			auto vert = line.Vertices().Get(i);
 			if (first_vert) {
 				first_vert = false;
 			} else {
@@ -330,7 +325,8 @@ string MultiPolygon::ToString() const {
 			}
 			str += "(";
 			bool first_vert = true;
-			for (auto &vert : ring) {
+			for (uint32_t v = 0; v < ring.Count(); v++) {
+				auto vert = ring.Get(v);
 				if (first_vert) {
 					first_vert = false;
 				} else {

spatial/src/spatial/core/geometry/geometry_factory.cpp

@@ -33,7 +33,7 @@ data_ptr_t GeometryFactory::ToWKB(const Geometry &geometry, uint32_t *size) {
 }
 
 VertexVector GeometryFactory::AllocateVertexVector(uint32_t capacity) {
-	auto data = reinterpret_cast<Vertex *>(allocator.AllocateAligned(sizeof(Vertex) * capacity));
+	auto data = allocator.AllocateAligned(sizeof(Vertex) * capacity);
 	return VertexVector(data, 0, capacity);
 }
 
@@ -563,11 +563,11 @@ Point GeometryFactory::DeserializePoint(Cursor &reader) {
 	// Points can be empty too, in which case the count is 0
 	auto count = reader.Read<uint32_t>();
 	if (count == 0) {
-		VertexVector vertex_data((Vertex *)reader.GetPtr(), 0, 0);
+		VertexVector vertex_data(reader.GetPtr(), 0, 0);
 		return Point(vertex_data);
 	} else {
 		D_ASSERT(count == 1);
-		VertexVector vertex_data((Vertex *)reader.GetPtr(), 1, 1);
+		VertexVector vertex_data(reader.GetPtr(), 1, 1);
 		// Move the pointer forward (in case we are reading from a collection type)
 		reader.Skip(sizeof(Vertex));
 		return Point(vertex_data);
@@ -581,7 +581,7 @@ LineString GeometryFactory::DeserializeLineString(Cursor &reader) {
 	// 0 if the linestring is empty
 	auto count = reader.Read<uint32_t>();
 	// read data
-	VertexVector vertex_data((Vertex *)reader.GetPtr(), count, count);
+	VertexVector vertex_data(reader.GetPtr(), count, count);
 
 	reader.Skip(count * sizeof(Vertex));
 
@@ -601,7 +601,7 @@ Polygon GeometryFactory::DeserializePolygon(Cursor &reader) {
 	auto data_ptr = reader.GetPtr() + sizeof(uint32_t) * num_rings + ((num_rings % 2) * sizeof(uint32_t));
 	for (uint32_t i = 0; i < num_rings; i++) {
 		auto count = reader.Read<uint32_t>();
-		rings[i] = VertexVector((Vertex *)data_ptr, count, count);
+		rings[i] = VertexVector(data_ptr, count, count);
 		data_ptr += count * sizeof(Vertex);
 	}
 	reader.SetPtr(data_ptr);
@@ -696,7 +696,7 @@ GeometryCollection GeometryFactory::DeserializeGeometryCollection(Cursor &reader
 
 VertexVector GeometryFactory::CopyVertexVector(const VertexVector &vector) {
 	auto result = VertexVector(vector);
-	result.data = (Vertex *)allocator.AllocateAligned(vector.capacity * sizeof(Vertex));
+	result.data = allocator.AllocateAligned(vector.capacity * sizeof(Vertex));
 	memcpy(result.data, vector.data, vector.capacity * sizeof(Vertex));
 	return result;
 }