SEPURI-SAI-KRISHNA opened a new pull request, #43589:
URL: https://github.com/apache/superset/pull/43589
### SUMMARY
`PostProcessingRolling`, `PostProcessingCum` and `PostProcessingDiff`
declare `options.columns` as `string[]`, but all three operations take a
string-to-string mapping. This corrects the exported types to `Record<string,
string>`.
The mapping is what the runtime has always required. Each operation takes
`columns: dict[str, str]` and indexes the frame by its keys:
| operation | signature | use |
| --- | --- | --- |
| `rolling()` | `columns: dict[str, str]` | `df.loc[:, columns.keys()]` |
| `cum()` | `columns: dict[str, str]` | `df[columns.keys()]` |
| `diff()` | `columns: dict[str, str]` | `df[columns.keys()]` |
An array reaching any of them calls `list.keys()` and raises
`AttributeError`.
It is also what the frontend already emits. `rollingWindowOperator` builds a
mapping and sends it for both operations it produces:
```ts
const columnsMap = Object.fromEntries(columns.map(col => [col, col]));
```
and the existing tests pin that object shape,
`rollingWindowOperator.test.ts` and `plugin-chart-echarts`'s
`MixedTimeseries/buildQuery.test.ts` both assert `columns: { 'count(*)':
'count(*)', ... }`. The only `string[]` in the tree was the fixture block in
`PostProcessing.test.ts`, which used arrays because the type asked for them;
those are updated here.
**Why this never broke the build.** `columns.map(col => [col, col])` widens
each pair to `(string | undefined)[]` instead of a tuple, so overload
resolution picks the untyped `Object.fromEntries`:
```ts
fromEntries<T = any>(entries: Iterable<readonly [PropertyKey, T]>): { [k:
string]: T };
fromEntries(entries: Iterable<readonly any[]>): any; // <- selected
```
`columnsMap` is therefore `any`, which assigns to `string[]` without
complaint. Annotating the tuple explicitly surfaces the underlying conflict:
```
error TS2322: Type 'string[]' is not assignable to type 'Record<string,
string>'.
Index signature for type 'string' is missing in type 'string[]'.
```
That is the error the corrected types now raise if an array is reintroduced.
Follow-up to the discussion on #43225, which restored `rolling.columns` to
the published OpenAPI spec and made the divergence visible to typed clients.
That PR is Python-only; this is the frontend half.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Not applicable, type declarations only.
### TESTING INSTRUCTIONS
No runtime behaviour changes: the emitted payloads are unchanged, and the
`isPostProcessing*` guards only inspect `.operation`, so their results are
unaffected.
```bash
cd superset-frontend
npm run type
npm run test --
packages/superset-ui-core/test/query/types/PostProcessing.test.ts
npm run test --
packages/superset-ui-chart-controls/test/operators/rollingWindowOperator.test.ts
```
To confirm the corrected types actually constrain, change any `columns`
value in `PostProcessing.test.ts` back to an array and re-run `npm run type`;
it fails with the `TS2322` above.
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in
[SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
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]
