Avoid order by on sub queries in postgresql reflection

[CaselIT]

I'm fairly sure that the order by we apply to subqueries in the postresql reflection, such as

sqlalchemy/lib/sqlalchemy/dialects/postgresql/base.py

Lines 3651 to 3678 in 9ae645d

	select(
	con_sq.c.conrelid,
	con_sq.c.conname,
	con_sq.c.description,
	pg_catalog.pg_attribute.c.attname,
	)
	.select_from(pg_catalog.pg_attribute)
	.join(
	con_sq,
	sql.and_(
	pg_catalog.pg_attribute.c.attnum == con_sq.c.attnum,
	pg_catalog.pg_attribute.c.attrelid == con_sq.c.conrelid,
	),
	)
	.order_by(con_sq.c.conname, con_sq.c.ord)
	.subquery("attr")
	)
	return (
	select(
	attr_sq.c.conrelid,
	sql.func.array_agg(attr_sq.c.attname).label("cols"),
	attr_sq.c.conname,
	sql.func.min(attr_sq.c.description).label("description"),
	)
	.group_by(attr_sq.c.conrelid, attr_sq.c.conname)
	.order_by(attr_sq.c.conrelid, attr_sq.c.conname)
	)

may not correct, since a db can technically ignore it from what I gather.

I don't think this ever happened in our test, but it's probably better to use an alternative or get confirmation that the query is indeed correct. @zzzeek I can try asking to the postgresql mailing list for help if you don't have a better suggestion.

Since the use case is use array_agg and a group by we investigate ordering in the array_agg construct, using aggregate_order_by.
In this case we would need to make aggregate_order_by generate a cache key, since at the moment it's not able to.

[zzzeek]

it's true that in regular SQL, order by in a subquery is not ideal , order by is for getting the rows back and should be on the outside.

that said, if this subq is being fed into an array_agg(), that might change everything, becuase arrays are ordered and we have to be able to order things, right?

do we have a failing case if the ORDER BY is removed? that would be evidence that it takes effect

[CaselIT]

that said, if this subq is being fed into an array_agg(), that might change everything, becuase arrays are ordered and we have to be able to order things, right?

that's my thinking too, but I would feel better if we can get a confirm on this

do we have a failing case if the ORDER BY is removed? that would be evidence that it takes effect

it seems we do not. removing it does not lead to any fail on my system. I've tested on windows, linux may lead to a different outcome

[CaselIT]

I've asked in the pg-general mailing list at https://www.postgresql.org/message-id/CAN19dycxr2f%2BGtKLR4X3OMepFzRGaA-uM6dNS6EqaNUEQTScww%40mail.gmail.com

let's see what they suggest

[CaselIT]

seems like we should use array_agg(col order by other_col)
https://www.postgresql.org/message-id/2617336.1664058035%40sss.pgh.pa.us

we first need to make aggregate_order_by partecipate in the cache key generation though. will open a separate issue

[sqla-tester]

Federico Caselli has proposed a fix for this issue in the main branch:

Use aggregate order by instead of order by in subquery https://gerrit.sqlalchemy.org/c/sqlalchemy/sqlalchemy/+/4100