Add support for SQL Server working copy #362
Conversation
|
|
||
|
|
||
| def quote(ident): | ||
| return _PREPARER.quote(ident) |
There was a problem hiding this comment.
We need to drop these for SQLAlchemy's rather than writing more :)
There was a problem hiding this comment.
This is sqlalchemy's quote - I just wrapped it since writing quote() is easier than writing IdentifierPreparer(MSDialect()).quote()
But you are right - I should be trying to avoid manually quoting things. This quote here is only called when generating a table spec eg ("FID" bigint, "GEOM" geography, "CRT_DATE" datetime) - it could go away if, instead of generating a table specification in SQL, I created one using sqlalchemy types and created that. It's about the same amount of work, but I just haven't switched to it yet. (It also might not end up being as symmetrical as the current code with regards to writing table specs and reading table specs - I don't know exactly unless I try to write it).
There was a problem hiding this comment.
One option is to use use Table()/etc, but my understanding is you can also use the SA SQL expression builder which has escaping functions/templating/etc for SQL strings
There was a problem hiding this comment.
Yes - will revisit this soon to try and get rid of lots of f-strings in all 3 working copies.
olsen232
force-pushed
the
sqlserver
branch
2 times, most recently
from
22d98b1
to
00c1429
Compare
olsen232
force-pushed
the
sqlserver
branch
3 times, most recently
from
e4abd0b
to
7955906
Compare
| @@ -603,6 +634,19 @@ def _apply_meta_metadata_dataset_json(self, sess, dataset, src_value, dest_value | |||
| def _update_last_write_time(self, sess, dataset, commit=None): | |||
| self._update_gpkg_contents(sess, dataset, commit) | |||
|
|
|||
| def _get_geom_extent(self, sess, dataset): | |||
| """Returns the envelope around the entire dataset as (min_x, min_y, max_x, max_y).""" | |||
There was a problem hiding this comment.
Using GetSpatialIndexExtent() should be a lot quicker
There was a problem hiding this comment.
Not done.
Ugh just spent a good half hour on this - for no particular reason, calling GetSpatialIndex from within Sno / sqlalchemy reliably segfaults for me, although I can call it from my sqlite3 CLI. My spatialite version should be the same for both. Left this as-is.
There was a problem hiding this comment.
Fair enough.
It's documented as returning a Spatialite geometry, so would need to cast it to GPKG/WKB or extract min/max values from it. Not sure if that's the issue though. 🤷
There was a problem hiding this comment.
Presumably not the issue - I was calling ST_MinX etc on it, it was working in sqlite3, just not in Sno
Description
A working implementation of a SQL server working copy.
Tests are passing - checkouts, diffs, and commits are working.
TODO:
Documentation is not yet updated.
Dependencies are not yet updated - so, this works for me locally, and all new sqlserver tests are passing, but this won't work for anyone else because they don't have the appropriate SQL server driver etc. I installed these manually to my local Sno environment. However, this won't be immediately obvious in github actions, since all of the relevant tests will be skipped anyway since no SQL server database is configured.