From ca2d45d4ee96684d3b55c967d7a944151caa642e Mon Sep 17 00:00:00 2001 From: Brad Hards Date: Sun, 31 Aug 2025 13:18:35 +1000 Subject: [PATCH 1/8] fix typo in variable name No functional changes. --- duckdb | 2 +- src/spatial/modules/main/spatial_functions_scalar.cpp | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/duckdb b/duckdb index 2ed9bf88..ff0f9595 160000 --- a/duckdb +++ b/duckdb @@ -1 +1 @@ -Subproject commit 2ed9bf887f61a0ac226ab8c8f1164601d985d607 +Subproject commit ff0f95954bdb4c7515481ac2b473261407bad18b diff --git a/src/spatial/modules/main/spatial_functions_scalar.cpp b/src/spatial/modules/main/spatial_functions_scalar.cpp index 6ca172a4..aa076f8f 100644 --- a/src/spatial/modules/main/spatial_functions_scalar.cpp +++ b/src/spatial/modules/main/spatial_functions_scalar.cpp @@ -5251,7 +5251,7 @@ struct ST_LineInterpolatePoint { auto &lstate = LocalState::ResetAndGet(state); BinaryExecutor::Execute( - args.data[0], args.data[1], result, args.size(), [&](const string_t &blob, const double faction) { + args.data[0], args.data[1], result, args.size(), [&](const string_t &blob, const double fraction) { sgl::geometry geom; lstate.Deserialize(blob, geom); @@ -5260,7 +5260,7 @@ struct ST_LineInterpolatePoint { } sgl::vertex_xyzm out_vertex = {0, 0, 0, 0}; - if (sgl::linestring::interpolate(geom, faction, out_vertex)) { + if (sgl::linestring::interpolate(geom, fraction, out_vertex)) { sgl::geometry point(sgl::geometry_type::POINT, geom.has_z(), geom.has_m()); point.set_vertex_array(&out_vertex, 1); return lstate.Serialize(result, point); From 43b38f27db7ee93be3f1d89cb0ae842c1d5e1a2b Mon Sep 17 00:00:00 2001 From: Gabor Szarnyas Date: Thu, 25 Sep 2025 22:05:42 +0200 Subject: [PATCH 2/8] Fix typo --- src/spatial/modules/main/spatial_functions_scalar.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/spatial/modules/main/spatial_functions_scalar.cpp b/src/spatial/modules/main/spatial_functions_scalar.cpp index 456f2a40..d1eef5dc 100644 --- a/src/spatial/modules/main/spatial_functions_scalar.cpp +++ b/src/spatial/modules/main/spatial_functions_scalar.cpp @@ -6092,7 +6092,7 @@ struct ST_Hilbert { static constexpr auto DESCRIPTION = R"( Encodes the X and Y values as the hilbert curve index for a curve covering the given bounding box. If a geometry is provided, the center of the approximate bounding box is used as the point to encode. - If no bounding box is provided, the hilbert curve index is mapped to the full range of a single-presicion float. + If no bounding box is provided, the hilbert curve index is mapped to the full range of a single-precision float. For the BOX_2D and BOX_2DF variants, the center of the box is used as the point to encode. )"; From 3456f189ca89a3f2d947a51d131ea3ee93c3439b Mon Sep 17 00:00:00 2001 From: Ilya Boyandin Date: Tue, 28 Oct 2025 23:09:59 +0100 Subject: [PATCH 3/8] fix: Linestring corrupt PBF issue --- src/spatial/modules/mvt/mvt_module.cpp | 18 +-- test/sql/mvt/st_asmvt_linestring.test | 164 +++++++++++++++++++++++++ 2 files changed, 173 insertions(+), 9 deletions(-) create mode 100644 test/sql/mvt/st_asmvt_linestring.test diff --git a/src/spatial/modules/mvt/mvt_module.cpp b/src/spatial/modules/mvt/mvt_module.cpp index fc355dd1..48907eaa 100644 --- a/src/spatial/modules/mvt/mvt_module.cpp +++ b/src/spatial/modules/mvt/mvt_module.cpp @@ -609,15 +609,15 @@ class MVTFeatureBuilder { const auto y = CastDouble(cursor.Read()); cursor.Skip(vertex_space); // Skip z and m if present - if (vertex_idx == 0) { - geometry.push_back((1 & 0x7) | (1 << 3)); // MoveTo, 1 part - geometry.push_back(protozero::encode_zigzag32(x - cursor_x)); - geometry.push_back(protozero::encode_zigzag32(y - cursor_y)); - geometry.push_back((2 & 0x7) | ((vertex_count - 2) << 3)); // LineTo, part count - } else { - geometry.push_back(protozero::encode_zigzag32(x - cursor_x)); - geometry.push_back(protozero::encode_zigzag32(y - cursor_y)); - } + if (vertex_idx == 0) { + geometry.push_back((1 & 0x7) | (1 << 3)); // MoveTo, 1 part + geometry.push_back(protozero::encode_zigzag32(x - cursor_x)); + geometry.push_back(protozero::encode_zigzag32(y - cursor_y)); + geometry.push_back((2 & 0x7) | ((vertex_count - 1) << 3)); // LineTo, part count + } else { + geometry.push_back(protozero::encode_zigzag32(x - cursor_x)); + geometry.push_back(protozero::encode_zigzag32(y - cursor_y)); + } cursor_x = x; cursor_y = y; diff --git a/test/sql/mvt/st_asmvt_linestring.test b/test/sql/mvt/st_asmvt_linestring.test new file mode 100644 index 00000000..5f1dd9ae --- /dev/null +++ b/test/sql/mvt/st_asmvt_linestring.test @@ -0,0 +1,164 @@ +# name: test/sql/mvt/st_asmvt_linestring.test +# group: [mvt] + +require spatial + +# Test LINESTRING encoding +statement ok +COPY ( + SELECT st_asmvt( + {"geom": geom}, + 'lines' + ) as mvt + FROM ( + SELECT + st_geomfromtext('LINESTRING(0 0, 100 100, 200 0)') as geom + ) +) TO '__TEST_DIR__/test_linestring.mvt' (FORMAT BLOB); + +query I +select count(*) from st_read('__TEST_DIR__/test_linestring.mvt'); +---- +1 + +# Test MULTI_LINESTRING encoding +statement ok +COPY ( + SELECT st_asmvt( + {"geom": geom}, + 'multilines' + ) as mvt + FROM ( + SELECT + st_geomfromtext('MULTILINESTRING((0 0, 100 100, 200 0), (300 0, 400 100, 500 0))') as geom + ) +) TO '__TEST_DIR__/test_multilinestring.mvt' (FORMAT BLOB); + +query I +select count(*) from st_read('__TEST_DIR__/test_multilinestring.mvt'); +---- +1 + +# Test LINESTRING with ST_AsMVTGeom (clipping can produce MULTI_LINESTRING) +statement ok +COPY ( + SELECT st_asmvt( + {"geom": ST_AsMVTGeom( + geom, + ST_MakeEnvelope(0, 0, 1000, 1000), + 4096, + 256, + true + )}, + 'clipped_lines' + ) as mvt + FROM ( + SELECT + st_geomfromtext('LINESTRING(100 100, 500 500, 900 100)') as geom + ) +) TO '__TEST_DIR__/test_clipped_linestring.mvt' (FORMAT BLOB); + +query I +select count(*) from st_read('__TEST_DIR__/test_clipped_linestring.mvt'); +---- +1 + +# Test LINESTRING crossing tile boundary (produces MULTI_LINESTRING after clipping) +statement ok +COPY ( + SELECT st_asmvt( + {"geom": ST_AsMVTGeom( + geom, + ST_MakeEnvelope(0, 0, 1000, 1000), + 4096, + 256, + true + )}, + 'crossing_lines' + ) as mvt + FROM ( + SELECT + st_geomfromtext('LINESTRING(-500 500, 500 500, 1500 500)') as geom + ) +) TO '__TEST_DIR__/test_crossing_linestring.mvt' (FORMAT BLOB); + +query I +select count(*) from st_read('__TEST_DIR__/test_crossing_linestring.mvt'); +---- +1 + +# Test multiple LINESTRINGs with various lengths +statement ok +COPY ( + SELECT st_asmvt( + {"geom": geom, "id": id}, + 'various_lines', + 4096, + 'geom', + 'id' + ) as mvt + FROM ( + SELECT + row_number() over () as id, + st_geomfromtext('LINESTRING(' || (x*100) || ' ' || (y*100) || ', ' || (x*100+50) || ' ' || (y*100+50) || ', ' || (x*100+100) || ' ' || (y*100) || ')') as geom + FROM range(0, 10) as r(x), + range(0, 10) as rr(y) + ) +) TO '__TEST_DIR__/test_various_linestrings.mvt' (FORMAT BLOB); + +query I +select count(*) from st_read('__TEST_DIR__/test_various_linestrings.mvt'); +---- +100 + +# Test global scale dataset scenario (like Natural Earth roads) +# This simulates the case where geometries at low zoom levels span large areas +statement ok +COPY ( + SELECT st_asmvt( + {"geom": ST_AsMVTGeom( + geom, + ST_TileEnvelope(2, 1, 1), + 4096, + 256, + false + )}, + 'global_lines' + ) as mvt + FROM ( + SELECT + st_geomfromtext('LINESTRING(-10000000 5000000, 0 0, 10000000 -5000000)') as geom + ) +) TO '__TEST_DIR__/test_global_linestring.mvt' (FORMAT BLOB); + +query I +select count(*) from st_read('__TEST_DIR__/test_global_linestring.mvt'); +---- +1 + +# Test that clipped MULTI_LINESTRING can be read back +statement ok +COPY ( + SELECT st_asmvt( + {"geom": ST_AsMVTGeom( + geom, + ST_TileEnvelope(5, 10, 12), + 4096, + 256, + true + ), "name": name}, + 'roads' + ) as mvt + FROM ( + VALUES + (st_geomfromtext('MULTILINESTRING((100 100, 500 500), (600 600, 900 900))'), 'road1'), + (st_geomfromtext('LINESTRING(200 200, 800 800)'), 'road2') + ) t(geom, name) + WHERE ST_Intersects(geom, ST_TileEnvelope(5, 10, 12)) +) TO '__TEST_DIR__/test_roads.mvt' (FORMAT BLOB); + +query II +select count(*), count(name) from st_read('__TEST_DIR__/test_roads.mvt'); +---- +2 2 + From b56d9ce0dea54f4af7f937c883bb1bc3afc77ce6 Mon Sep 17 00:00:00 2001 From: Ilya Boyandin Date: Wed, 29 Oct 2025 19:16:47 +0100 Subject: [PATCH 4/8] fix tests --- test/sql/mvt/st_asmvt_linestring.test | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/test/sql/mvt/st_asmvt_linestring.test b/test/sql/mvt/st_asmvt_linestring.test index 5f1dd9ae..bb9676c0 100644 --- a/test/sql/mvt/st_asmvt_linestring.test +++ b/test/sql/mvt/st_asmvt_linestring.test @@ -45,7 +45,7 @@ COPY ( SELECT st_asmvt( {"geom": ST_AsMVTGeom( geom, - ST_MakeEnvelope(0, 0, 1000, 1000), + ST_Extent(ST_MakeEnvelope(0, 0, 1000, 1000)), 4096, 256, true @@ -69,7 +69,7 @@ COPY ( SELECT st_asmvt( {"geom": ST_AsMVTGeom( geom, - ST_MakeEnvelope(0, 0, 1000, 1000), + ST_Extent(ST_MakeEnvelope(0, 0, 1000, 1000)), 4096, 256, true @@ -118,7 +118,7 @@ COPY ( SELECT st_asmvt( {"geom": ST_AsMVTGeom( geom, - ST_TileEnvelope(2, 1, 1), + ST_Extent(ST_TileEnvelope(2, 1, 1)), 4096, 256, false @@ -136,17 +136,11 @@ select count(*) from st_read('__TEST_DIR__/test_global_linestring.mvt'); ---- 1 -# Test that clipped MULTI_LINESTRING can be read back +# Test that LINESTRING with attributes can be read back statement ok COPY ( SELECT st_asmvt( - {"geom": ST_AsMVTGeom( - geom, - ST_TileEnvelope(5, 10, 12), - 4096, - 256, - true - ), "name": name}, + {"geom": geom, "name": name}, 'roads' ) as mvt FROM ( @@ -154,7 +148,6 @@ COPY ( (st_geomfromtext('MULTILINESTRING((100 100, 500 500), (600 600, 900 900))'), 'road1'), (st_geomfromtext('LINESTRING(200 200, 800 800)'), 'road2') ) t(geom, name) - WHERE ST_Intersects(geom, ST_TileEnvelope(5, 10, 12)) ) TO '__TEST_DIR__/test_roads.mvt' (FORMAT BLOB); query II From a78bfe420eb5c480eaab4cad3b11276cc5f39fb1 Mon Sep 17 00:00:00 2001 From: Jesper Paulsen Date: Thu, 23 Oct 2025 09:30:06 +0200 Subject: [PATCH 5/8] fix invalid geometries in ST_AsMVTGeom Changes GEOS_PREC_NO_TOPO to GEOS_PREC_VALID_OUTPUT in get_gridded() to ensure topological validity during grid snapping operations. --- src/spatial/modules/geos/geos_geometry.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/spatial/modules/geos/geos_geometry.hpp b/src/spatial/modules/geos/geos_geometry.hpp index 6ebf063c..a275e915 100644 --- a/src/spatial/modules/geos/geos_geometry.hpp +++ b/src/spatial/modules/geos/geos_geometry.hpp @@ -369,7 +369,7 @@ inline GeosGeometry GeosGeometry::get_transformed(const double matrix[6]) const } inline GeosGeometry GeosGeometry::get_gridded(double grid_size) const { - return GeosGeometry(handle, GEOSGeom_setPrecision_r(handle, geom, grid_size, GEOS_PREC_NO_TOPO)); + return GeosGeometry(handle, GEOSGeom_setPrecision_r(handle, geom, grid_size, GEOS_PREC_VALID_OUTPUT)); } inline GeosGeometry GeosGeometry::get_maximum_inscribed_circle() const { From 9d9825807471444e165d8087fffc78f34807cf06 Mon Sep 17 00:00:00 2001 From: Max Gabrielsson Date: Fri, 31 Oct 2025 10:53:47 +0100 Subject: [PATCH 6/8] format, always orient MVT geom after processing --- src/spatial/modules/geos/geos_module.cpp | 14 +++++++++----- src/spatial/modules/mvt/mvt_module.cpp | 18 +++++++++--------- 2 files changed, 18 insertions(+), 14 deletions(-) diff --git a/src/spatial/modules/geos/geos_module.cpp b/src/spatial/modules/geos/geos_module.cpp index 09bd42f7..c5424af5 100644 --- a/src/spatial/modules/geos/geos_module.cpp +++ b/src/spatial/modules/geos/geos_module.cpp @@ -327,9 +327,6 @@ struct ST_AsMVTGeom { const auto &blob = geom_data[geom_idx]; auto geom = lstate.Deserialize(blob); - // Orient polygons in place - geom.orient_polygons(true); - // Compute bounds const auto extent = bind_data.extent; @@ -363,10 +360,14 @@ struct ST_AsMVTGeom { const auto transformed = geom.get_transformed(affine_matrix); // Snap to grid (round coordinates to integers) - const auto snapped = transformed.get_gridded(1.0); + auto snapped = transformed.get_gridded(1.0); // Should we clip? if not, return the snapped geometry if (!bind_data.clip) { + + // But first orient in place + snapped.orient_polygons(true); + res_data[out_idx] = lstate.Serialize(result, snapped); continue; } @@ -385,7 +386,10 @@ struct ST_AsMVTGeom { } // Snap again to clean up any potential issues from clipping - const auto cleaned_clipped = clipped.get_gridded(1.0); + auto cleaned_clipped = clipped.get_gridded(1.0); + + // Also orient the polygons in place + cleaned_clipped.orient_polygons(true); res_data[out_idx] = lstate.Serialize(result, cleaned_clipped); } diff --git a/src/spatial/modules/mvt/mvt_module.cpp b/src/spatial/modules/mvt/mvt_module.cpp index 48907eaa..ef636733 100644 --- a/src/spatial/modules/mvt/mvt_module.cpp +++ b/src/spatial/modules/mvt/mvt_module.cpp @@ -609,15 +609,15 @@ class MVTFeatureBuilder { const auto y = CastDouble(cursor.Read()); cursor.Skip(vertex_space); // Skip z and m if present - if (vertex_idx == 0) { - geometry.push_back((1 & 0x7) | (1 << 3)); // MoveTo, 1 part - geometry.push_back(protozero::encode_zigzag32(x - cursor_x)); - geometry.push_back(protozero::encode_zigzag32(y - cursor_y)); - geometry.push_back((2 & 0x7) | ((vertex_count - 1) << 3)); // LineTo, part count - } else { - geometry.push_back(protozero::encode_zigzag32(x - cursor_x)); - geometry.push_back(protozero::encode_zigzag32(y - cursor_y)); - } + if (vertex_idx == 0) { + geometry.push_back((1 & 0x7) | (1 << 3)); // MoveTo, 1 part + geometry.push_back(protozero::encode_zigzag32(x - cursor_x)); + geometry.push_back(protozero::encode_zigzag32(y - cursor_y)); + geometry.push_back((2 & 0x7) | ((vertex_count - 1) << 3)); // LineTo, part count + } else { + geometry.push_back(protozero::encode_zigzag32(x - cursor_x)); + geometry.push_back(protozero::encode_zigzag32(y - cursor_y)); + } cursor_x = x; cursor_y = y; From c10ae36eab4dfb68e7113f95a12e911b1c547d31 Mon Sep 17 00:00:00 2001 From: Max Gabrielsson Date: Fri, 31 Oct 2025 10:55:14 +0100 Subject: [PATCH 7/8] fix typo --- src/spatial/modules/geos/geos_module.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/spatial/modules/geos/geos_module.cpp b/src/spatial/modules/geos/geos_module.cpp index c5424af5..d5b2556f 100644 --- a/src/spatial/modules/geos/geos_module.cpp +++ b/src/spatial/modules/geos/geos_module.cpp @@ -1290,7 +1290,7 @@ struct ST_DistanceWithin { }); func.SetDescription(R"( - Returns if two geometries are within a target distance of each-other + Returns true if two geometries are within a target distance of each-other )"); func.SetTag("ext", "spatial"); From af00774620046fb2332bc11a41837a998d5f1879 Mon Sep 17 00:00:00 2001 From: Max Gabrielsson Date: Fri, 31 Oct 2025 11:09:34 +0100 Subject: [PATCH 8/8] fix 2D_FromWKB functions --- .../modules/main/spatial_functions_scalar.cpp | 41 +++++++++++++++---- test/sql/geometry/st_2d_fromwkb.test | 19 +++++++++ 2 files changed, 51 insertions(+), 9 deletions(-) create mode 100644 test/sql/geometry/st_2d_fromwkb.test diff --git a/src/spatial/modules/main/spatial_functions_scalar.cpp b/src/spatial/modules/main/spatial_functions_scalar.cpp index 5387b778..a5aa86a8 100644 --- a/src/spatial/modules/main/spatial_functions_scalar.cpp +++ b/src/spatial/modules/main/spatial_functions_scalar.cpp @@ -4804,7 +4804,7 @@ struct ST_GeomFromWKB { y_data[i] = vertex.y; } - if (args.AllConstant()) { + if (args.AllConstant() || args.size() == 1) { result.SetVectorType(VectorType::CONSTANT_VECTOR); } } @@ -4872,7 +4872,7 @@ struct ST_GeomFromWKB { ListVector::SetListSize(result, total_size); - if (args.AllConstant()) { + if (args.AllConstant() || args.size() == 1) { result.SetVectorType(VectorType::CONSTANT_VECTOR); } } @@ -4967,7 +4967,7 @@ struct ST_GeomFromWKB { ListVector::SetListSize(result, total_ring_count); ListVector::SetListSize(ring_vec, total_point_count); - if (count == 1) { + if (args.AllConstant() || args.size() == 1) { result.SetVectorType(VectorType::CONSTANT_VECTOR); } } @@ -4986,8 +4986,16 @@ struct ST_GeomFromWKB { static void Register(ExtensionLoader &loader) { FunctionBuilder::RegisterScalar(loader, "ST_Point2DFromWKB", [](ScalarFunctionBuilder &builder) { builder.AddVariant([](ScalarFunctionVariantBuilder &variant) { - variant.AddParameter("point", GeoTypes::POINT_2D()); - variant.SetReturnType(GeoTypes::GEOMETRY()); + variant.AddParameter("wkb", GeoTypes::WKB_BLOB()); + variant.SetReturnType(GeoTypes::POINT_2D()); + + variant.SetInit(LocalState::Init); + variant.SetFunction(ExecutePoint); + }); + + builder.AddVariant([](ScalarFunctionVariantBuilder &variant) { + variant.AddParameter("blob", LogicalType::BLOB); + variant.SetReturnType(GeoTypes::POINT_2D()); variant.SetInit(LocalState::Init); variant.SetFunction(ExecutePoint); @@ -5001,8 +5009,16 @@ struct ST_GeomFromWKB { FunctionBuilder::RegisterScalar(loader, "ST_LineString2DFromWKB", [](ScalarFunctionBuilder &builder) { builder.AddVariant([](ScalarFunctionVariantBuilder &variant) { - variant.AddParameter("linestring", GeoTypes::LINESTRING_2D()); - variant.SetReturnType(GeoTypes::GEOMETRY()); + variant.AddParameter("wkb", GeoTypes::WKB_BLOB()); + variant.SetReturnType(GeoTypes::LINESTRING_2D()); + + variant.SetInit(LocalState::Init); + variant.SetFunction(ExecuteLineString); + }); + + builder.AddVariant([](ScalarFunctionVariantBuilder &variant) { + variant.AddParameter("blob", LogicalType::BLOB); + variant.SetReturnType(GeoTypes::LINESTRING_2D()); variant.SetInit(LocalState::Init); variant.SetFunction(ExecuteLineString); @@ -5016,8 +5032,15 @@ struct ST_GeomFromWKB { FunctionBuilder::RegisterScalar(loader, "ST_Polygon2DFromWKB", [](ScalarFunctionBuilder &builder) { builder.AddVariant([](ScalarFunctionVariantBuilder &variant) { - variant.AddParameter("polygon", GeoTypes::POLYGON_2D()); - variant.SetReturnType(GeoTypes::GEOMETRY()); + variant.AddParameter("wkb", GeoTypes::WKB_BLOB()); + variant.SetReturnType(GeoTypes::POLYGON_2D()); + + variant.SetInit(LocalState::Init); + variant.SetFunction(ExecutePolygon); + }); + builder.AddVariant([](ScalarFunctionVariantBuilder &variant) { + variant.AddParameter("blob", LogicalType::BLOB); + variant.SetReturnType(GeoTypes::POLYGON_2D()); variant.SetInit(LocalState::Init); variant.SetFunction(ExecutePolygon); diff --git a/test/sql/geometry/st_2d_fromwkb.test b/test/sql/geometry/st_2d_fromwkb.test new file mode 100644 index 00000000..539734f5 --- /dev/null +++ b/test/sql/geometry/st_2d_fromwkb.test @@ -0,0 +1,19 @@ +# name: test/sql/geometry/st_2d_fromwkb.test +# group: [geometry] + +require spatial + +query I +select ST_Point2DFromWKB(ST_AsWKB(ST_Point(1, 2))); +---- +POINT (1 2) + +query I +SELECT ST_Linestring2DFromWKB(ST_AsWKB(ST_GeomFromText('LINESTRING(0 0, 1 1, 2 2)'))); +---- +LINESTRING (0 0, 1 1, 2 2) + +query I +SELECT ST_Polygon2DFromWKB(ST_AsWKB(ST_GeomFromText('POLYGON((0 0, 1 0, 1 1, 0 1, 0 0))'))); +---- +POLYGON ((0 0, 1 0, 1 1, 0 1, 0 0)) \ No newline at end of file