bob icon indicating copy to clipboard operation
bob copied to clipboard

.One() does not add a limit - resulting in unexpectedly large queries

Open barneyferry opened this issue 3 months ago • 3 comments

Noticed this while investigating an increasingly large query to get the latest "version" of a resource, query.One does not seem to impose a limit at all, it simply calls

	sql, args, err := Build(ctx, q)
	if err != nil {
		return t, err
	}

	if l, ok := q.(MapperModder); ok {
		if loaders := l.GetMapperMods(); len(loaders) > 0 {
			m = scan.Mod(m, loaders...)
		}
	}

	t, err = scan.One(ctx, exec, m, sql, args...)
	if err != nil {
		return t, err
	}

meaning every .One call actually needs:

	query.Expression.Limit.SetLimit(1)

	r, err := query.One(ctx, q.db)

which is quite misleading compared to other ORMs I've used. The "One" implies it will query one row, not query all rows and only return one.

barneyferry avatar Sep 25 '25 15:09 barneyferry

Well........................ it isn't easy to reliably make this change.

While it is possible to change this for built queries, there are many queries that are impossible to limit. For example:

// No way to easily know if this is limited, or add a limit
psql.Raw("SELECT * FROM users")

I think not having it, is better than having it but it only applies sometimes. Being reliable and predictable is essential.

However, to prevent misunderstandings, I think the documentation of the One method should make it clear that no limit is applied, and the query is run as-is

stephenafamo avatar Sep 26 '25 11:09 stephenafamo

Or maybe the name should be changed from .One() to .First()?

jacobmolby avatar Sep 28 '25 15:09 jacobmolby

Or maybe the name should be changed from .One() to .First()?

That's a really good suggestion. The name of the method will accurately convey what it does.

This method will have to also be changed in the scan package.

stephenafamo avatar Sep 28 '25 18:09 stephenafamo