villebro commented on pull request #13434: URL: https://github.com/apache/superset/pull/13434#issuecomment-791730928
> I was actually contemplating sending the `orderby` only columns back to client (at least in the Data panel), just so users can have confidence the order by was correctly applied. Currently I'm removing the `orderby` columns with `labels_expected`, but they could also be removed in `query_actions.py`, or be handled by each chart itself. Either way, it seems useful to have the engine always return the sort by field while it's not clear how much the bandwidth saving matters, so I'm inclined to not add this `EngineSpec` parameter just to keep things simple... I'm not super passionate about adding distinct support, so if there's any risk of it causing added complexity or getting in the way I'm ok with just supporting group bys. Same goes for adding vs removing order bys from the select - I'm fine adding them in the select, and agree that it's good to have them around for validation purposes (the real optimizations are probably elsewhere anyways, like using binary protocols, streaming etc). But in general I'm in the camp of "if a feature can easily be added, why not", and tend to prefer to avoid adding limitations or forcing features if there are no clear motivations for doing so. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
