From 761a8f6c3124a3130951e3926f3dfc7be385bb18 Mon Sep 17 00:00:00 2001
From: Georges Chaudy <chaudyg@gmail.com>
Date: Mon, 17 Nov 2025 17:39:50 +0100
Subject: [PATCH] Enhance resource_history SQL update to include key_path
 column

- Updated SQL to set key_path based on resource attributes and action type.
- Removed the migration to make key_path NOT NULL, keeping it nullable to accommodate specific write patterns.
- Adjusted comments to clarify the rationale behind key_path's nullable status.

This change improves the handling of resource history updates and maintains flexibility in data management.
---
 .../sql/data/resource_history_update_rv.sql   | 48 ++++++++++++++++---
 .../unified/sql/db/migrations/resource_mig.go | 36 ++------------
 2 files changed, 44 insertions(+), 40 deletions(-)

diff --git a/pkg/storage/unified/sql/data/resource_history_update_rv.sql b/pkg/storage/unified/sql/data/resource_history_update_rv.sql
index 767338ddb34..31b293d0ce4 100644
--- a/pkg/storage/unified/sql/data/resource_history_update_rv.sql
+++ b/pkg/storage/unified/sql/data/resource_history_update_rv.sql
@@ -1,11 +1,45 @@
 UPDATE {{ .Ident "resource_history" }}
-SET {{ .Ident "resource_version" }} = (
-    CASE
-    {{ range $guid, $rv := .GUIDToRV }}
-    WHEN {{ $.Ident "guid" }} = {{ $.Arg $guid }} THEN CAST({{ $.Arg $rv }} AS {{ if eq $.DialectName "postgres" }}BIGINT{{ else }}SIGNED{{ end }})
-    {{ end }}
-    END
-)
+SET 
+    {{ .Ident "resource_version" }} = (
+        CASE
+        {{ range $guid, $rv := .GUIDToRV }}
+        WHEN {{ $.Ident "guid" }} = {{ $.Arg $guid }} THEN CAST({{ $.Arg $rv }} AS {{ if eq $.DialectName "postgres" }}BIGINT{{ else }}SIGNED{{ end }})
+        {{ end }}
+        END
+    ),
+    {{ .Ident "key_path" }} = {{ if eq .DialectName "sqlite" -}}
+        {{ .Ident "group" }} || CHAR(47) || {{ .Ident "resource" }} || CHAR(47) || {{ .Ident "namespace" }} || CHAR(47) || {{ .Ident "name" }} || CHAR(47) ||
+            CAST((CASE
+                {{- range $guid, $rv := .GUIDToRV }}
+                WHEN {{ $.Ident "guid" }} = {{ $.Arg $guid }} THEN ((({{ $.Arg $rv }} / 1000) - 1288834974657) * 4194304) + ({{ $.Arg $rv }} % 1000)
+                {{- end }}
+            END) AS TEXT) || CHAR(126) ||
+            CASE {{ .Ident "action" }}
+                WHEN 1 THEN 'created'
+                WHEN 2 THEN 'updated'
+                WHEN 3 THEN 'deleted'
+                ELSE 'unknown'
+            END || CHAR(126) || COALESCE({{ .Ident "folder" }}, '')
+        {{- else -}}
+        CONCAT(
+            {{ .Ident "group" }}, CHAR(47),
+            {{ .Ident "resource" }}, CHAR(47),
+            {{ .Ident "namespace" }}, CHAR(47),
+            {{ .Ident "name" }}, CHAR(47),
+            CAST((CASE
+                {{- range $guid, $rv := .GUIDToRV }}
+                WHEN {{ $.Ident "guid" }} = {{ $.Arg $guid }} THEN ((({{ $.Arg $rv }} DIV 1000) - 1288834974657) * 4194304) + ({{ $.Arg $rv }} MOD 1000)
+                {{- end }}
+            END) AS {{ if eq .DialectName "postgres" }}TEXT{{ else }}CHAR{{ end }}), CHAR(126),
+            CASE {{ .Ident "action" }}
+                WHEN 1 THEN 'created'
+                WHEN 2 THEN 'updated'
+                WHEN 3 THEN 'deleted'
+                ELSE 'unknown'
+            END, CHAR(126),
+            COALESCE({{ .Ident "folder" }}, '')
+        )
+        {{- end }}
 WHERE {{ .Ident "guid" }} IN (
     {{$first := true}}
     {{ range $guid, $rv := .GUIDToRV }}{{if $first}}{{$first = false}}{{else}}, {{end}}{{ $.Arg $guid }}{{ end }}
diff --git a/pkg/storage/unified/sql/db/migrations/resource_mig.go b/pkg/storage/unified/sql/db/migrations/resource_mig.go
index c044b15ec8e..a095f8997dc 100644
--- a/pkg/storage/unified/sql/db/migrations/resource_mig.go
+++ b/pkg/storage/unified/sql/db/migrations/resource_mig.go
@@ -198,8 +198,9 @@ func initResourceTables(mg *migrator.Migrator) string {
 	// Backfill key_path column in resource_history
 	mg.AddMigration("Backfill key_path column in resource_history", &resourceHistoryKeyBackfillMigrator{})
 
-	// Make key_path column NOT NULL after backfill
-	mg.AddMigration("Make key_path column NOT NULL in resource_history", &makeKeyColumnNotNullMigrator{})
+	// Note: key_path remains nullable because the write pattern is:
+	// 1. INSERT (key_path = NULL)
+	// 2. UPDATE (key_path = actual value after RV allocation)
 
 	// Add index on key_path column
 	mg.AddMigration("Add index on key_path column in resource_history", migrator.NewAddIndexMigration(resource_history_table, &migrator.Index{
@@ -353,34 +354,3 @@ func (m *resourceHistoryKeyBackfillMigrator) Exec(sess *xorm.Session, mg *migrat
 	logger.Info("Backfill completed", "total_processed", processed)
 	return nil
 }
-
-// makeKeyColumnNotNullMigrator makes the key_path column NOT NULL after backfill
-type makeKeyColumnNotNullMigrator struct {
-	migrator.MigrationBase
-}
-
-func (m *makeKeyColumnNotNullMigrator) SQL(dialect migrator.Dialect) string {
-	return "Make key_path column NOT NULL in resource_history"
-}
-
-func (m *makeKeyColumnNotNullMigrator) Exec(sess *xorm.Session, mg *migrator.Migrator) error {
-	dialect := mg.Dialect.DriverName()
-
-	var alterSQL string
-
-	switch dialect {
-	case "mysql":
-		alterSQL = "ALTER TABLE resource_history MODIFY key_path VARCHAR(2048) NOT NULL"
-	case "postgres":
-		alterSQL = "ALTER TABLE resource_history ALTER COLUMN key_path SET NOT NULL"
-	case "sqlite3":
-		// SQLite doesn't support ALTER COLUMN directly, so we skip this for SQLite
-		// The column will remain nullable in SQLite, which is acceptable
-		return nil
-	default:
-		return fmt.Errorf("unsupported database dialect: %s", dialect)
-	}
-
-	_, err := sess.Exec(alterSQL)
-	return err
-}