valderman
Cannot order by on several columns
The current behavior of order seems to erase last call to order, which sucks, because we cannot express something like that:
order forename ascending
order lastname ascending
Either make order monoidal – that is, it accumulates the order rules – or change its type so that it takes a list of ordering somewhat.
Danke schön!
This is already the case, but up until a minute a go it was very poorly documented. Later calls to order override earlier calls, but do not erase them. Your example should generate a query along the lines of ... ORDER BY lastname ASC, forename ASC. If it doesn't, that's a bug.
See the brand new docs for order for more information.
Could you post the whole query if it doesn't generate the expected SQL? The problem should be significantly easier to debug that way.
I’m sorry, it’s work code, I can’t :/, and trust me, I’m an Open-Source guy, I feel terrible not being able to provide the code… It’s basically after a leftJoin that I do the sorting on fields coming from both the two tables.
Hm, it might, but I don’t have time for that today. I’ll try to provide you with that tomorrow on my lunch break!
There's a weird semantic issue (I would probably call it a bug) with order and limit in Selda. They really ought to be functions :: Query a -> Query a rather than actions :: Query (). It's possible you're running into something like this @phaazon.
See
- https://www.reddit.com/r/haskell/comments/66ih1l/selda_a_monadic_database_edsl/dgjb39z/
- https://www.reddit.com/r/haskell/comments/66ih1l/selda_a_monadic_database_edsl/dgje93m/
@tomjaguarpaw, you may want to be a bit more clear about the "weird semantics issue" you're seeing, because I'm still not seeing it. Could you explain what the exact surprise is that you're referring to in the first link? I'm open to changing the type of order, but I'd like to have a clearer understanding of what's surprising about the current semantics first, to avoid messing something else up.
As for the second link, limit, in fact, now does have (approximately) the type Query s a -> Query s a.
The limit example provided by tathougies has a much clearer problem, but as you note
limit, in fact, now does have (approximately) the type Query s a -> Query s a.
so limit is not a problem anymore. Maybe my example in the Reddit link is not very good because the problem with order is much less serious. However, it still does have strange semantics. What is the meaning of
do ...
order foo Asc
The meaning is not dependent on this do block alone. It also depends on everything that can come before this do block in a query. Maybe this will never be noticed in practice because SQL ordering is so volatile anyway.
That is indeed true, but I'm not entirely sure how serious a problem it is. For instance, it is no less unpredictable than any non-stable sorting algorithm. If you need your results sorted first on A, then on B and finally on C, you'd always specify that anyway, since you'd otherwise leave any unspecified ordering pretty much undefined, regardless of how order composes.
I agree that, all things equal, it would be better to have a type more like the one you suggest, since that would make the order of the orderings quite explicit. However, I can't come up with a nice type for this manner of composition, at least not off the top of my head.
You'd clearly need to make the inner query a "proper" inner query (in type as well as in name) or the clearer semantics (and possibly the soundness of the generated SQL) go right out the window, and you'd need a separate argument to determine the column to order on. The best I can come up with right now is order :: Order -> (a -> Col (Inner s) b) -> Query (Inner s) a -> Query s (OuterCols a), but I'm not convinced the slightly clearer composition would be worth the added inconvenience and complexity.
I still don't see the difference between the expected behavior and the behavior you're seeing. The query in your example produces the following ordering clause: ORDER BY "occ_id_6" DESC, "foo_id_0" DESC. That is, primarily order on oid, and use ordering on bid' for any rows with the same oid. This is precisely the result I would expect from order bid' descending ; order oid descending.
If this is not the code you're expecting for that query, or if you're for some reason not seeing the same ordering clause, I'm afraid I'll have to request more information in order to do something about it because right now it looks like this is working as intended.
I just tested again, and I have the right behavior. I don’t know what happened. Closing.
Damn, I confirm back that this doesn’t work properly.
EDIT: I’ve found the problem. It’s because of generated id names (the AS id_34_124964). They seem to overlap each others and that ends up in bad cases.
I don’t see how to solve that.
Do the overlapping IDs only appear when ordering on columns, or does it happen when doing something else with them as well? It would be extremely helpful to have a query which triggers this bug, or at least a general idea of what shape of query can be expected to.
Well, I cannot leak that code, I’d love to find a way to reproduce though. I don’t know whether it’s linked, but it’s something like:
bid :*: _ <- select somethings
fooid :*: _ <- flip leftJoin (select foods) $ \(_ :*: bid' :*: _) -> bid' .== bid
order bid descending
order fooid descending
I tried to re-get the bid (fooid :*: bid' :*: _ <- flip leftJoin…). It doesn’t change much.
Are you saying that if you reverse the orders, different IDs are generated? That's absolutely insane and I have no idea why that would happen. ID generation should happen at the leftJoin, and definitely not be affected by anything outside. I'll have to take a closer look at that code; obviously I'm doing something stupid there.
Getting confirmation that nested queries are involved is great though; I suspected as much, but I thought all name generation issues had been ironed out by now.
I dumped the query string (via postgresql logging), and i see several IDs, so I guess there’s a problem. And no, reversing doesn’t actually fix my problem… False alarm.
@phaazon Does the "fix" in aed18b4c7169f979bb13a00f66c8a0db4b6d1630 affect the issue? I've been digging around in the renaming code for inner queries and it seems more or less robust to me, so this is a bit of a long shot.
I’ll try when I get back from holidays, since that was a code at work. That should be pretty simple to test though.
~I've reverted the commit in question for now since it potentially causes other problems, but knowing whether it fixed the naming issue would still be very useful in figuring out the actual fix. Just make sure to test with aed18b4c7169f979bb13a00f66c8a0db4b6d1630 when you get the time, and not with HEAD.~
EDIT: disregard this, HEAD is again the best place to test against after introducing the process-global name supply.
As of f03e6f11e456a504b542c7d06224652e1eb34d26 I've modified the name generation to use a global IORef. A horrible hack, and it makes IDs more annoying to read, but if this problem does indeed stem from name generation this commit will fix it.
EDIT: assuming that you're testing on GHC 8.2+. This hack breaks 8.0 and 7.10 horribly.
If this turns out to fix the problem, we should probably look into removing the hack in favor of a splittable name supply or something else that gives us the same benefits as the hack but with none of the drawbacks.