Skip to content

Conversation

@StounhandJ
Copy link

  • Do only one thing
  • Non breaking API changes
  • Tested

What did this pull request do?

When creating a Count request, if a Select was passed with count, substitute it in Clause to avoid unexpected addition of the request.

User Case Description

InnerJoins is used here as adding an internal verification condition.

r.db.Model(&models.KubeRBAC{}).
	Select("count(distinct(subj->>'kind', subj->>'apiGroup', subj->>'name', subj->>'namespace', \"ClusterRef\".id))").
	InnerJoins("ClusterRef").
	Count(&total)

Extra fields appear in the final sql, which violates the correctness of the work.

SELECT
  count(distinct(
    subj->>'kind',
    subj->>'apiGroup',
    subj->>'name',
    subj->>'namespace',
    "ClusterRef".id
  )),
  "ClusterRef"."id"         AS "ClusterRef__id",
  "ClusterRef"."name"       AS "ClusterRef__name",
  "ClusterRef"."updated_at" AS "ClusterRef__updated_at",
  "ClusterRef"."created_at" AS "ClusterRef__created_at"
FROM
  "kube_rbac"
INNER JOIN
  "kube_clusters" "ClusterRef"
    ON "kube_rbac"."kube_cluster_id" = "ClusterRef"."id"
    AND "ClusterRef"."updated_at" > now() - '1 hours'::interval;

The result after correction

SELECT
  count(distinct(
    subj->>'kind',
    subj->>'apiGroup',
    subj->>'name',
    subj->>'namespace',
    "ClusterRef".id
  ))
FROM
  "kube_rbac"
INNER JOIN
  "kube_clusters" "ClusterRef"
    ON "kube_rbac"."kube_cluster_id" = "ClusterRef"."id"
    AND "ClusterRef"."updated_at" > now() - '1 hours'::interval;

@propel-code-bot
Copy link
Contributor

propel-code-bot bot commented Oct 22, 2025

Fix: prevent duplicate select columns when Count() receives an explicit COUNT expression

The PR adjusts the Count() implementation so that when the caller has already supplied an explicit COUNT(...) expression via Select(), GORM will reuse that expression as the SELECT clause instead of auto-appending its own COUNT(*). This eliminates unintended extra columns in generated SQL, especially when JOINs are present. A corresponding regression test covering DISTINCT with JOIN has been added.

Key Changes

• Added prefix check for count( in finisher_api.go and inject clause.Select with the provided expression instead of defaulting to COUNT(*)
• Extended tests/count_test.go with two dry-run cases validating Distinct + Join path

Affected Areas

finisher_api.go – logic inside Count() around clause.Select handling
tests/count_test.go – new test cases

This summary was automatically generated by @propel-code-bot

@jinzhu
Copy link
Member

jinzhu commented Oct 30, 2025

Hi @StounhandJ

Can you add some tests?

@propel-code-bot propel-code-bot bot changed the title Count always add Clause Fix Count: honor custom SELECT count expressions, avoid extra columns Nov 7, 2025
@StounhandJ
Copy link
Author

Hi @jinzhu

I missed the message, but here are two Count tests along with the Join

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants