luizotavio32 commented on PR #42864: URL: https://github.com/apache/superset/pull/42864#issuecomment-5217446196
Confirmed — I checked each name against the `buildQuery` that consumes it, and all of them do reach a query. Fixed in cb1aea5. Added to the lookup tables: - metrics: `tooltip_metrics`, `timeseries_limit_metric_b`, `point_radius_fixed`, plus `series_limit_metric_b` (Query B's section is generated by `createQuerySection(..., '_b')`, and `retainFormDataSuffix` strips the suffix, so the `_b` twin of every Query A control is live) - columns: `groupby_b`, `source_category`, `target_category`, `dimension`, `line_column`, `js_columns`, `geom_column` The three structural gaps are decomposed the way the frontend does, so they compare against the flat names a request carries: - `spatial`/`start_spatial`/`end_spatial` → the lat/lon/geohash columns they name, mirroring `getSpatialColumns()`. All of `lonCol`/`latCol`/`lonlatCol`/`geohashCol` are collected regardless of declared `type`, since each is a column the chart is saved with. - `tooltip_contents` → the columns its entries wrap, mirroring `extractTooltipColumns()`. Only `item_type: "column"` entries count; metric entries are read from data already fetched and select nothing, so requesting one as a column is still rejected. - `selected_chart` → decoded, its `params` decoded again, and recursed into, so the nested chart's own metrics/group-bys are reachable. A malformed value yields no params rather than raising, and grants nothing. Decomposition only reaches names a control already points at, so nothing opens up: a `fix`-typed `point_radius_fixed` is a radius in pixels and grants no metric. Both of those are covered by tests, along with a tamper case per structural gap. You're right that the description overclaimed; I've reworded it to say the controls are enumerated from what `buildQuery` feeds into a query, and to name the nested ones explicitly rather than implying exhaustiveness. One note on verification, since it bears on how much the green suite is worth here: I confirmed each new legitimate-load test fails against the previous comparator (10 of them did), so they exercise the fix rather than restating it. The tamper tests reject in both versions, as they should. -- 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]
