From ad2a2ff05af8df1ba552d906e377afa345a449a7 Mon Sep 17 00:00:00 2001
From: Dan Goodliffe <dan@randomdan.homeip.net>
Date: Sun, 28 Jun 2026 18:47:14 +0100
Subject: Fix entity function sometimes returning null/wrong id

Includes tests over the errant behaviour.
Didn't match on type and control flow could return multiple rows.
---
 src/schema.sql       | 24 ++++++++++++------------
 test/test-ingest.cpp | 51 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 62 insertions(+), 13 deletions(-)

diff --git a/src/schema.sql b/src/schema.sql
index 81080ff..6f28f2d 100644
--- a/src/schema.sql
+++ b/src/schema.sql
@@ -57,11 +57,6 @@ DECLARE
 	recid integer;
 	nulldetail boolean;
 BEGIN
-	IF newValue IS NULL THEN
-		RETURN query
-	VALUES (NULL,
-		NULL);
-	END IF;
 	INSERT INTO entities(value, type)
 	SELECT
 		newValue,
@@ -72,26 +67,31 @@ BEGIN
 			FROM
 				entities
 			WHERE
-				md5(value) = md5(newValue))
+				md5(value) = md5(newValue)
+				AND type = newType)
 	ON CONFLICT
 		DO NOTHING
 	RETURNING
 		entities.id,
-		entities.detail IS NULL INTO recid,
+		entities.detail IS NULL
+	INTO
+		recid,
 		nulldetail;
 	IF recid IS NULL THEN
-		RETURN query
+		RETURN QUERY
 		SELECT
 			e.id,
 			e.detail IS NULL
 		FROM
 			entities e
 		WHERE
-			md5(e.value) = md5(newValue);
+			md5(e.value) = md5(newValue)
+			AND e.type = newType;
+	ELSE
+		RETURN QUERY
+	VALUES (recid,
+		nulldetail);
 	END IF;
-	RETURN query
-VALUES (recid,
-	nulldetail);
 END;
 $$
 LANGUAGE plpgSQL
diff --git a/test/test-ingest.cpp b/test/test-ingest.cpp
index 408e720..9860f08 100644
--- a/test/test-ingest.cpp
+++ b/test/test-ingest.cpp
@@ -7,6 +7,7 @@
 #include <selectcommandUtil.impl.h>
 
 #include <ingestor.hpp>
+#include <sql.hpp>
 #include <uaLookup.hpp>
 
 namespace {
@@ -290,8 +291,19 @@ public:
 	mutable size_t logsWritten = 0;
 };
 
-BOOST_FIXTURE_TEST_SUITE(I, TestIngestor);
+static constexpr std::array<std::string_view, 9> ENTITY_TYPE_VALUES {{
+		{"host"},
+		{"virtual_host"},
+		{"path"},
+		{"query_string"},
+		{"referrer"},
+		{"user_agent"},
+		{"unparsable_line"},
+		{"uninsertable_line"},
+		{"content_type"},
+}};
 
+BOOST_FIXTURE_TEST_SUITE(I, TestIngestor);
 BOOST_TEST_DECORATOR(*boost::unit_test::depends_on("ExtractFields"))
 
 BOOST_DATA_TEST_CASE(StoreLogLine,
@@ -537,6 +549,43 @@ BOOST_AUTO_TEST_CASE(LogResetSignal)
 	BOOST_CHECK_EQUAL(stats, Stats {});
 }
 
+using CreateEntitiesData = std::tuple<std::string_view, WebStat::EntityType, int>;
+
+BOOST_DATA_TEST_CASE(CreateEntities,
+		boost::unit_test::data::make<CreateEntitiesData>({
+				{"host1", WebStat::EntityType::Host, 31},
+				{"host2", WebStat::EntityType::Host, 33},
+				{"host3", WebStat::EntityType::Host, 35},
+				{"host1", WebStat::EntityType::Host, 31},
+				{"host2", WebStat::EntityType::Host, 33},
+				{"host3", WebStat::EntityType::Host, 35},
+				{"host1", WebStat::EntityType::VirtualHost, 40},
+				{"host2", WebStat::EntityType::VirtualHost, 42},
+				{"host3", WebStat::EntityType::VirtualHost, 44},
+				{"host1", WebStat::EntityType::VirtualHost, 40},
+				{"host2", WebStat::EntityType::VirtualHost, 42},
+				{"host3", WebStat::EntityType::VirtualHost, 44},
+				{"host2", WebStat::EntityType::Host, 33},
+				{"host1", WebStat::EntityType::VirtualHost, 40},
+				{"host3", WebStat::EntityType::Host, 35},
+				{"host1", WebStat::EntityType::Host, 31},
+		}),
+		value, type, expectedId)
+{
+	auto dbc = dbpool->get();
+	auto entityInsert = dbc->select(WebStat::SQL::ENTITY_INSERT, WebStat::SQL::ENTITY_INSERT_OPTS);
+	entityInsert->bindParamS(0, value);
+	entityInsert->bindParamS(1, ENTITY_TYPE_VALUES[std::to_underlying(type)]);
+	BOOST_REQUIRE_NO_THROW(entityInsert->execute());
+	size_t rows = 0;
+	for (auto [entityId, detailIsNull] : entityInsert->as<int, bool>()) {
+		++rows;
+		BOOST_REQUIRE_EQUAL(rows, 1);
+		BOOST_CHECK_EQUAL(entityId, expectedId);
+		BOOST_CHECK_EQUAL(detailIsNull, true);
+	}
+}
+
 BOOST_AUTO_TEST_SUITE_END();
 
 BOOST_AUTO_TEST_CASE(FetchRealUserAgentDetail, *boost::unit_test::disabled())
-- 
cgit v1.3