Constructing ORM instance with geometry from str can retain raw str value for future queries

[huonw]

Thanks for Geoalchemy2!

We're constructing ORM instances by specifying a WKT value as a string and inserting them as rows into PostgreSQL. If the ORM model instance is cached, later queries returning that row use the original string for the geometry, rather than the WKBElement value returned from the queries.

In particular, if an instance with geometry specified like Val(..., point='POINT(0 1)') is add'd to a session, and code is still holding a reference to it when a later query runs, that query will return the original instance directly without updating any properties (even though the query explicitly returns the appropriate WKBElement). Notes about the reproducer below:

• the debug logging shows that the sa.select(Val) query is reading and converting the WKB correctly even in the BUG case
• the printed queried.point is a string in the BUG case but WKBElement in the GOOD ones
• expiring the instance doesn't work well when using async SQLAlchemy, but expunging does

from geoalchemy2 import Geography
import sqlalchemy as sa
from sqlalchemy import orm

Base = orm.declarative_base()

class Val(Base):
    __tablename__ = "val"

    id = sa.Column(sa.BIGINT, primary_key=True)

    point = sa.Column(Geography("POINT"))

url = sa.engine.url.URL.create(
    "postgresql",
    username="postgres",
    password="postgres",
    host="localhost",
    port=5431,
    database="geoalchemy_testing",
)
engine = sa.create_engine(url)

with engine.begin() as conn:
    Base.metadata.drop_all(conn)
    Base.metadata.create_all(conn)

def run(mode):
    print(f"\n\n**** {mode=}")
    with orm.Session(engine) as session:

        original = Val(id=1, point="POINT(0 1)")
        session.add(original)
        session.flush()

        if mode == "expire":
            session.expire(original)
        elif mode == "expunge":
            session.expunge(original)

        result = session.execute(sa.select(Val))
        queried = result.scalars().one()

        print(f"{queried.point=}\n{queried is original=}")

engine.echo = "debug"

# BUG:
run(None)
# output:
# ...
# 2022-05-26 11:40:04,357 DEBUG sqlalchemy.engine.Engine Row (1, <WKBElement at 0x10732f5b0; 01010000000000000000000000000000000000f03f>)
# queried.point='POINT(0 1)'
# queried is original=True
# ...

engine.echo = False

# GOOD:
run("expire")
# output:
# queried.point=<WKBElement at 0x102494550; 01010000000000000000000000000000000000f03f>
# queried is original=True

run("expunge")
# output:
# queried.point=<WKBElement at 0x102494490; 01010000000000000000000000000000000000f03f>
# queried is original=False

geoalchemy2==0.11.1
greenlet==1.1.2
packaging==21.3
psycopg2-binary==2.9.3
pyparsing==3.0.9
sqlalchemy==1.4.36

This may not be a Geoalchemy2 bug per-se, but it's error prone: later calls to geoalchemy2.point.to_shape(queried.point) will explode because the input is a string, not something it understands.

Are there any SQLAlchemy tricks Geoalchemy2 can do to alleviate some of the danger here? For instance, convert to geometries specified as raw strings to be stored as WKTElement, so that they can be at least be safely passed to to_shape.

[adrien-berchet]

Hi @huonw
Thanks for this report.
Indeed there is no automatic cast or even check in this case. I would suggest to explicitly use a WKTElement if you want to use a WKT string.

original = Val(id=1, point=WKTElement("POINT(0 1)"))

I will check if it is possible and reasonable to ensure that a value passed to a spatial type is either a WKTElement or a WKBElement.

[adrien-berchet]

Hi @huonw
Did you have the opportunity to test this solution? Is it acceptable for you?