Use Sql builder - squirrel

[michalkrzyz]

On-behalf-of: SAP Michal Krzyz michal.krzyz@sap.com

Description

Use squirrel for most querries.

Please include a summary of the changes and the related issue. Please also include relevant motivation and context. List any dependencies that are required for this change.

What type of PR is this? (check all applicable)

• 🍕 Feature
• 🐛 Bug Fix
• 📝 Documentation Update
• 🎨 Style
• 🧑‍💻 Code Refactor
• 🔥 Performance Improvements
• ✅ Test
• 🤖 Build
• 🔁 CI
• 📦 Chore (Release)
• ⏩ Revert

Related Tickets & Documents

• Related Issue # (issue)
• Closes # (issue)
• Fixes # (issue)

Remove if not applicable

Added tests?

• 👍 yes
• 🙅 no, because they aren't needed
• 🙋 no, because I need help
• Separate ticket for tests # (issue/pr)

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

Added to documentation?

• 📜 README.md
• 🤝 Documentation pages updated
• 🙅 no documentation needed
• (if applicable) generated OpenAPI docs for CRD changes

Checklist

• My code follows the style guidelines of this project
• I have performed a self-review of my code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes

[Copilot]

Pull request overview

Refactors most read-side MariaDB queries in internal/database/mariadb/* from fmt.Sprintf-templated SQL strings to the Masterminds/squirrel SQL builder. The shared Statement/BuildStatement machinery and DbObject/FilterProperty types are reshaped to operate on a sq.SelectBuilder, while two not-yet-migrated aggregation paths still rely on _tmp-suffixed copies of the old helpers.

Changes:

• Replace raw SQL templates and EnsureFilter/getXColumns helpers in 12 entity files with sq.Select(...).From(...).GroupBy(...) builders feeding a unified BuildStatement.
• Rework db_object.go: introduce AddJoins/AddFilter/AddCursor, rename FilterProperty fields to BuildQuery/GetParam/BuildParams, drop the Check* statement flags, and add *_tmp duplicates of the old join/filter/parameter helpers for the remaining aggregation queries.
• Minor cleanups: switch a few logrus.Warnf to scoped l.Warnf, pass ctx instead of context.Background() to performListScan, and refactor getXColumns(...) into appendXColumns([]string, ...) []string.

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
internal/database/mariadb/db_object.go	Core refactor: squirrel-based BuildStatement, new AddJoins/AddFilter/AddCursor, renamed FilterProperty fields, plus _tmp duplicates of legacy helpers.
internal/database/mariadb/user.go	Switch user queries to squirrel; comment out removed Check* flags.
internal/database/mariadb/support_group.go	Same migration for support group queries.
internal/database/mariadb/service.go	Migrate service queries; route GetServicesWithAggregations through *_tmp helpers.
internal/database/mariadb/service_issue_variant.go	Migrate to squirrel; embed multi-line JoinClause for issue-variant joins.
internal/database/mariadb/remediation.go	Migrate remediation queries; minor logger scoping fix.
internal/database/mariadb/patch.go	Migrate patch queries.
internal/database/mariadb/issue.go	Migrate issue queries; route GetIssuesWithAggregations through *_tmp helpers; rework getIssueColumns to appendIssueColumns.
internal/database/mariadb/issue_variant.go	Migrate issue-variant queries.
internal/database/mariadb/issue_repository.go	Migrate issue-repository queries.
internal/database/mariadb/issue_match.go	Migrate issue-match queries; refactor column helper.
internal/database/mariadb/component.go	Migrate component queries; refactor column helper.
internal/database/mariadb/component_version.go	Migrate component-version queries; refactor column helper.
internal/database/mariadb/component_instance.go	Migrate component-instance queries; adds GroupBy in getComponentInstanceAttr.

Comments suppressed due to low confidence (3)

internal/database/mariadb/db_object.go:161

• AddCursor calls qb.Having(...) and qb.Where(...) but discards their return values. Squirrel's SelectBuilder is immutable — these methods return a new builder, so the cursor predicate is never actually attached to the query. Only the Limit call takes effect because it is in the return statement. This silently drops the cursor WHERE/HAVING clause for every paginated query that uses BuildStatement. The assignments should be qb = qb.Having(...) and qb = qb.Where(...).

	if aggregated {
		qb.Having(cursorQuery, cursorParams...)
	} else {
		qb.Where(cursorQuery, cursorParams...)
	}
	return qb.Limit(uint64(limit))

internal/database/mariadb/db_object.go:161

• uint64(limit) will wrap around to a very large value when pagFirst is set to a negative int (e.g. -1 used elsewhere in tests/API callers as "no limit"). Consider clamping limit to a non-negative value before the conversion, or treating non-positive values the same as the nil default.

	return qb.Limit(uint64(limit))

internal/database/mariadb/db_object.go:755

• A large set of *_tmp duplicates (GetJoins_tmp, Build_tmp, GetFilterWhereClause_tmp, GetFilterQuery_tmp, GetFilterParameters_tmp) has been introduced solely so the not-yet-migrated GetServicesWithAggregations / GetIssuesWithAggregations paths still compile. This duplicates a lot of non-trivial logic (the Build_tmp body is a copy of Build) and there is no tracked follow-up or ticket reference. At minimum, please add a // TODO(...) referencing a concrete issue, and consider keeping a single implementation (e.g. by extracting the shared body and re-using it from both Build and Build_tmp) to avoid the two copies drifting apart.

// DEPRECATED: TODO: remove
func (do *DbObject[ET]) GetJoins_tmp(filter any, order *Order) string {
	return NewJoinResolver(do.JoinDefs).Build_tmp(filter, order)
}

func (jr *JoinResolver) Build_tmp(filter any, order *Order) string {
	for _, def := range jr.defs {
		if def.Condition != nil && def.Condition(filter, order) {
			jr.require(def.Name)
		}
	}

	var result []string

	// This is little tricky part, but we need to deal with that this way
	// until we have stateful join pattern which is created for issue.go
	// with non-uniq tablename 'IM IssueMatch' which join operation
	// depends on filter pattern with precedence for some members (there
	// is if...else if which cannot be replaced by if... and if... what
	// is a mess and misconception
	uniqTableName := make(map[string]struct{})

	for _, name := range jr.order {
		j, ok := lo.Find(jr.defs, func(jd *JoinDef) bool {
			return jd.Name == name
		})
		if !ok {
			panic("JoinResolver::Build(...) Unknown join: " + name)
		}

		if _, ok := uniqTableName[j.Table]; ok {
			continue
		}

		uniqTableName[j.Table] = struct{}{}

		joinSQL := fmt.Sprintf("%s %s ON %s",
			j.Type,
			j.Table,
			j.On,
		)

		result = append(result, joinSQL)
	}

	return strings.Join(result, "\n")
}

func (do *DbObject[ET]) GetFilterWhereClause_tmp(filter any, withCursor bool) string {
	if filterStr := do.GetFilterQuery_tmp(filter); filterStr != "" || withCursor {
		return fmt.Sprintf("WHERE %s", filterStr)
	}

	return ""
}

func (do *DbObject[ET]) GetFilterQuery_tmp(filter any) string {
	var fl []string
	for _, v := range do.FilterProperties {
		fl = append(fl, v.GetQuery(filter))
	}

	return combineFilterQueries(fl, OP_AND)
}

func (do *DbObject[ET]) GetFilterParameters_tmp(
	filter entity.HasPagination,
	withCursor bool,
	cursorFields []Field,
) []any {
	var filterParameters []any
	for _, v := range do.FilterProperties {
		filterParameters = append(filterParameters, v.BuildParams(filter)...)
	}

	if withCursor {
		paginated := filter.GetPaginated()
		filterParameters = append(
			filterParameters,
			GetCursorQueryParameters(paginated.First, cursorFields)...)
	}

	return filterParameters
}

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.