dialect/sql/sqlgraph: allow OrderByField without aggregate function on OrderByNeighborTerms/O2M

[crossworth]

This allows for a fix to #3722

client.Card.Query().Order(func(s *sql.Selector) {
	sqlgraph.OrderByNeighborTerms(s,
		sqlgraph.NewStep(
			sqlgraph.From(card.Table, card.FieldID),
			sqlgraph.To(user.Table, user.FieldID),
			sqlgraph.Edge(
				sqlgraph.O2M,
				false,
				card.OwnerTable,
				card.OwnerColumn,
				card.FieldNumber, // <--- This PR adds support for this. 
			),
		),
		sql.OrderByField(card.FieldNumber, sql.OrderDesc()),
	)
}).IDsX(ctx)

Previous the query generated was:

SELECT "cards"."id" FROM "cards" LEFT JOIN (SELECT "cards"."owner_id", "cards"."number" FROM "cards" GROUP BY "cards"."owner_id") AS "t1" ON "cards"."id" = "t1"."owner_id" ORDER BY "t1"."number" DESC NULLS LAST

Generating the error:

ERROR: column "cards.number" must appear in the GROUP BY clause or be used in an aggregate function (SQLSTATE 42803)

With this PR, the query generate is:

SELECT "cards"."id" FROM "cards" LEFT JOIN (SELECT "cards"."owner_id", "cards"."number" FROM "cards" GROUP BY "cards"."owner_id", "cards"."number") AS "t1" ON "cards"."id" = "t1"."owner_id" ORDER BY "t1"."number" DESC NULLS LAST

This PR will not solve the problem when using ByCardField method, it only allows for possible fix using Ent constructors.

[a8m]

Thanks for the contribution, @crossworth. However, a sqlgraph.Edge describes graph edge using SQL relations, and adding card.FieldNumber should not be part of it. The group by info should be extracted from the "order field" option.

Let me know if you need help with this.

[crossworth]

hi @a8m, I'm not sure I understood.

The option calls sqlgraph.OrderByNeighborTerms to build the query:

ent/entc/integration/ent/user/user.go

Line 312 in 8a848a6

sqlgraph.OrderByNeighborTerms(s, newCardStep(), sql.OrderByField(field, opts...))

Ofcourse would be possible to extract the info from the order option, but would that require this change as well, since the invalid query is generated inside OrderByNeighborTerms?

Is there something else that I'm missing?

[crossworth]

PR #3943 fix the #3722 issue and is way smaller and simpler solution.

Closing this one.