Skip to content
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

PostGIS: use ST_Intersects instead of && for bounding box (fixes #6181, fixes #6230) #6347

Merged
merged 2 commits into from Jun 9, 2021

Conversation

rouault
Copy link
Contributor

@rouault rouault commented Jun 8, 2021

No description provided.

@rouault rouault added the backport branch-7-6 To backport a pull request to branch-7-6 label Jun 8, 2021
@rouault rouault marked this pull request as draft June 8, 2021 16:43
@rouault rouault marked this pull request as ready for review June 8, 2021 17:51
@rouault rouault merged commit 2d73b37 into MapServer:main Jun 9, 2021
@github-actions
Copy link
Contributor

github-actions bot commented Jun 9, 2021

The backport to branch-7-6 failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-branch-7-6 branch-7-6
# Navigate to the new working tree
cd .worktrees/backport-branch-7-6
# Create a new branch
git switch --create backport-6347-to-branch-7-6
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick 40f310f5d6591e06c3997ace8648c0a8a1f74229,bd6b73a71567c1272d5e758265e1e921f5d7dce6
# Push it to GitHub
git push --set-upstream origin backport-6347-to-branch-7-6
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-branch-7-6

Then, create a pull request where the base branch is branch-7-6 and the compare/head branch is backport-6347-to-branch-7-6.

@sdlime
Copy link
Member

sdlime commented Jun 22, 2021

I'm not positive on this but I was trying branch-7-6 in production and ran into some weirdness with mode=nquery and PostGIS layers. I was doing point queries on polygon layers and experiencing inconsistent results across multiple layers - some features would return as a result and some wouldn't. Reverting to the 7.6.3 release fixed things. This looks to be the only addition that could affect those operations but it's not obvious to me why. I'll have to try and narrow it down. Is there anything that might be specific to PostGIS version with this change?

--Steve

@rouault
Copy link
Contributor Author

rouault commented Jun 22, 2021

I'm not aware of PostGIS version making a difference for ST_Intersection(), but one thing I noticed is that ST_Intersection() was more picky about SRID than &&. I had to do https://github.com/MapServer/MapServer/blob/main/mappostgis.cpp#L1824 because of that

@sdlime
Copy link
Member

sdlime commented Jun 22, 2021 via email

@sdlime
Copy link
Member

sdlime commented Jun 28, 2021

Hi Even: Back at this and with debugging on with point queries the SQL that gets generated looks something like:

select "objectid"::text,"dowlknum"::text,"lake_name"::text,"lakefinder"::text,ST_AsBinary(("shape"),'NDR') as geom,"objectid"::text from (SELECT shape, objectid, dowlknum, map_label AS lake_name, in_lakefinder AS lakefinder FROM gdrs.dnr_hydro
_features_compass WHERE map_display='Y' AND map_label != '') as foo where ST_Intersects("shape", ST_GeomFromText('POLYGON((440050.852933846 5129741.71595332,440050.852933846 5129741.71595332,440050.852933846 5129741.71595332,440050.852933846 5129741.71595332,440050.852933846 5129741.71595332))',26915))

The bounding box is actually just a point in these cases (xmin=xmax, ymin=ymax) which never caused problems. In this case the resulting POLYGON has the same issue where all points in the geometry are the same. The ST_Intersects() function seems to be indeterminate in this case - sometimes it works, sometimes it doesn't.

If I edit the SQL and turn the POLYGON into a POINT, so:

select "objectid"::text,"dowlknum"::text,"lake_name"::text,"lakefinder"::text,ST_AsBinary(("shape"),'NDR') as geom,"objectid"::text from 
(SELECT shape, objectid, dowlknum, map_label AS lake_name, in_lakefinder AS lakefinder FROM gdrs.dnr_hydro_features_compass WHERE map_display='Y' AND map_label != '') as foo 
where ST_Intersects("shape", ST_GeomFromText('POINT(440050.852933846 5129741.71595332)',26915))

then the correct feature is found.

--Steve

BTW the MapServer call looks like:

mapserv "QUERY_STRING=map=test.map&mode=nquery&mapxy=440050.852933846+5129741.71595332&layers=state%20lake"

@rouault
Copy link
Contributor Author

rouault commented Jun 28, 2021

Hi Even: Back at this and with debugging on with point queries the SQL that gets generated looks something like:

should be fixed per #6354

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport branch-7-6 To backport a pull request to branch-7-6
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants