fix(data): detect sparse object keys. close #11322#21699
fix(data): detect sparse object keys. close #11322#21699rahul37wallst-sudo wants to merge 1 commit into
Conversation
|
Thanks for your contribution! Please DO NOT commit the files in dist, i18n, and ssr/client/dist folders in a non-release pull request. These folders are for release use only. |
| const dimensionNameMap = createHashMap<true, DimensionName>(); | ||
| const dimensionsDefine: DimensionDefinitionLoose[] = []; | ||
|
|
||
| for (let i = 0; i < data.length; i++) { | ||
| const obj = data[i]; | ||
| if (!obj) { | ||
| continue; | ||
| } | ||
| each(keys(obj), function (name) { | ||
| if (!dimensionNameMap.get(name)) { | ||
| dimensionNameMap.set(name, true); | ||
| dimensionsDefine.push(name); | ||
| } | ||
| }); | ||
| } |
There was a problem hiding this comment.
Thanks for fixing this issue. I agree that collecting keys from all object rows is necessary to handle sparse fields correctly, but it also adds an O(total number of object keys) pre-scan during source initialization when dimensions is not explicitly provided.
Could you please add a small performance check to verify that this does not introduce a noticeable overhead for large datasets? It would be useful to compare:
- the previous behavior (reading only the first non-empty object)
- the new full-scan behavior
- the path where
dataset.dimensionsis explicitly provided
For example, testing with around 100k rows would be helpful, covering both stable object shapes and sparse keys that appear only in later rows. Measuring setOption or source creation time should be sufficient. This would help us assess the practical impact of the additional scan.
Also, the current implementation creates a keys(obj) array and uses a callback for every row. If this shows up in the benchmark, could we avoid the intermediate allocation and callback overhead with something like:
for (const name in obj) {
if (hasOwn(obj, name) && !dimensionNameMap.get(name)) {
dimensionNameMap.set(name, true);
dimensionsDefine.push(name);
}
}This preserves the intended semantics, collecting every field while retaining first-seen order, while potentially reducing allocations and per-row overhead.
Brief Information
This pull request is in the type of:
What does this PR do?
Collects dimensions from all object-row dataset entries so sparse fields referenced by
series.encodemap to the correct data.Fixed issues
Details
Before: What was the problem?
ECharts detected object-row dataset dimensions using only the first non-empty object.
For data such as:
the
GSWYdimension was not detected because it was absent from the first row.Consequently, a bar series configured with:
could silently map its y-axis data to another detected dimension, such as
ECM, and render incorrect values.After: How does it behave after the fixing?
Object-row dimension detection now collects each unique property across all rows while preserving first-seen order.
As a result,
GSWYis registered as a dataset dimension even when it is absent from earlier rows. The corresponding series correctly maps its y-axis data toGSWY; rows without that property remain empty instead of using values from another dimension.Regression tests cover both source dimension detection and the stacked bar chart behavior from the issue.
Document Info
One of the following should be checked.
Misc
Security Checking
ZRender Changes
Related test cases or examples to use the new APIs
Regression coverage was added in:
test/ut/spec/data/SeriesData.test.tstest/ut/spec/series/bar.test.tsMerging options
Other information
Validated with: