Fix SQL selected / SQL explain for gis queries#1426
Fix SQL selected / SQL explain for gis queries#1426auvipy merged 3 commits intodjango-commons:mainfrom
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1426 +/- ##
==========================================
- Coverage 85.89% 85.78% -0.12%
==========================================
Files 35 35
Lines 1893 1899 +6
Branches 276 279 +3
==========================================
+ Hits 1626 1629 +3
- Misses 187 190 +3
Partials 80 80
Continue to review full report at Codecov.
|
matthiask
left a comment
There was a problem hiding this comment.
Thanks! This looks very hacky. I think I understand why this may be necessary but I fear the complexity a bit. Also, this change would definitely need a test to prevent regressions in the future.
Yes, fully agreed! I only tried to make the existing solution a bit more readable by avoiding the numerical offsets. I just looked into adding tests, which would require another test matrix entry with |
|
@jieter Regarding |
|
I've added postgis to the tests (not yet added to github actions). I also added a test, but this still crashes with the same error, and (for as far as I can see) the same input as captured while running debug-toolbar with the fix in my development environment. I'll try to revisit this later. |
auvipy
left a comment
There was a problem hiding this comment.
would you mind fix the merge conflicts?
Without this fix, pushing the 'sel' or 'explain' button for a query containing some EWKB-encoded geometry
as parameter results in this crash:
```
Internal Server Error: /__debug__/sql_explain/
Traceback (most recent call last):
File "/Users/jieter/.pyenv/versions/obs/lib/python3.8/site-packages/django/db/backends/utils.py", line 86, in _execute
return self.cursor.execute(sql, params)
psycopg2.errors.InternalError_: parse error - invalid geometry
LINE 1: ...ure" IN (0, 1, 5)) AND "waarneming"."geo_point" @ 'ST_GeomFr...
^
HINT: "ST" <-- parse error at position 2 within geometry
```
I'm not sure if this is the appropriate location in the code, but with this fix, both `sql_select` and `sql_explain`
work without flaws.
Previous PR adding a similar fix: django-commons#1130
Fixes: django-commons#423
for more information, see https://pre-commit.ci
|
thanks you! you can provide aditional tests if needed in another PR |
|
@auvipy no problem! |
Without this fix, pushing the 'sel' or 'explain' button for a query containing some EWKB-encoded geometry
as parameter results in this crash:
I'm not sure if this is the appropriate location in the code, but with this fix, both
sql_selectandsql_explainwork without flaws.
Previous PR adding a similar fix: #1130
Fixes: #423