-
Notifications
You must be signed in to change notification settings - Fork 112
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
Type: Fix Modified check_ctor_args to pass default SRS_ID value in case of null #488
Type: Fix Modified check_ctor_args to pass default SRS_ID value in case of null #488
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @satyamsoni2211
Thanks for this! It's indeed possible to have inappropriate srid value here.
Apart from my 2 suggestions, could you also add a test that fails on the master branch and succeeds with this PR please?
@adrien-berchet ~/Documents/alchemy ·················································································································································· alchemy Py │ at 09:15:53 ─╮
❯ pytest -v ─╯
======================================================================================= test session starts ========================================================================================
platform darwin -- Python 3.11.4, pytest-7.4.4, pluggy-1.4.0 -- /Users/satyamsoni/Documents/alchemy/.venv/bin/python
cachedir: .pytest_cache
rootdir: /Users/satyamsoni/Documents/alchemy
plugins: asyncio-0.23.4, custom-exit-code-0.3.0, cov-4.1.0, mock-3.12.0
asyncio: mode=Mode.STRICT
collected 38 items
tests/test_geometry.py::TestGeometry::test_get_col_spec PASSED [ 2%]
tests/test_geometry.py::TestGeometry::test_get_col_spec_no_srid PASSED [ 5%]
tests/test_geometry.py::TestGeometry::test_get_col_spec_invalid_srid PASSED [ 7%]
tests/test_geometry.py::TestGeometry::test_get_col_spec_no_typmod PASSED [ 10%]
tests/test_geometry.py::TestGeometry::test_check_ctor_args_bad_srid PASSED [ 13%]
tests/test_geometry.py::TestGeometry::test_get_col_spec_geometryzm PASSED [ 15%]
tests/test_geometry.py::TestGeometry::test_get_col_spec_geometryz PASSED [ 18%]
tests/test_geometry.py::TestGeometry::test_get_col_spec_geometrym PASSED [ 21%]
tests/test_geometry.py::TestGeometry::test_check_ctor_args_srid_not_enforced PASSED [ 23%]
tests/test_geometry.py::TestGeometry::test_check_ctor_args_use_typmod_nullable PASSED [ 26%]
tests/test_geometry.py::TestGeometry::test_column_expression PASSED [ 28%]
tests/test_geometry.py::TestGeometry::test_select_bind_expression PASSED [ 31%]
tests/test_geometry.py::TestGeometry::test_insert_bind_expression PASSED [ 34%]
tests/test_geometry.py::TestGeometry::test_function_call PASSED [ 36%]
tests/test_geometry.py::TestGeometry::test_non_ST_function_call PASSED [ 39%]
tests/test_geometry.py::TestGeometry::test_subquery PASSED [ 42%]
tests/test_geometry.py::TestGeography::test_get_col_spec PASSED [ 44%]
tests/test_geometry.py::TestGeography::test_get_col_spec_no_typmod PASSED [ 47%]
tests/test_geometry.py::TestGeography::test_column_expression PASSED [ 50%]
tests/test_geometry.py::TestGeography::test_select_bind_expression PASSED [ 52%]
tests/test_geometry.py::TestGeography::test_insert_bind_expression PASSED [ 55%]
tests/test_geometry.py::TestGeography::test_function_call PASSED [ 57%]
tests/test_geometry.py::TestGeography::test_non_ST_function_call PASSED [ 60%]
tests/test_geometry.py::TestGeography::test_subquery PASSED [ 63%]
tests/test_geometry.py::TestPoint::test_get_col_spec PASSED [ 65%]
tests/test_geometry.py::TestCurve::test_get_col_spec PASSED [ 68%]
tests/test_geometry.py::TestLineString::test_get_col_spec PASSED [ 71%]
tests/test_geometry.py::TestPolygon::test_get_col_spec PASSED [ 73%]
tests/test_geometry.py::TestMultiPoint::test_get_col_spec PASSED [ 76%]
tests/test_geometry.py::TestMultiLineString::test_get_col_spec PASSED [ 78%]
tests/test_geometry.py::TestMultiPolygon::test_get_col_spec PASSED [ 81%]
tests/test_geometry.py::TestGeometryCollection::test_get_col_spec PASSED [ 84%]
tests/test_geometry.py::TestRaster::test_get_col_spec PASSED [ 86%]
tests/test_geometry.py::TestRaster::test_column_expression PASSED [ 89%]
tests/test_geometry.py::TestRaster::test_insert_bind_expression PASSED [ 92%]
tests/test_geometry.py::TestRaster::test_function_call PASSED [ 94%]
tests/test_geometry.py::TestRaster::test_non_ST_function_call PASSED [ 97%]
tests/test_geometry.py::TestCompositeType::test_ST_Dump PASSED [100%]
======================================================================================== 38 passed in 0.11s ======================================================================================== |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks nice, thanks!
Just a minor typo left.
Thanks @satyamsoni2211 ! |
@adrien-berchet patch version bump would work |
I would also like you to see if my library can be of use to you. Lazy Alchemy. This lazily reflects table from any schema and caches it. |
The new release is up! https://pypi.org/project/GeoAlchemy2/0.14.4/ About your project, that's interesting indeed. But I currently don't work on any project where the number of schemas is an issue (I actually didn't work on any project with a DB for a while 😅 ). Did you propose to integrate this into SQLAchemy? So it would be available to all the users. |
Currently the column conversion from NullType to Geometry by
SQLAlchemy
is failing due toNULL
SRS_ID
from the DB. In such scenario, where a null value is received, function should be able to override it by default value of-1
rather than trying to convertNoneType
toint
. This fix takes into consideration failure and overrideNoneType
by default value.Below is the error that occurred due to missing
SRS_ID
.Post fix this seems to be working fine.
Checklist
This pull request is:
Fixes: #<issue number>
in the description if it solves an existing issue(which must include a complete example of the issue).
main
branch and pass with the provided fix.Fixes: #<issue number>
in the description if it solves an existing issue(which must include a complete example of the feature).