Merge pull request #653 from Maxxen/main-dev

Misc bugfixes

Author: Maxxen

src/spatial/modules/main/spatial_functions_scalar.cpp

@@ -2018,11 +2018,11 @@ struct ST_CollectionExtract {
 struct ST_Contains {
 
 	//------------------------------------------------------------------------------------------------------------------
-	// POINT_2D -> POLYGON_2D
+	// POLYGON_2D -> POINT_2D
 	//------------------------------------------------------------------------------------------------------------------
 	// TODO: This should probably be revised. Im not sure if the current implementation is entirely accurate
 
-	static void Operation(Vector &in_point, Vector &in_polygon, Vector &result, idx_t count) {
+	static void Operation(Vector &in_polygon, Vector &in_point, Vector &result, idx_t count) {
 		enum class Side { LEFT, RIGHT, ON };
 
 		in_polygon.Flatten(count);
@@ -2043,7 +2043,6 @@ struct ST_Contains {
 		auto y_data = FlatVector::GetData<double>(*coord_children[1]);
 
 		auto result_data = FlatVector::GetData<bool>(result);
-
 		for (idx_t polygon_idx = 0; polygon_idx < count; polygon_idx++) {
 			auto polygon = polygon_entries[polygon_idx];
 			auto polygon_offset = polygon.offset;
@@ -8702,7 +8701,7 @@ struct ST_Within {
 		auto &polygon_in = args.data[1];
 
 		// Just execute ST_Contains, but reversed
-		ST_Contains::Operation(point_in, polygon_in, result, args.size());
+		ST_Contains::Operation(polygon_in, point_in, result, args.size());
 	}
 
 	//------------------------------------------------------------------------------------------------------------------

src/spatial/operators/spatial_join_optimizer.cpp

@@ -7,16 +7,17 @@
 #include "duckdb/planner/operator/logical_any_join.hpp"
 #include "duckdb/catalog/catalog_entry/scalar_function_catalog_entry.hpp"
 #include "duckdb/planner/operator/logical_filter.hpp"
+#include "spatial/spatial_types.hpp"
 
 namespace duckdb {
 
 // All of these imply bounding box intersection
-static case_insensitive_set_t spatial_predicate_map = {
+static const case_insensitive_set_t spatial_predicate_map = {
     "ST_Equals",   "ST_Intersects", "ST_Touches",   "ST_Crosses",          "ST_Within",         "ST_Contains",
     "ST_Overlaps", "ST_Covers",     "ST_CoveredBy", "ST_ContainsProperly", "ST_WithinProperly", "ST_DWithin",
 };
 
-static case_insensitive_map_t<string> spatial_predicate_inverse_map = {
+static const case_insensitive_map_t<string> spatial_predicate_inverse_map = {
     {"ST_Equals", "ST_Equals"},
     {"ST_Intersects", "ST_Intersects"},           // Symmetric
     {"ST_Touches", "ST_Touches"},                 // Symmetric
@@ -139,6 +140,13 @@ static void InsertSpatialJoin(OptimizerExtensionInput &input, unique_ptr<Logical
 			continue;
 		}
 
+		// The function must operate on two GEOMETRY types
+		if (func.children[0]->return_type != GeoTypes::GEOMETRY() ||
+		    func.children[1]->return_type != GeoTypes::GEOMETRY()) {
+			extra_predicates.push_back(std::move(expr));
+			continue;
+		}
+
 		// The function must be a recognized spatial predicate
 		if (spatial_predicate_map.count(func.function.name) == 0) {
 			extra_predicates.push_back(std::move(expr));

src/spatial/operators/spatial_join_physical.cpp

@@ -452,6 +452,8 @@ class SpatialJoinGlobalState final : public GlobalSinkState {
 
 	// This is initialized in the finalize state
 	unique_ptr<FlatRTree> rtree = nullptr;
+
+	mutex combine_lock;
 };
 
 unique_ptr<GlobalSinkState> PhysicalSpatialJoin::GetGlobalSinkState(ClientContext &context) const {
@@ -580,6 +582,7 @@ SinkCombineResultType PhysicalSpatialJoin::Combine(ExecutionContext &context, Op
 	lstate.collection->FinalizePinState(lstate.append_state.pin_state);
 
 	// Append the local collection to the global collection
+	lock_guard<mutex> lock(gstate.combine_lock);
 	gstate.collection->Combine(*lstate.collection);
 
 	// Merge the non-null and non-empty count
@@ -1019,14 +1022,13 @@ class SpatialJoinGlobalSourceState final : public GlobalSourceState {
 		column_ids.push_back(op.build_side_key_types.size() + op.build_side_payload_types.size());
 
 		// We dont need to keep the tuples aroun after scanning
-		state.collection->InitializeScan(scan_state, std::move(column_ids), TupleDataPinProperties::DESTROY_AFTER_DONE);
+		state.collection->InitializeScan(scan_state, std::move(column_ids), TupleDataPinProperties::KEEP_EVERYTHING_PINNED);
 
 		tuples_maximum = state.collection->Count();
 	}
 
 	const PhysicalSpatialJoin &op;
 	TupleDataParallelScanState scan_state;
-
 	// How many tuples we have scanned so far
 	idx_t tuples_maximum = 0;
 	atomic<idx_t> tuples_scanned = {0};
@@ -1056,7 +1058,7 @@ class SpatialJoinLocalSourceState final : public LocalSourceState {
 		column_ids.push_back(op.build_side_key_types.size() + op.build_side_payload_types.size());
 
 		// We dont need to keep the tuples aroun after scanning
-		state.collection->InitializeScan(scan_state, std::move(column_ids), TupleDataPinProperties::DESTROY_AFTER_DONE);
+		state.collection->InitializeScan(scan_state, std::move(column_ids));
 		state.collection->InitializeScanChunk(scan_state, scan_chunk);
 	}
 

test/sql/join/spatial_join_cte.test

@@ -0,0 +1,29 @@
+# name: test/sql/join/spatial_join_cte.test
+# group: [join]
+
+require spatial
+
+query IIII
+with bigger as (
+select 'a' as region, ST_GeomFromText('POLYGON ((0 0, 0 10, 10 10, 10 0 , 0 0 ))') as geom
+      union all
+select 'b' as region, ST_GeomFromText('POLYGON ((50 50, 50 60, 60 60, 60 50 , 50 50 ))') as geom
+      union all
+select 'c' as region, ST_GeomFromText('POLYGON ((50 50, 50 60, 60 60, 60 50 , 50 50 ))') as geom
+      union all
+select 'd' as region, ST_GeomFromText('POLYGON ((50 50, 50 60, 60 60, 60 50 , 50 50 ))') as geom
+      union all
+select 'e' as region, ST_GeomFromText('POLYGON ((50 50, 50 60, 60 60, 60 50 , 50 50 ))') as geom
+      union all
+select 'f' as region, ST_GeomFromText('POLYGON ((50 50, 50 60, 60 60, 60 50 , 50 50 ))') as geom
+      union all
+select 'g' as region, ST_GeomFromText('POLYGON ((50 50, 50 60, 60 60, 60 50 , 50 50 ))') as geom
+), smaller as  (
+  select '1' as region, ST_GeomFromText('POLYGON ((2 2, 2 8, 8 8, 8 2 , 2 2 ))') as geom
+  union all
+  select '2' as region, ST_GeomFromText('POLYGON ((3 3, 3 5, 5 5, 5 3 , 3 3 ))') as geom
+)
+select * from smaller left join bigger on st_intersects(bigger.geom, smaller.geom) order by all;
+----
+1	POLYGON ((2 2, 2 8, 8 8, 8 2, 2 2))	a	POLYGON ((0 0, 0 10, 10 10, 10 0, 0 0))
+2	POLYGON ((3 3, 3 5, 5 5, 5 3, 3 3))	a	POLYGON ((0 0, 0 10, 10 10, 10 0, 0 0))