mikebridge commented on code in PR #44806:
URL: https://github.com/apache/superset/pull/44806#discussion_r4191499104
##########
superset-frontend/plugins/plugin-chart-table/src/buildQuery.ts:
##########
@@ -270,14 +269,24 @@ export const buildQuery: BuildQuery<TableChartFormData> =
(
sortByFromOwnState = [[sortByItem?.key, !sortByItem?.desc]];
}
+ const requestedOrderby =
+ formData.server_pagination && sortByFromOwnState
+ ? sortByFromOwnState
+ : orderby;
+ const selectedColumns = new Set(columns.filter(isPhysicalColumn));
+ const effectiveOrderby =
+ queryMode === QueryMode.Raw && isSemanticView
Review Comment:
Addressed in cd594b6d3e. Semantic Aggregate now clears stale Raw ordering
when there is no ordinary metric, including groupby-only and percent-only
queries. I also covered server-pagination header sorts, which could otherwise
reintroduce the stale field, while preserving valid groupby, metric and
temporal sorts. The new regression tests failed before the fix and pass after.
##########
superset-frontend/plugins/plugin-chart-table/test/buildQuery.test.ts:
##########
@@ -116,6 +116,19 @@ test.each(['2__semantic_view', '11__table'])(
},
);
+test('semantic raw table drops ordering for a column no longer selected', ()
=> {
+ const query = buildQueryUncached({
+ ...basicFormData,
+ datasource: '2__semantic_view',
+ query_mode: QueryMode.Raw,
+ all_columns: ['played_at', 'song_name'],
+ order_by_cols: ['["artist_name",false]', '["played_at",false]'],
+ }).queries[0];
+
+ expect(query.columns).toEqual(['played_at', 'song_name']);
+ expect(query.orderby).toEqual([['played_at', false]]);
+});
Review Comment:
Addressed in cd594b6d3e. The new parameterized test pins dataset Raw sorting
and semantic Aggregate metric sorting. Broadening the guard to all Raw queries
now fails the dataset case, and broadening it to all semantic queries fails the
explicit Aggregate sort-by-metric case.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
