fix: ensure sorts are always done on columns of the right table

[lukapeschke]

closes #5302

This fixes #5302 with two changes:

1. First, it recomputes cid_redirects after compiling a relation (i.e. a pipeline), because we rely on them in the postprocess step for sorting. This is needed because CTEs get modified during compile_pipeline: some are merged or removed, and some are added
2. Then, rather than reverting sort columns when iterating over every CTE, we do it once when processing the main pipeline: the sort columns are reverted back to their very first form via cid_mappings, and are then reverted "back" to the front, but we prefer using their alias when they are renamed. This allows to pass named columns around rather expressions, and ensure that the sort columns are available in the main pipeline, even with intermediary select steps. An example where this is needed is the new group_sort_filter_derive_select_join integration test: if you remove the aliasing part, we will attempt sorting on a column that has been de-selected.

The two changes feel a bit clunky (especially alias_last_sorting), but I'm not really sure how to fix this without a proper lineage for columns 😕 Something ideal would be an object for every column, with information such as "this column comes from table X /expression Y, becomes FOO in table T, then BAR in table T2". It could avoid having to keep ctes and relation_instances in sync.

@kgutwin @max-sixty thanks a lot for your help on this one, it has made it much easier to dive into the code!

[max-sixty]

this looks great!

(I agree it's a bit clunky, but unless I'm misunderstanding, the result is very good! and we can re-architect one day™)

I'll merge; @kgutwin if you have any feedback please add for a post-review!

[kgutwin]

Glad for the merge!

I agree that proper lineage tracking would be very useful here. In general, the SQL processing code is tough to follow and reason through (hence why @lukapeschke and I had so much trouble fixing these bugs) so any refactoring we could do for modularization or just better data structures would be a really good idea.

Thanks!

[vanillajonathan]

Is this something that should be mentioned in the changelog?

[max-sixty]

Glad for the merge!

I agree that proper lineage tracking would be very useful here. In general, the SQL processing code is tough to follow and reason through (hence why @lukapeschke and I had so much trouble fixing these bugs) so any refactoring we could do for modularization or just better data structures would be a really good idea.

Thanks!

yes, strongly agree...

[lukapeschke]

thanks for the quick merge 🎉