semohr commented on code in PR #37396:
URL: https://github.com/apache/superset/pull/37396#discussion_r3195655009
##########
superset-frontend/src/explore/components/DataTableControl/index.tsx:
##########
@@ -315,9 +350,10 @@ export const useTableColumns = (
? colnames
.filter((column: string) => Object.keys(data[0]).includes(column))
.map((key, index) => {
- const colType = coltypes?.[index];
+ const originalIndex = (colnames || []).indexOf(key);
+ const colType = coltypes?.[originalIndex];
+ const colLabel = collabels?.[originalIndex];
const firstValue = data[0][key];
- const headerLabel = columnDisplayNames?.[key] ?? key;
const originalFormattedTimeColumnIndex =
colType === GenericDataType.Temporal
? originalFormattedTimeColumns.indexOf(key)
Review Comment:
I have run into the issue in testing somehow that's why I introduced the
originalIndex. Not sure how I got that issue anymore as this was quite some
time ago.
As the number of columns is pretty low normally I think it makes no real
difference. Adding a map might be a premature optimization which adds
complexity for no measurable benefit.
On my hardware the difference here is within the standard deviation (see
below)
That said I have proposed a version using map in the latest commit. **Please
have a look if we are fine with the proposal**.
---
```js
const COL_SIZE = 100; // small number of colums
const RUNS = 50;
// helpers
function mean(arr) {
return arr.reduce((a, b) => a + b, 0) / arr.length;
}
function stddev(arr) {
const avg = mean(arr);
const variance =
arr.reduce((sum, x) => {
const diff = x - avg;
return sum + diff * diff;
}, 0) / arr.length;
return Math.sqrt(variance);
}
const colnames = Array.from({ length: COL_SIZE }, (_, i) => `col_${i}`);
const lookupKeys = [...colnames].sort(() => Math.random() - 0.5);
const map = new Map(colnames.map((key, i) => [key, i]));
const indexOfTimes = [];
for (let r = 0; r < RUNS; r++) {
const start = performance.now();
lookupKeys.map((key) => colnames.indexOf(key));
indexOfTimes.push(performance.now() - start);
}
const mapTimes = [];
for (let r = 0; r < RUNS; r++) {
const start = performance.now();
lookupKeys.map((key) => map.get(key));
mapTimes.push(performance.now() - start);
}
function report(name, arr) {
console.log(`\n${name}`);
console.log("mean:", mean(arr).toFixed(4), "ms");
console.log("std dev:", stddev(arr).toFixed(4), "ms");
}
report("indexOf", indexOfTimes);
report("Map", mapTimes);
```
--
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]
