-
Notifications
You must be signed in to change notification settings - Fork 828
Fixed an issue where Geometry Viewer was showing stale data and not a… #9644
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
b4fee8d
a6b77a8
1829dee
aae18b2
bc672e2
217771f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,7 +6,7 @@ | |
| // This software is released under the PostgreSQL Licence | ||
| // | ||
| ////////////////////////////////////////////////////////////// | ||
| import React, { useEffect, useRef } from 'react'; | ||
| import React, { useEffect, useRef, useMemo } from 'react'; | ||
| import { styled } from '@mui/material/styles'; | ||
| import ReactDOMServer from 'react-dom/server'; | ||
| import _ from 'lodash'; | ||
|
|
@@ -22,6 +22,7 @@ import { PANELS } from '../QueryToolConstants'; | |
| import { QueryToolContext } from '../QueryToolComponent'; | ||
|
|
||
| const StyledBox = styled(Box)(({theme}) => ({ | ||
| position: 'relative', | ||
| '& .GeometryViewer-mapContainer': { | ||
| backgroundColor: theme.palette.background.default, | ||
| height: '100%', | ||
|
|
@@ -49,6 +50,8 @@ const StyledBox = styled(Box)(({theme}) => ({ | |
| }, | ||
| })); | ||
|
|
||
| const PK_COLUMN_NAMES = ['id', 'oid']; | ||
|
|
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why use hardcoded PK_COLUMN_NAMES instead of queryData.primary_keys?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here PK_COLUMN_NAMES variable does not mean it will will hold the Primary key, this is used for reference purpose in data output tab's visible columns which have unique values in id or oid columns.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you should be using clientPk instead. |
||
| function parseEwkbData(rows, column) { | ||
| let key = column.key; | ||
| const maxRenderByteLength = 20 * 1024 * 1024; //render geometry data up to 20MB | ||
|
|
@@ -191,6 +194,42 @@ function parseData(rows, columns, column) { | |
| }; | ||
| } | ||
|
|
||
| // Find primary key column i.e a column with unique values from columns array in Data Output tab | ||
| function findPkColumn(columns) { | ||
| return columns.find(c => PK_COLUMN_NAMES.includes(c.name)); | ||
| } | ||
|
|
||
| // Hash function for row objects | ||
| function hashRow(row) { | ||
| const str = Object.keys(row).sort().map(k => `${k}:${row[k]}`).join('|'); | ||
| let hash = 0; | ||
| for (let i = 0; i < str.length; i++) { | ||
| const char = str.charCodeAt(i); | ||
| hash = ((hash << 5) - hash) + char; | ||
| hash = hash & hash; | ||
| } | ||
| return `hash_${hash}`; | ||
| } | ||
|
|
||
| // Get unique row identifier using PK column or first column | ||
| function getRowIdentifier(row, pkColumn, columns) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. there is no need for this.. we already have a unique identifier - clientPK in result set |
||
| if (pkColumn?.key && row[pkColumn.key] !== undefined) { | ||
| return row[pkColumn.key]; | ||
| } | ||
| const firstKey = columns[0]?.key; | ||
| if (firstKey && row[firstKey] !== undefined) { | ||
| return row[firstKey]; | ||
| } | ||
| return hashRow(row); | ||
| } | ||
|
|
||
| // Match rows from previous selection to current rows | ||
| function matchRowSelection(prevIdentifiers, currentRows, pkColumn, columns) { | ||
| if (prevIdentifiers.size === 0) return []; | ||
|
|
||
| return currentRows.filter(row => prevIdentifiers.has(getRowIdentifier(row, pkColumn, columns))); | ||
| } | ||
|
|
||
| function PopupTable({data}) { | ||
|
|
||
| return ( | ||
|
|
@@ -285,20 +324,10 @@ GeoJsonLayer.propTypes = { | |
|
|
||
| function TheMap({data}) { | ||
| const mapObj = useMap(); | ||
| const infoControl = useRef(null); | ||
| const resetLayersKey = useRef(0); | ||
| const zoomControlWithHome = useRef(null); | ||
| const homeCoordinates = useRef(null); | ||
| useEffect(()=>{ | ||
| infoControl.current = Leaflet.control({position: 'topright'}); | ||
| infoControl.current.onAdd = function () { | ||
| let ele = Leaflet.DomUtil.create('div', 'geometry-viewer-info-control'); | ||
| ele.innerHTML = data.infoList.join('<br />'); | ||
| return ele; | ||
| }; | ||
| if(data.infoList.length > 0) { | ||
| infoControl.current.addTo(mapObj); | ||
| } | ||
| resetLayersKey.current++; | ||
|
|
||
| zoomControlWithHome.current = Leaflet.control.zoom({ | ||
|
|
@@ -348,7 +377,6 @@ function TheMap({data}) { | |
| zoomControlWithHome.current.addTo(mapObj); | ||
|
|
||
| return ()=>{ | ||
| infoControl.current?.remove(); | ||
| zoomControlWithHome.current?.remove(); | ||
| }; | ||
| }, [data]); | ||
|
|
@@ -359,6 +387,25 @@ function TheMap({data}) { | |
|
|
||
| return ( | ||
| <> | ||
| {data.infoList.length > 0 && ( | ||
| <div style={{ | ||
| position: 'absolute', | ||
| top: '50%', | ||
| left: '50%', | ||
| transform: 'translate(-50%, -50%)', | ||
| zIndex: 1000, | ||
| maxWidth: '80%', | ||
| textAlign: 'center', | ||
| whiteSpace: 'normal', | ||
| wordWrap: 'break-word', | ||
| fontSize: '16px', | ||
| pointerEvents: 'none', | ||
| }}> | ||
| {data.infoList.map((info, idx) => ( | ||
| <div key={idx}>{info}</div> | ||
| ))} | ||
| </div> | ||
| )} | ||
| {data.selectedSRID === 4326 && | ||
| <LayersControl position="topright"> | ||
| <LayersControl.BaseLayer checked name={gettext('Empty')}> | ||
|
|
@@ -436,25 +483,125 @@ export function GeometryViewer({rows, columns, column}) { | |
|
|
||
| const mapRef = React.useRef(); | ||
| const contentRef = React.useRef(); | ||
| const data = parseData(rows, columns, column); | ||
| const queryToolCtx = React.useContext(QueryToolContext); | ||
|
|
||
| // Track previous column state AND selected row data | ||
| const prevStateRef = React.useRef({ | ||
| columnKey: null, | ||
| columnNames: null, | ||
| selectedRowIdentifiers: new Set(), | ||
| }); | ||
|
|
||
| const [mapKey, setMapKey] = React.useState(0); | ||
| const currentColumnKey = useMemo(() => column?.key, [column]); | ||
| const currentColumnNames = React.useMemo( | ||
| () => columns.map(c => c.key).sort().join(','), | ||
| [columns] | ||
| ); | ||
|
|
||
| const pkColumn = useMemo(() => findPkColumn(columns), [columns]); | ||
|
|
||
| // Detect when to clear, filter, or re-render the map based on changes in geometry column, columns list, or rows | ||
| useEffect(() => { | ||
| const prevState = prevStateRef.current; | ||
|
|
||
| if (!currentColumnKey) { | ||
| setMapKey(prev => prev + 1); | ||
| prevStateRef.current = { | ||
| columnKey: null, | ||
| columnNames: null, | ||
| selectedRowIdentifiers: new Set(), | ||
| }; | ||
| return; | ||
| } | ||
|
|
||
| if (currentColumnKey !== prevState.columnKey || | ||
| currentColumnNames !== prevState.columnNames) { | ||
| setMapKey(prev => prev + 1); | ||
| prevStateRef.current = { | ||
| columnKey: currentColumnKey, | ||
| columnNames: currentColumnNames, | ||
| selectedRowIdentifiers: new Set(rows.map(r => getRowIdentifier(r, pkColumn, columns))), | ||
| }; | ||
| return; | ||
| } | ||
|
|
||
| if (currentColumnKey === prevState.columnKey && | ||
| currentColumnNames === prevState.columnNames && | ||
| rows.length > 0) { | ||
| prevStateRef.current.selectedRowIdentifiers = new Set( | ||
| displayRows.map(r => getRowIdentifier(r, pkColumn, columns)) | ||
| ); | ||
| } | ||
| }, [currentColumnKey, currentColumnNames, rows, pkColumn, columns]); | ||
|
|
||
| // Get rows to display based on selection | ||
| const displayRows = React.useMemo(() => { | ||
anilsahoo20 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| // No geometry column selected or no rows available - nothing to display | ||
| if (!currentColumnKey || rows.length === 0) return []; | ||
| const prevState = prevStateRef.current; | ||
|
|
||
| // Column context changed (different geometry column or different query schema) | ||
| // Show all new rows since previous selection is no longer valid | ||
| if (currentColumnKey !== prevState.columnKey || currentColumnNames !== prevState.columnNames) { | ||
| return rows; | ||
| } | ||
|
|
||
| const prevIdentifiers = prevState.selectedRowIdentifiers; | ||
| // No previous selection recorded - show all rows | ||
| if (prevIdentifiers.size === 0) return rows; | ||
|
|
||
| // Previous selection was a subset of total rows, meaning user had specific rows selected. | ||
| // Try to match those previously selected rows in the new result set using stable | ||
| // row identifiers (PK value, first column value, or hash fallback). | ||
| // This handles the case where same query reruns with more/fewer rows | ||
| if (prevIdentifiers.size < rows.length) { | ||
| const matched = matchRowSelection(prevIdentifiers, rows, pkColumn, columns); | ||
| // If matched rows found, show only those; otherwise fall back to all rows | ||
| return matched.length > 0 ? matched : rows; | ||
| } | ||
| // Previous selection covered all rows (or same count) - show all current rows | ||
| return rows; | ||
| }, [rows, currentColumnKey, currentColumnNames, pkColumn, columns]); | ||
|
|
||
| // Parse geometry data only when needed | ||
| const data = React.useMemo(() => { | ||
| if (!currentColumnKey) { | ||
| const hasGeometryColumn = columns.some(c => c.cell === 'geometry' || c.cell === 'geography'); | ||
| return { | ||
| 'geoJSONs': [], | ||
| 'selectedSRID': 0, | ||
| 'getPopupContent': undefined, | ||
| 'infoList': hasGeometryColumn | ||
| ? [gettext('Query complete. Use the Geometry Viewer button in the Data Output tab to visualize results.')] | ||
| : [gettext('No spatial data found. At least one geometry or geography column is required for visualization.')], | ||
| }; | ||
| } | ||
| return parseData(displayRows, columns, column); | ||
| }, [displayRows, columns, column, currentColumnKey]); | ||
|
|
||
| useEffect(()=>{ | ||
| let timeoutId; | ||
| const contentResizeObserver = new ResizeObserver(()=>{ | ||
| clearTimeout(timeoutId); | ||
| if(queryToolCtx.docker.isTabVisible(PANELS.GEOMETRY)) { | ||
| if(queryToolCtx?.docker?.isTabVisible(PANELS.GEOMETRY)) { | ||
| timeoutId = setTimeout(function () { | ||
| mapRef.current?.invalidateSize(); | ||
| }, 100); | ||
| } | ||
| }); | ||
| contentResizeObserver.observe(contentRef.current); | ||
| }, []); | ||
| if(contentRef.current) { | ||
| contentResizeObserver.observe(contentRef.current); | ||
| } | ||
| return () => { | ||
| clearTimeout(timeoutId); | ||
| contentResizeObserver.disconnect(); | ||
| }; | ||
| }, [queryToolCtx]); | ||
|
|
||
| // Dyanmic CRS is not supported. Use srid as key and recreate the map on change | ||
| // Dynamic CRS is not supported. Use srid and mapKey as key and recreate the map on change | ||
| return ( | ||
| <StyledBox ref={contentRef} width="100%" height="100%" key={data.selectedSRID}> | ||
| <StyledBox ref={contentRef} width="100%" height="100%" key={`${data.selectedSRID}-${mapKey}`}> | ||
| <MapContainer | ||
| crs={data.selectedSRID === 4326 ? CRS.EPSG3857 : CRS.Simple} | ||
| zoom={2} center={[20, 100]} | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -876,6 +876,14 @@ export function ResultSet() { | |
| rsu.current.setLoaderText = setLoaderText; | ||
|
|
||
| const isDataChangedRef = useRef(false); | ||
| const prevRowsRef = React.useRef(null); | ||
| const prevColumnsRef = React.useRef(null); | ||
| const gvClearedForColumnsRef = useRef(null); | ||
| const lastGvSelectionRef = useRef({ | ||
| type: 'all', // 'all' | 'rows' | 'columns' | ||
| selectedColumns: new Set(), | ||
| }); | ||
|
|
||
| useEffect(()=>{ | ||
| isDataChangedRef.current = Boolean(_.size(dataChangeStore.updated) || _.size(dataChangeStore.added) || _.size(dataChangeStore.deleted)); | ||
| }, [dataChangeStore]); | ||
|
|
@@ -1460,30 +1468,86 @@ export function ResultSet() { | |
| return ()=>eventBus.deregisterListener(QUERY_TOOL_EVENTS.TRIGGER_ADD_ROWS, triggerAddRows); | ||
| }, [columns, selectedRows.size]); | ||
|
|
||
| const getFilteredRowsForGeometryViewer = React.useCallback((useLastGvSelection = false) => { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. isn't there a function already for getting selected rows for geom? |
||
| let selRowsData = rows; | ||
| if(selectedRows.size != 0) { | ||
| selRowsData = rows.filter((r)=>selectedRows.has(rowKeyGetter(r))); | ||
| } else if(selectedColumns.size > 0) { | ||
| let selectedCols = _.filter(columns, (_c, i)=>selectedColumns.has(i+1)); | ||
| selRowsData = _.map(rows, (r)=>_.pick(r, _.map(selectedCols, (c)=>c.key))); | ||
| } else if(useLastGvSelection && lastGvSelectionRef.current.type === 'columns' | ||
| && lastGvSelectionRef.current.selectedColumns.size > 0) { | ||
| let selectedCols = _.filter(columns, (_c, i)=>lastGvSelectionRef.current.selectedColumns.has(i+1)); | ||
| if(selectedCols.length > 0) { | ||
| selRowsData = _.map(rows, (r)=>_.pick(r, _.map(selectedCols, (c)=>c.key))); | ||
| } | ||
| } else if(selectedRange.current) { | ||
| let [,, startRowIdx, endRowIdx] = getRangeIndexes(); | ||
| selRowsData = rows.slice(startRowIdx, endRowIdx+1); | ||
| } else if(selectedCell.current?.[0]) { | ||
| selRowsData = [selectedCell.current[0]]; | ||
| } | ||
| return selRowsData; | ||
| }, [rows, columns, selectedRows, selectedColumns]); | ||
|
|
||
| const openGeometryViewerTab = React.useCallback((column, rowsData) => { | ||
| layoutDocker.openTab({ | ||
| id: PANELS.GEOMETRY, | ||
| title: gettext('Geometry Viewer'), | ||
| content: <GeometryViewer rows={rowsData} columns={columns} column={column} />, | ||
| closable: true, | ||
| }, PANELS.MESSAGES, 'after-tab', true); | ||
| }, [layoutDocker, columns]); | ||
|
|
||
| // Handle manual Geometry Viewer opening | ||
| useEffect(()=>{ | ||
| const renderGeometries = (column)=>{ | ||
| let selRowsData = rows; | ||
| if(selectedRows.size != 0) { | ||
| selRowsData = rows.filter((r)=>selectedRows.has(rowKeyGetter(r))); | ||
| gvClearedForColumnsRef.current = null; | ||
| if(selectedRows.size > 0) { | ||
| lastGvSelectionRef.current = { type: 'rows', selectedColumns: new Set() }; | ||
| } else if(selectedColumns.size > 0) { | ||
| let selectedCols = _.filter(columns, (_c, i)=>selectedColumns.has(i+1)); | ||
| selRowsData = _.map(rows, (r)=>_.pick(r, _.map(selectedCols, (c)=>c.key))); | ||
| } else if(selectedRange.current) { | ||
| let [,, startRowIdx, endRowIdx] = getRangeIndexes(); | ||
| selRowsData = rows.slice(startRowIdx, endRowIdx+1); | ||
| } else if(selectedCell.current?.[0]) { | ||
| selRowsData = [selectedCell.current[0]]; | ||
| lastGvSelectionRef.current = { type: 'columns', selectedColumns: new Set(selectedColumns) }; | ||
| } else { | ||
| lastGvSelectionRef.current = { type: 'all', selectedColumns: new Set() }; | ||
| } | ||
| layoutDocker.openTab({ | ||
| id: PANELS.GEOMETRY, | ||
| title:gettext('Geometry Viewer'), | ||
| content: <GeometryViewer rows={selRowsData} columns={columns} column={column} />, | ||
| closable: true, | ||
| }, PANELS.MESSAGES, 'after-tab', true); | ||
| const selRowsData = getFilteredRowsForGeometryViewer(); | ||
| openGeometryViewerTab(column, selRowsData); | ||
| }; | ||
| eventBus.registerListener(QUERY_TOOL_EVENTS.TRIGGER_RENDER_GEOMETRIES, renderGeometries); | ||
| return ()=>eventBus.deregisterListener(QUERY_TOOL_EVENTS.TRIGGER_RENDER_GEOMETRIES, renderGeometries); | ||
| }, [rows, columns, selectedRows.size, selectedColumns.size]); | ||
| }, [getFilteredRowsForGeometryViewer, openGeometryViewerTab, eventBus, selectedRows, selectedColumns]); | ||
|
|
||
| // Auto-update Geometry Viewer when rows/columns change | ||
| useEffect(()=>{ | ||
| const rowsChanged = prevRowsRef.current !== rows; | ||
| const columnsChanged = prevColumnsRef.current !== columns; | ||
| const currentGeometryColumn = columns.find(col => col.cell === 'geometry' || col.cell === 'geography'); | ||
|
|
||
| if((rowsChanged || columnsChanged) && layoutDocker.isTabOpen(PANELS.GEOMETRY)) { | ||
|
|
||
| const prevColumnNames = prevColumnsRef.current?.map(c => c.key).sort().join(',') ?? ''; | ||
| const currColumnNames = columns.map(c => c.key).sort().join(','); | ||
| const columnsChanged = prevColumnNames !== currColumnNames; | ||
|
|
||
| if(columnsChanged && currentGeometryColumn) { | ||
| gvClearedForColumnsRef.current = currColumnNames; | ||
| lastGvSelectionRef.current = { type: 'all', selectedColumns: new Set() }; | ||
| openGeometryViewerTab(null, []); | ||
| } else if(gvClearedForColumnsRef.current === currColumnNames) { | ||
| openGeometryViewerTab(null, []); | ||
| } else if(currentGeometryColumn && rowsChanged) { | ||
| const useColSelection = lastGvSelectionRef.current.type === 'columns'; | ||
| const selRowsData = getFilteredRowsForGeometryViewer(useColSelection); | ||
| openGeometryViewerTab(currentGeometryColumn, selRowsData); | ||
| } else { | ||
| // No geometry column | ||
| openGeometryViewerTab(null, []); | ||
| } | ||
| } | ||
|
|
||
| prevRowsRef.current = rows; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you're literally duplicating the rows fetched.. can have huge impact on performance. |
||
| prevColumnsRef.current = columns; | ||
| }, [rows, columns, getFilteredRowsForGeometryViewer, layoutDocker]); | ||
|
|
||
| const triggerResetScroll = () => { | ||
| // Reset the scroll position to previously saved location. | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.