You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "DamianPendrak (via GitHub)" <gi...@apache.org> on 2026/06/20 09:45:46 UTC
[PR] fix(deck.gl): render all rings of a MultiPolygon in Polygon layer [superset]
DamianPendrak opened a new pull request, #41262:
URL: https://github.com/apache/superset/pull/41262
<!---
Please write the PR title following the conventions at https://www.conventionalcommits.org/en/v1.0.0/
Example:
fix(dashboard): load charts correctly
-->
Fixes: #32127
### SUMMARY
Expand each record into one feature per polygon ring so MultiPolygon geometries display fully instead of only the first polygon. Skip records whose geohash fails to decode instead of emitting an empty polygon.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
<!--- Skip this if not applicable -->
Before:
<img width="1485" height="998" alt="Screenshot 2026-06-20 at 11 15 39" src="https://github.com/user-attachments/assets/f6289e8a-96e3-439c-8c65-eaee3e58b8ef" />
After:
<img width="1486" height="1001" alt="Screenshot 2026-06-20 at 11 13 58" src="https://github.com/user-attachments/assets/ee17f81a-23a0-4f2d-a1b9-6935228a584e" />
### TESTING INSTRUCTIONS
<!--- Required! What steps can be taken to manually verify the changes? -->
sql for testing and instructions in #32127
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [x] Has associated issue: #32127
- [ ] 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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(deck.gl): render all rings of a MultiPolygon in Polygon layer [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #41262:
URL: https://github.com/apache/superset/pull/41262#issuecomment-4757213397
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(deck.gl): render all rings of a MultiPolygon in Polygon layer [superset]
Posted by "DamianPendrak (via GitHub)" <gi...@apache.org>.
DamianPendrak closed pull request #41262: fix(deck.gl): render all rings of a MultiPolygon in Polygon layer
URL: https://github.com/apache/superset/pull/41262
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(deck.gl): render all rings of a MultiPolygon in Polygon layer [superset]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #41262:
URL: https://github.com/apache/superset/pull/41262#discussion_r3445847271
##########
superset-frontend/plugins/preset-chart-deckgl/src/layers/Polygon/transformProps.ts:
##########
@@ -83,98 +83,92 @@ function processPolygonData(
const excludeKeys = new Set([line_column, ...(js_columns || [])]);
- return records
- .map(record => {
- let feature: PolygonFeature = {
- extraProps: {},
- metrics: {},
- };
-
- feature = addJsColumnsToExtraProps(feature, record, js_columns);
- const updatedFeature = addPropertiesToFeature(
- feature as unknown as Record<string, unknown>,
- record,
- excludeKeys,
- );
- feature = updatedFeature as unknown as PolygonFeature;
-
- const rawPolygonData = record[line_column];
- if (!rawPolygonData) {
- return null;
- }
+ return records.flatMap(record => {
+ let feature: PolygonFeature = {
+ extraProps: {},
+ metrics: {},
+ };
Review Comment:
`addPropertiesToFeature` can copy a record field named `extraProps`/`metrics` onto the feature and overwrite the internal objects you rely on (e.g. JS columns stored in `extraProps`). This can also lead to runtime errors later when the code assumes these fields are objects. Consider excluding these reserved keys from `addPropertiesToFeature` the same way other DeckGL layers exclude internal fields.
##########
superset-frontend/plugins/preset-chart-deckgl/src/layers/Polygon/transformProps.ts:
##########
@@ -83,98 +83,92 @@ function processPolygonData(
const excludeKeys = new Set([line_column, ...(js_columns || [])]);
- return records
- .map(record => {
- let feature: PolygonFeature = {
- extraProps: {},
- metrics: {},
- };
-
- feature = addJsColumnsToExtraProps(feature, record, js_columns);
- const updatedFeature = addPropertiesToFeature(
- feature as unknown as Record<string, unknown>,
- record,
- excludeKeys,
- );
- feature = updatedFeature as unknown as PolygonFeature;
-
- const rawPolygonData = record[line_column];
- if (!rawPolygonData) {
- return null;
- }
+ return records.flatMap(record => {
+ let feature: PolygonFeature = {
+ extraProps: {},
+ metrics: {},
+ };
- try {
- let polygonCoords: number[][];
-
- switch (line_type) {
- case 'json': {
- const parsed =
- typeof rawPolygonData === 'string'
- ? JSON.parse(rawPolygonData)
- : rawPolygonData;
-
- if (parsed.coordinates) {
- polygonCoords = parsed.coordinates[0] || parsed.coordinates;
- } else if (parsed.geometry?.coordinates) {
- // Non-standard format with nested geometry
- polygonCoords =
- parsed.geometry.coordinates[0] || parsed.geometry.coordinates;
- } else if (Array.isArray(parsed)) {
- polygonCoords = parsed;
- } else {
- return null;
- }
- break;
- }
- case 'geohash':
- polygonCoords = [];
- const decoded = decode_bbox(String(rawPolygonData));
- if (decoded) {
- polygonCoords.push([decoded[1], decoded[0]]); // SW (minLon, minLat)
- polygonCoords.push([decoded[1], decoded[2]]); // NW (minLon, maxLat)
- polygonCoords.push([decoded[3], decoded[2]]); // NE (maxLon, maxLat)
- polygonCoords.push([decoded[3], decoded[0]]); // SE (maxLon, minLat)
- polygonCoords.push([decoded[1], decoded[0]]); // SW (close polygon)
- }
- break;
- case 'zipcode':
- default: {
- polygonCoords = Array.isArray(rawPolygonData) ? rawPolygonData : [];
- break;
- }
- }
+ feature = addJsColumnsToExtraProps(feature, record, js_columns);
+ feature = addPropertiesToFeature(feature, record, excludeKeys);
- if (reverse_long_lat && polygonCoords.length > 0) {
- polygonCoords = polygonCoords.map(coord => [coord[1], coord[0]]);
- }
-
- feature.polygon = polygonCoords;
+ const rawPolygonData = record[line_column];
+ if (!rawPolygonData) {
+ return [];
+ }
- if (fixedElevationValue !== undefined) {
- feature.elevation = fixedElevationValue;
- } else if (elevationLabel && record[elevationLabel] != null) {
- const elevationValue = parseMetricValue(record[elevationLabel]);
- if (elevationValue !== undefined) {
- feature.elevation = elevationValue;
+ try {
+ // One ring per polygon; a MultiPolygon yields several rings, all other
+ // shapes yield exactly one.
+ let rings: number[][][];
+
+ switch (line_type) {
+ case 'json': {
+ const parsed =
+ typeof rawPolygonData === 'string'
+ ? JSON.parse(rawPolygonData)
+ : rawPolygonData;
+ // Unwrap GeoJSON Feature ({ geometry: { type, coordinates } })
+ const geom = parsed.geometry ?? parsed;
+
+ if (geom.type === 'MultiPolygon') {
+ // Only the outer ring of each polygon is used; inner rings (holes) are
+ // intentionally ignored because the downstream layer does not support them.
+ rings = geom.coordinates.map((poly: number[][][]) => poly[0]);
+ } else if (geom.coordinates) {
+ // coordinates[0] is the outer ring for Polygon; holes are not rendered.
+ rings = [geom.coordinates[0] || geom.coordinates];
+ } else if (Array.isArray(geom)) {
+ rings = [geom];
+ } else {
+ return [];
}
+ break;
}
-
- if (metricLabel && record[metricLabel] != null) {
- const metricValue = record[metricLabel];
- if (
- typeof metricValue === 'string' ||
- typeof metricValue === 'number'
- ) {
- feature.metrics![metricLabel] = metricValue;
+ case 'geohash': {
+ const decoded = decode_bbox(String(rawPolygonData));
+ if (!decoded) {
+ return [];
}
+ rings = [
+ [
+ [decoded[1], decoded[0]], // SW
+ [decoded[1], decoded[2]], // NW
+ [decoded[3], decoded[2]], // NE
+ [decoded[3], decoded[0]], // SE
+ [decoded[1], decoded[0]], // close
+ ],
+ ];
+ break;
}
- } catch {
- return null;
+ case 'zipcode':
+ default:
+ rings = [Array.isArray(rawPolygonData) ? rawPolygonData : []];
+ }
+
+ let elevation: number | undefined;
+ if (fixedElevationValue !== undefined) {
+ elevation = fixedElevationValue;
+ } else if (elevationLabel && record[elevationLabel] != null) {
+ elevation = parseMetricValue(record[elevationLabel]);
}
- return feature;
- })
- .filter((feature): feature is PolygonFeature => feature !== null);
+ const metrics = { ...feature.metrics };
+ const metricValue = metricLabel ? record[metricLabel] : undefined;
+ if (typeof metricValue === 'string' || typeof metricValue === 'number') {
+ metrics[metricLabel!] = metricValue;
+ }
Review Comment:
`const metrics = { ...feature.metrics };` will throw if `feature.metrics` is ever `null`/`undefined` (e.g. if a dataset has a `metrics` column that overwrote the internal field). Since this spread is unconditional, it can crash even when no metric is configured. Use a null-safe default and avoid the non-null assertion on `metricLabel`.
##########
superset-frontend/plugins/preset-chart-deckgl/src/layers/Polygon/transformProps.ts:
##########
@@ -83,98 +83,92 @@ function processPolygonData(
const excludeKeys = new Set([line_column, ...(js_columns || [])]);
- return records
- .map(record => {
- let feature: PolygonFeature = {
- extraProps: {},
- metrics: {},
- };
-
- feature = addJsColumnsToExtraProps(feature, record, js_columns);
- const updatedFeature = addPropertiesToFeature(
- feature as unknown as Record<string, unknown>,
- record,
- excludeKeys,
- );
- feature = updatedFeature as unknown as PolygonFeature;
-
- const rawPolygonData = record[line_column];
- if (!rawPolygonData) {
- return null;
- }
+ return records.flatMap(record => {
+ let feature: PolygonFeature = {
+ extraProps: {},
+ metrics: {},
+ };
- try {
- let polygonCoords: number[][];
-
- switch (line_type) {
- case 'json': {
- const parsed =
- typeof rawPolygonData === 'string'
- ? JSON.parse(rawPolygonData)
- : rawPolygonData;
-
- if (parsed.coordinates) {
- polygonCoords = parsed.coordinates[0] || parsed.coordinates;
- } else if (parsed.geometry?.coordinates) {
- // Non-standard format with nested geometry
- polygonCoords =
- parsed.geometry.coordinates[0] || parsed.geometry.coordinates;
- } else if (Array.isArray(parsed)) {
- polygonCoords = parsed;
- } else {
- return null;
- }
- break;
- }
- case 'geohash':
- polygonCoords = [];
- const decoded = decode_bbox(String(rawPolygonData));
- if (decoded) {
- polygonCoords.push([decoded[1], decoded[0]]); // SW (minLon, minLat)
- polygonCoords.push([decoded[1], decoded[2]]); // NW (minLon, maxLat)
- polygonCoords.push([decoded[3], decoded[2]]); // NE (maxLon, maxLat)
- polygonCoords.push([decoded[3], decoded[0]]); // SE (maxLon, minLat)
- polygonCoords.push([decoded[1], decoded[0]]); // SW (close polygon)
- }
- break;
- case 'zipcode':
- default: {
- polygonCoords = Array.isArray(rawPolygonData) ? rawPolygonData : [];
- break;
- }
- }
+ feature = addJsColumnsToExtraProps(feature, record, js_columns);
+ feature = addPropertiesToFeature(feature, record, excludeKeys);
- if (reverse_long_lat && polygonCoords.length > 0) {
- polygonCoords = polygonCoords.map(coord => [coord[1], coord[0]]);
- }
-
- feature.polygon = polygonCoords;
+ const rawPolygonData = record[line_column];
+ if (!rawPolygonData) {
+ return [];
+ }
- if (fixedElevationValue !== undefined) {
- feature.elevation = fixedElevationValue;
- } else if (elevationLabel && record[elevationLabel] != null) {
- const elevationValue = parseMetricValue(record[elevationLabel]);
- if (elevationValue !== undefined) {
- feature.elevation = elevationValue;
+ try {
+ // One ring per polygon; a MultiPolygon yields several rings, all other
+ // shapes yield exactly one.
+ let rings: number[][][];
+
+ switch (line_type) {
+ case 'json': {
+ const parsed =
+ typeof rawPolygonData === 'string'
+ ? JSON.parse(rawPolygonData)
+ : rawPolygonData;
+ // Unwrap GeoJSON Feature ({ geometry: { type, coordinates } })
+ const geom = parsed.geometry ?? parsed;
+
+ if (geom.type === 'MultiPolygon') {
+ // Only the outer ring of each polygon is used; inner rings (holes) are
+ // intentionally ignored because the downstream layer does not support them.
+ rings = geom.coordinates.map((poly: number[][][]) => poly[0]);
+ } else if (geom.coordinates) {
Review Comment:
In the `MultiPolygon` branch, `poly[0]` can be `undefined` for malformed/empty polygons. That would cause `ring.map(...)` to throw and the whole record to be dropped by the catch, even if other polygons in the multipolygon are valid. Filtering out missing rings avoids losing all polygons for a partially-invalid geometry.
##########
superset-frontend/plugins/preset-chart-deckgl/src/layers/Polygon/transformProps.test.ts:
##########
@@ -409,6 +409,90 @@ describe('Polygon transformProps', () => {
]);
});
+ test('should explode a MultiPolygon into one feature per polygon', () => {
+ const multiPolygonProps = {
+ ...mockChartProps,
+ queriesData: [
+ {
+ data: [
+ {
+ geom: JSON.stringify({
+ type: 'MultiPolygon',
+ coordinates: [
+ [
+ [
+ [-122.4, 37.8],
+ [-122.3, 37.8],
+ [-122.3, 37.9],
+ [-122.4, 37.8],
+ ],
+ ],
+ [
+ [
+ [-121.4, 36.8],
+ [-121.3, 36.8],
+ [-121.3, 36.9],
+ [-121.4, 36.8],
+ ],
+ ],
+ ],
+ }),
+ population: 50000,
+ },
+ ],
+ },
+ ],
+ };
+
+ const result = transformProps(multiPolygonProps as ChartProps);
+
+ const features = result.payload.data.features as PolygonFeature[];
+ expect(features).toHaveLength(2);
+ expect(features[0]?.polygon).toEqual([
+ [-122.4, 37.8],
+ [-122.3, 37.8],
+ [-122.3, 37.9],
+ [-122.4, 37.8],
+ ]);
+ expect(features[1]?.polygon).toEqual([
+ [-121.4, 36.8],
+ [-121.3, 36.8],
+ [-121.3, 36.9],
+ [-121.4, 36.8],
+ ]);
+ });
+
+ test('should explode a MultiPolygon wrapped in a GeoJSON Feature', () => {
+ const featureProps = {
+ ...mockChartProps,
+ queriesData: [
+ {
+ data: [
+ {
+ geom: JSON.stringify({
+ type: 'Feature',
+ geometry: {
+ type: 'MultiPolygon',
+ coordinates: [
+ [[[0, 0], [1, 0], [1, 1], [0, 0]]],
+ [[[2, 2], [3, 2], [3, 3], [2, 2]]],
+ ],
+ },
+ }),
+ },
+ ],
+ },
+ ],
+ };
+
+ const result = transformProps(featureProps as ChartProps);
+
+ const features = result.payload.data.features as PolygonFeature[];
+ expect(features).toHaveLength(2);
+ expect(features[0]?.polygon).toEqual([[0, 0], [1, 0], [1, 1], [0, 0]]);
+ expect(features[1]?.polygon).toEqual([[2, 2], [3, 2], [3, 3], [2, 2]]);
+ });
+
test('should handle geohash decoding successfully', () => {
Review Comment:
The new behavior to *skip* records whose geohash fails to decode isn't covered by tests. Adding a case with a mix of valid + invalid geohashes would guard against regressions (and ensure invalid rows don't break rendering for valid ones).
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(deck.gl): render all rings of a MultiPolygon in Polygon layer [superset]
Posted by "netlify[bot] (via GitHub)" <gi...@apache.org>.
netlify[bot] commented on PR #41262:
URL: https://github.com/apache/superset/pull/41262#issuecomment-4757219846
### <span aria-hidden="true">✅</span> Deploy Preview for *superset-docs-preview* ready!
| Name | Link |
|:-:|------------------------|
|<span aria-hidden="true">🔨</span> Latest commit | 584d5c1577122f9a88bcebd544b93898c3fd153c |
|<span aria-hidden="true">🔍</span> Latest deploy log | https://app.netlify.com/projects/superset-docs-preview/deploys/6a36614a797b8d00089f3815 |
|<span aria-hidden="true">😎</span> Deploy Preview | [https://deploy-preview-41262--superset-docs-preview.netlify.app](https://deploy-preview-41262--superset-docs-preview.netlify.app) |
|<span aria-hidden="true">📱</span> Preview on mobile | <details><summary> Toggle QR Code... </summary><br /><br /><br /><br />_Use your smartphone camera to open QR code link._</details> |
|<span aria-hidden="true">🤖</span> Make changes | [Run an agent on this branch](https://app.netlify.com/projects/superset-docs-preview/agent-runs#q1YqKMrPLShRslJS0lFKKkrMS84AstMyK_RTUpOz03N0C_JzKtPz83RzS3NKMqEcoNKU1IKc_ErPFKBis0RjMzNDk0RzS_MkixQDAwMLyzRjC0NTpVoA) |
---
<!-- [superset-docs-preview Preview](https://deploy-preview-41262--superset-docs-preview.netlify.app) -->
_To edit notification comments on pull requests, go to your [Netlify project configuration](https://app.netlify.com/projects/superset-docs-preview/configuration/notifications#deploy-notifications)._
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org