[Allex-Nik]

Fixes #780

Currently, if a user tries to sort rows within groups of a GroupBy using the sortBy or sortByDesc functions, and this column is not found in a group, the column is skipped and both functions finish silently without sorting anything and without throwing any exception.
The proposed changes make this functions throw an IllegalStateException with a clear report when a column the user wants to sort by is not found in a group.

Preparatory work was done in #2176

[jetbrains-air]

Thanks for tackling this one — two things are worth a fix before merge, both noted inline.

Produced by Air Automations. Name: AI Review / Run: https://air.jetbrains.cloud/org/05cf1a7f-6ab5-713b-abd3-29d0c8a05e2d/automations/8daf2412-0b6c-4433-90c0-b40ab5aebcdb?run=c1b37ce6-75b9-4a47-b73c-be32d1726806

[jetbrains-air]

Thanks for the quick turnaround — one more thing to fix before merge, noted inline.

Produced by Air Automations. Name: AI Review / Run: https://air.jetbrains.cloud/org/05cf1a7f-6ab5-713b-abd3-29d0c8a05e2d/automations/8daf2412-0b6c-4433-90c0-b40ab5aebcdb?run=4d940783-869f-41a8-932d-dc688c139fe3

[jetbrains-air]

Looks good — approving; one non-blocking note inline for whenever you get to it.

Produced by Air Automations. Name: AI Review / Run: https://air.jetbrains.cloud/org/05cf1a7f-6ab5-713b-abd3-29d0c8a05e2d/automations/8daf2412-0b6c-4433-90c0-b40ab5aebcdb?run=9b5f720c-f4ca-4a3a-a086-c99a334771c2

[jetbrains-air]

The columns.isNotEmpty() half of this guard silently turns the new validation off whenever the group schema was lost, and nothing here or in the tests says so.

Failure scenario: df.groupBy { … }.filter { false }.sortBy("typo") returns normally (verified on this branch), while df.filter { false }.groupBy { … }.sortBy("typo") throws. GroupByImpl.filter rebuilds the frame column via df[indices].asGroupBy(groups), which drops FrameColumnImpl.columnSchema and recomputes the schema as empty, so the pre-check is skipped. No wrong data results (the GroupBy is empty either way), but the missing-column diagnostic #780 asks for is absent on that path — and the neighbouring grouped sort accepts valid selections after all groups are filtered out test only pins the happy path, so the suite doesn't record the gap.

Suggested fix: carry the original schema through when GroupByImpl.filter re-wraps the frame column; if that's out of scope here, say in a comment on this condition why validation is skipped and add a test pinning the current no-throw behaviour, so the next reader doesn't "simplify" the condition away or assume coverage is complete.

[Allex-Nik]

This will be fixed by #2154 (FrameColumnImpl)

[Jolanrensen]

Also, we have groups.isEmpty if you want it :)

[Jolanrensen]

I know our exceptions are a mess, but what do you think of IlegalArgumentException? Since the provided 'argument', namely the column, doesn't exist. On the other hand, you could say it's a "valid argument" in the function but resolving it breaks and thus we reach an illegal state... something can be said for both

[Allex-Nik]

@Jolanrensen I agree that IlegalArgumentException suits better. But df.sortBy("wrongCol") (and some other functions) also throws IllegalStateException because of the way the check function in DataFrameReceiver.kt is implemented:

UnresolvedColumnsPolicy.Fail -> error(formatMissingColumnMessage(path))

Changing the type of exception here changes it for some other functions as well (from tests that break after this change I can see allColsExcept, colGroup, col, frameCol, valueCol, groupBy.aggregate).

So I was thinking that it might make sense to consider it as a possible separate change in another PR. What do you think?