Merge pull request #2737 from KhokhlovDV/fix/gc-builder-placeholders

fix: use gc db builder for correct query placheolder. #2375

Author: omer-topal

internal/storage/postgres/gc/gc.go

@@ -195,7 +195,7 @@ func (gc *GC) getLastTransactionIDForTenant(ctx context.Context, tenantID string
 
 // deleteRecordsForTenant generates and executes DELETE queries for relation_tuples and attributes tables for a specific tenant.
 func (gc *GC) deleteRecordsForTenant(ctx context.Context, table, tenantID string, lastTransactionID uint64) error {
-	queryBuilder := utils.GenerateGCQueryForTenant(table, tenantID, lastTransactionID)
+	queryBuilder := utils.GenerateGCQueryForTenant(gc.database.Builder, table, tenantID, lastTransactionID)
 	query, args, err := queryBuilder.ToSql()
 	if err != nil {
 		return err

internal/storage/postgres/utils/common.go

@@ -85,9 +85,9 @@ func SnapshotQuery(sl squirrel.SelectBuilder, value uint64, snapshotValue string
 // GenerateGCQuery generates a Squirrel DELETE query builder for garbage collection.
 // It constructs a query to delete expired records from the specified table
 // based on the provided value, which represents a transaction ID.
-func GenerateGCQuery(table string, value uint64) squirrel.DeleteBuilder {
+func GenerateGCQuery(sl squirrel.StatementBuilderType, table string, value uint64) squirrel.DeleteBuilder {
 	// Create a Squirrel DELETE builder for the specified table.
-	deleteBuilder := squirrel.Delete(table)
+	deleteBuilder := sl.Delete(table)
 
 	// Create an expression to check if 'expired_tx_id' is not equal to ActiveRecordTxnID (expired records).
 	expiredNotActiveExpr := squirrel.Expr("expired_tx_id <> ?::xid8", ActiveRecordTxnID)
@@ -102,9 +102,9 @@ func GenerateGCQuery(table string, value uint64) squirrel.DeleteBuilder {
 // GenerateGCQueryForTenant generates a Squirrel DELETE query builder for tenant-aware garbage collection.
 // It constructs a query to delete expired records from the specified table for a specific tenant
 // based on the provided value, which represents a transaction ID.
-func GenerateGCQueryForTenant(table, tenantID string, value uint64) squirrel.DeleteBuilder {
+func GenerateGCQueryForTenant(sl squirrel.StatementBuilderType, table, tenantID string, value uint64) squirrel.DeleteBuilder {
 	// Create a Squirrel DELETE builder for the specified table.
-	deleteBuilder := squirrel.Delete(table)
+	deleteBuilder := sl.Delete(table)
 
 	// Create an expression to check if 'expired_tx_id' is not equal to ActiveRecordTxnID (expired records).
 	expiredNotActiveExpr := squirrel.Expr("expired_tx_id <> ?::xid8", ActiveRecordTxnID)

internal/storage/postgres/utils/common_test.go

@@ -69,9 +69,10 @@ var _ = Describe("Common", func() {
 		})
 	})
 
-	Context("TestGarbageCollectQuery", func() {
+	Context("TestGarbageCollectQuery question placeholder", func() {
 		It("Case 1", func() {
-			query := utils.GenerateGCQuery("relation_tuples", 100)
+			sl := squirrel.StatementBuilder.PlaceholderFormat(squirrel.Question)
+			query := utils.GenerateGCQuery(sl, "relation_tuples", 100)
 			sql, args, err := query.ToSql()
 			Expect(err).ShouldNot(HaveOccurred())
 
@@ -81,7 +82,8 @@ var _ = Describe("Common", func() {
 		})
 
 		It("Case 2 - Tenant Aware", func() {
-			query := utils.GenerateGCQueryForTenant("relation_tuples", "tenant1", 100)
+			sl := squirrel.StatementBuilder.PlaceholderFormat(squirrel.Question)
+			query := utils.GenerateGCQueryForTenant(sl, "relation_tuples", "tenant1", 100)
 			sql, args, err := query.ToSql()
 			Expect(err).ShouldNot(HaveOccurred())
 
@@ -91,6 +93,30 @@ var _ = Describe("Common", func() {
 		})
 	})
 
+	Context("TestGarbageCollectQuery dollar placeholder", func() {
+		It("Case 1", func() {
+			sl := squirrel.StatementBuilder.PlaceholderFormat(squirrel.Dollar)
+			query := utils.GenerateGCQuery(sl, "relation_tuples", 100)
+			sql, args, err := query.ToSql()
+			Expect(err).ShouldNot(HaveOccurred())
+
+			expectedSQL := "DELETE FROM relation_tuples WHERE expired_tx_id <> $1::xid8 AND expired_tx_id < $2::xid8"
+			Expect(expectedSQL).Should(Equal(sql))
+			Expect(args).Should(Equal([]interface{}{utils.ActiveRecordTxnID, uint64(100)}))
+		})
+
+		It("Case 2 - Tenant Aware", func() {
+			sl := squirrel.StatementBuilder.PlaceholderFormat(squirrel.Dollar)
+			query := utils.GenerateGCQueryForTenant(sl, "relation_tuples", "tenant1", 100)
+			sql, args, err := query.ToSql()
+			Expect(err).ShouldNot(HaveOccurred())
+
+			expectedSQL := "DELETE FROM relation_tuples WHERE tenant_id = $1 AND expired_tx_id <> $2::xid8 AND expired_tx_id < $3::xid8"
+			Expect(expectedSQL).Should(Equal(sql))
+			Expect(args).Should(Equal([]interface{}{"tenant1", utils.ActiveRecordTxnID, uint64(100)}))
+		})
+	})
+
 	Context("Error Handling", func() {
 		var (
 			ctx  context.Context