Repository: ibis-project/ibis

Issue:
bug: Loading geometry from postgres with SRID gives error
### What happened?

Very similar to #8807. When trying to load a table with a geometry that has an SRID set, I get the following error:

`TypeError: SqlglotType._from_sqlglot_GEOMETRY() takes from 1 to 2 positional arguments but 3 were given`

Here is a reproducible example:

```sql
drop table if exists v_test;
CREATE TABLE v_test (
    id int8 NULL,
    lonlat geometry(Point, 4326) NULL
);

INSERT INTO v_test (id, lonlat)
VALUES 
    (2, st_setsrid(st_makepoint(5.7, 53.2),4326));
```

```python
import ibis

ibis.options.interactive = True

pg_con = ibis.connect(
    "postgresql://blablabla",
)

pg_con.sql("select * from v_test").execute()
```

If it helps, loading this table with geopandas does not give errors.


### What version of ibis are you using?

ibis-framework[postgres,geospatial]==9.1.0

(Also tried --pre)

### What backend(s) are you using, if any?

Postgres

### Relevant log output

_No response_

### Code of Conduct

- [X] I agree to follow this project's Code of Conduct

Discussion:
cpcloud: This isn't hitting the codepath because the round trip assertion first produces a SQLGlot type, but without the SRID, which is incorrect, and then converts that back to an Ibis type that doesn't have the SRID (because it wasn't produced).
cpcloud: You have to also adjust the `_from_ibis_GeoSpatial` method to include the SRID in the SQLGlot output.
ncclementi: I added the srid in the  `_from_ibis_GeoSpatial`  and that seems to be generating the right thing, but for the roundtrip, I'm not sure how to modify the `to_ibis` pass the srid properly to the method. As is, with the test example I'm working, I get
```
-> restored_dtype = SqlglotType.to_ibis(sqlglot_result)
(Pdb) pp sqlglot_result
DataType(
  this=Type.GEOMETRY,
  expressions=[
    DataTypeParam(
      this=Literal(this=1234, is_string=False))])
(Pdb) pp SqlglotType.to_ibis(sqlglot_result)
*** KeyError: '1234'
```

Clearly I'm missing something here, but not sure what the syntax is to be ale to pass the srid, 

```python
 if method := getattr(cls, f"_from_sqlglot_{typecode.name}", None):
            dtype = method(*typ.expressions)
 ```
cpcloud: Pushed up a few changes to get this working for both geometry and geography, was a bit gnarlier than I thought. The issue was that all the Point/MultiPoint etc types need to be handled separately so that they can be passed as a `DataTypeParam` argument when coming **from** Ibis, and then also that they are transformed to the right bits when going **to** Ibis.
cpcloud: Looks like I broke a few things. Have an idea how to fix it, but checking out for the day, happy 4th!
cpcloud: Gonna add the test case from the original issue here as well!

Patch:
diff --git a/ibis/backends/sql/datatypes.py b/ibis/backends/sql/datatypes.py
index 432642fa1..b8bff67a1 100644
--- a/ibis/backends/sql/datatypes.py
+++ b/ibis/backends/sql/datatypes.py
@@ -27,8 +27,6 @@ _from_sqlglot_types = {
     typecode.ENUM16: dt.String,
     typecode.FLOAT: dt.Float32,
     typecode.FIXEDSTRING: dt.String,
-    typecode.GEOMETRY: partial(dt.GeoSpatial, geotype="geometry"),
-    typecode.GEOGRAPHY: partial(dt.GeoSpatial, geotype="geography"),
     typecode.HSTORE: partial(dt.Map, dt.string, dt.string),
     typecode.INET: dt.INET,
     typecode.INT128: partial(dt.Decimal, 38, 0),
@@ -298,15 +296,27 @@ class SqlglotType(TypeMapper):
 
     @classmethod
     def _from_sqlglot_GEOMETRY(
-        cls, arg: sge.DataTypeParam | None = None
+        cls, arg: sge.DataTypeParam | None = None, srid: sge.DataTypeParam | None = None
     ) -> sge.DataType:
         if arg is not None:
-            return _geotypes[str(arg).upper()](nullable=cls.default_nullable)
-        return dt.GeoSpatial(geotype="geometry", nullable=cls.default_nullable)
+            typeclass = _geotypes[arg.this.this]
+        else:
+            typeclass = dt.GeoSpatial
+        if srid is not None:
+            srid = int(srid.this.this)
+        return typeclass(geotype="geometry", nullable=cls.default_nullable, srid=srid)
 
     @classmethod
-    def _from_sqlglot_GEOGRAPHY(cls) -> sge.DataType:
-        return dt.GeoSpatial(geotype="geography", nullable=cls.default_nullable)
+    def _from_sqlglot_GEOGRAPHY(
+        cls, arg: sge.DataTypeParam | None = None, srid: sge.DataTypeParam | None = None
+    ) -> sge.DataType:
+        if arg is not None:
+            typeclass = _geotypes[arg.this.this]
+        else:
+            typeclass = dt.GeoSpatial
+        if srid is not None:
+            srid = int(srid.this.this)
+        return typeclass(geotype="geography", nullable=cls.default_nullable, srid=srid)
 
     @classmethod
     def _from_ibis_Interval(cls, dtype: dt.Interval) -> sge.DataType:
@@ -374,13 +384,30 @@ class SqlglotType(TypeMapper):
 
     @classmethod
     def _from_ibis_GeoSpatial(cls, dtype: dt.GeoSpatial):
-        if (geotype := dtype.geotype) is not None:
-            return sge.DataType(this=getattr(typecode, geotype.upper()))
-        return sge.DataType(this=typecode.GEOMETRY)
+        expressions = [None]
+
+        if (srid := dtype.srid) is not None:
+            expressions.append(sge.DataTypeParam(this=sge.convert(srid)))
+
+        this = getattr(typecode, dtype.geotype.upper())
+
+        return sge.DataType(this=this, expressions=expressions)
+
+    @classmethod
+    def _from_ibis_SpecificGeometry(cls, dtype: dt.GeoSpatial):
+        expressions = [
+            sge.DataTypeParam(this=sge.Var(this=dtype.__class__.__name__.upper()))
+        ]
+
+        if (srid := dtype.srid) is not None:
+            expressions.append(sge.DataTypeParam(this=sge.convert(srid)))
+
+        this = getattr(typecode, dtype.geotype.upper())
+        return sge.DataType(this=this, expressions=expressions)
 
     _from_ibis_Point = _from_ibis_LineString = _from_ibis_Polygon = (
         _from_ibis_MultiLineString
-    ) = _from_ibis_MultiPoint = _from_ibis_MultiPolygon = _from_ibis_GeoSpatial
+    ) = _from_ibis_MultiPoint = _from_ibis_MultiPolygon = _from_ibis_SpecificGeometry
 
 
 class PostgresType(SqlglotType):
@@ -780,7 +807,9 @@ class BigQueryType(SqlglotType):
         return dt.Timestamp(timezone="UTC", nullable=cls.default_nullable)
 
     @classmethod
-    def _from_sqlglot_GEOGRAPHY(cls) -> dt.GeoSpatial:
+    def _from_sqlglot_GEOGRAPHY(
+        cls, arg: sge.DataTypeParam | None = None, srid: sge.DataTypeParam | None = None
+    ) -> dt.GeoSpatial:
         return dt.GeoSpatial(
             geotype="geography", srid=4326, nullable=cls.default_nullable
         )
diff --git a/ibis/expr/datatypes/core.py b/ibis/expr/datatypes/core.py
index 2a2cfa4ea..ff0a42ba8 100644
--- a/ibis/expr/datatypes/core.py
+++ b/ibis/expr/datatypes/core.py
@@ -935,7 +935,7 @@ class JSON(Variadic):
 class GeoSpatial(DataType):
     """Geospatial values."""
 
-    geotype: Optional[Literal["geography", "geometry"]] = None
+    geotype: Literal["geography", "geometry"] = "geometry"
     """The specific geospatial type."""
 
     srid: Optional[int] = None
diff --git a/ibis/expr/types/geospatial.py b/ibis/expr/types/geospatial.py
index 5ae14ee2a..558dfb877 100644
--- a/ibis/expr/types/geospatial.py
+++ b/ibis/expr/types/geospatial.py
@@ -1227,7 +1227,7 @@ class GeoSpatialValue(NumericValue):
         ┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓
         ┃ GeoCentroid(geom)                ┃
         ┡━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┩
-        │ point                            │
+        │ point:geometry                   │
         ├──────────────────────────────────┤
         │ <POINT (935996.821 191376.75)>   │
         │ <POINT (1031085.719 164018.754)> │
@@ -1261,7 +1261,7 @@ class GeoSpatialValue(NumericValue):
         ┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓
         ┃ GeoEnvelope(geom)                                                            ┃
         ┡━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┩
-        │ polygon                                                                      │
+        │ polygon:geometry                                                             │
         ├──────────────────────────────────────────────────────────────────────────────┤
         │ <POLYGON ((931553.491 183788.05, 941810.009 183788.05, 941810.009            │
         │ 197256.211...>                                                               │
diff --git a/ibis/expr/types/numeric.py b/ibis/expr/types/numeric.py
index 91609dcc2..36da3740a 100644
--- a/ibis/expr/types/numeric.py
+++ b/ibis/expr/types/numeric.py
@@ -742,7 +742,7 @@ class NumericValue(Value):
         ┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓
         ┃ GeoPoint(x_cent, y_cent)         ┃
         ┡━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┩
-        │ point                            │
+        │ point:geometry                   │
         ├──────────────────────────────────┤
         │ <POINT (935996.821 191376.75)>   │
         │ <POINT (1031085.719 164018.754)> │
diff --git a/poetry.lock b/poetry.lock
index 93bbe1490..d1310019a 100644
--- a/poetry.lock
+++ b/poetry.lock
@@ -6715,13 +6715,13 @@ sqlcipher = ["sqlcipher3_binary"]
 
 [[package]]
 name = "sqlglot"
-version = "25.4.1"
+version = "25.5.0"
 description = "An easily customizable SQL parser and transpiler"
 optional = false
 python-versions = ">=3.7"
 files = [
-    {file = "sqlglot-25.4.1-py3-none-any.whl", hash = "sha256:4aa019f0cb28097ef03b8b38403dd77727702a380185dabfdf15760e80855fe8"},
-    {file = "sqlglot-25.4.1.tar.gz", hash = "sha256:6c2dc0ce897d7098bca7aafdf7d060c9ae3b3d9e0dcae210378e5a1622ac1de1"},
+    {file = "sqlglot-25.5.0-py3-none-any.whl", hash = "sha256:fe1b2a45e0ffe3d6832b200cf6517ca8befaa9fa2c6998c0a910f6facabd07b9"},
+    {file = "sqlglot-25.5.0.tar.gz", hash = "sha256:33d8e28f342ced643fc77f832ee54219c2a64dfa1e71e1ca542fcffce051a176"},
 ]
 
 [package.extras]
@@ -7671,4 +7671,4 @@ visualization = ["graphviz"]
 [metadata]
 lock-version = "2.0"
 python-versions = "^3.10"
-content-hash = "4b2a61a9e93f55a0862f4d4524cc28f8556926806d8df8ddaba3e001e50c9773"
+content-hash = "c9c2d6ae775188d8776fd6d6ec563edf98377b56ddd59d934161d8712dca9d90"
diff --git a/pyproject.toml b/pyproject.toml
index 93637a6f5..865f03251 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -46,7 +46,7 @@ pyarrow-hotfix = ">=0.4,<1"
 python-dateutil = ">=2.8.2,<3"
 pytz = ">=2022.7"
 rich = ">=12.4.4,<14"
-sqlglot = ">=23.4,<25.5"
+sqlglot = ">=23.4,<25.6"
 toolz = ">=0.11,<1"
 typing-extensions = ">=4.3.0,<5"
 black = { version = ">=22.1.0,<25", optional = true }
diff --git a/requirements-dev.txt b/requirements-dev.txt
index ebefd59b5..6232363e8 100644
--- a/requirements-dev.txt
+++ b/requirements-dev.txt
@@ -253,7 +253,7 @@ sortedcontainers==2.4.0 ; python_version >= "3.10" and python_version < "4.0"
 soupsieve==2.5 ; python_version >= "3.10" and python_version < "3.13"
 sphobjinv==2.3.1.1 ; python_version >= "3.10" and python_version < "3.13"
 sqlalchemy==2.0.31 ; python_version >= "3.10" and python_version < "3.13"
-sqlglot==25.4.1 ; python_version >= "3.10" and python_version < "4.0"
+sqlglot==25.5.0 ; python_version >= "3.10" and python_version < "4.0"
 stack-data==0.6.3 ; python_version >= "3.10" and python_version < "4.0"
 statsmodels==0.14.2 ; python_version >= "3.10" and python_version < "3.13"
 stdlib-list==0.10.0 ; python_version >= "3.10" and python_version < "4.0"


Test patch:
diff --git a/ibis/backends/postgres/tests/test_client.py b/ibis/backends/postgres/tests/test_client.py
index ee94731ce..bde3637cf 100644
--- a/ibis/backends/postgres/tests/test_client.py
+++ b/ibis/backends/postgres/tests/test_client.py
@@ -14,6 +14,7 @@
 from __future__ import annotations
 
 import os
+import string
 from urllib.parse import quote_plus
 
 import hypothesis as h
@@ -30,6 +31,7 @@ import ibis.common.exceptions as com
 import ibis.expr.datatypes as dt
 import ibis.expr.types as ir
 from ibis.backends.tests.errors import PsycoPg2OperationalError
+from ibis.util import gen_name
 
 pytest.importorskip("psycopg2")
 
@@ -390,3 +392,28 @@ def test_password_with_bracket():
         match=f'password authentication failed for user "{IBIS_POSTGRES_USER}"',
     ):
         ibis.connect(url)
+
+
+def test_create_geospatial_table_with_srid(con):
+    name = gen_name("geospatial")
+    column_names = string.ascii_lowercase
+    column_types = [
+        "Point",
+        "LineString",
+        "Polygon",
+        "MultiLineString",
+        "MultiPoint",
+        "MultiPolygon",
+    ]
+    schema_string = ", ".join(
+        f"{column} geometry({dtype}, 4326)"
+        for column, dtype in zip(column_names, column_types)
+    )
+    con.raw_sql(f"CREATE TEMP TABLE {name} ({schema_string})")
+    schema = con.get_schema(name)
+    assert schema == ibis.schema(
+        {
+            column: getattr(dt, dtype)(srid=4326)
+            for column, dtype in zip(column_names, column_types)
+        }
+    )
diff --git a/ibis/backends/sql/tests/test_datatypes.py b/ibis/backends/sql/tests/test_datatypes.py
index 07c50dbeb..b772cdddd 100644
--- a/ibis/backends/sql/tests/test_datatypes.py
+++ b/ibis/backends/sql/tests/test_datatypes.py
@@ -41,8 +41,7 @@ roundtripable_types = st.deferred(
         | its.array_dtypes(roundtripable_types, nullable=true)
         | its.map_dtypes(roundtripable_types, roundtripable_types, nullable=true)
         | its.struct_dtypes(roundtripable_types, nullable=true)
-        | its.geometry_dtypes(nullable=true)
-        | its.geography_dtypes(nullable=true)
+        | its.geospatial_dtypes(nullable=true)
         | its.decimal_dtypes(nullable=true)
         | its.interval_dtype(nullable=true)
     )
@@ -59,16 +58,6 @@ def test_roundtripable_types(ibis_type):
     assert_dtype_roundtrip(ibis_type)
 
 
-@h.given(its.specific_geometry_dtypes(nullable=true))
-def test_specific_geometry_types(ibis_type):
-    sqlglot_result = SqlglotType.from_ibis(ibis_type)
-    assert isinstance(sqlglot_result, sge.DataType)
-    assert sqlglot_result == sge.DataType(this=sge.DataType.Type.GEOMETRY)
-    assert SqlglotType.to_ibis(sqlglot_result) == dt.GeoSpatial(
-        geotype="geometry", nullable=ibis_type.nullable
-    )
-
-
 def test_interval_without_unit():
     with pytest.raises(com.IbisTypeError, match="precision is None"):
         SqlglotType.from_string("INTERVAL")
diff --git a/ibis/tests/strategies.py b/ibis/tests/strategies.py
index f417b3719..d31e1ca03 100644
--- a/ibis/tests/strategies.py
+++ b/ibis/tests/strategies.py
@@ -178,30 +178,17 @@ def struct_dtypes(
     return dt.Struct(fields, nullable=draw(nullable))
 
 
-def geometry_dtypes(nullable=_nullable):
-    return st.builds(dt.GeoSpatial, geotype=st.just("geometry"), nullable=nullable)
-
-
-def geography_dtypes(nullable=_nullable):
-    return st.builds(dt.GeoSpatial, geotype=st.just("geography"), nullable=nullable)
-
-
-def specific_geometry_dtypes(nullable=_nullable):
-    return st.one_of(
-        st.builds(dt.Point, nullable=nullable),
-        st.builds(dt.LineString, nullable=nullable),
-        st.builds(dt.Polygon, nullable=nullable),
-        st.builds(dt.MultiPoint, nullable=nullable),
-        st.builds(dt.MultiLineString, nullable=nullable),
-        st.builds(dt.MultiPolygon, nullable=nullable),
-    )
-
-
 def geospatial_dtypes(nullable=_nullable):
+    geotype = st.one_of(st.just("geography"), st.just("geometry"))
+    srid = st.one_of(st.just(None), st.integers(min_value=0))
     return st.one_of(
-        specific_geometry_dtypes(nullable=nullable),
-        geometry_dtypes(nullable=nullable),
-        geography_dtypes(nullable=nullable),
+        st.builds(dt.Point, geotype=geotype, nullable=nullable, srid=srid),
+        st.builds(dt.LineString, geotype=geotype, nullable=nullable, srid=srid),
+        st.builds(dt.Polygon, geotype=geotype, nullable=nullable, srid=srid),
+        st.builds(dt.MultiPoint, geotype=geotype, nullable=nullable, srid=srid),
+        st.builds(dt.MultiLineString, geotype=geotype, nullable=nullable, srid=srid),
+        st.builds(dt.MultiPolygon, geotype=geotype, nullable=nullable, srid=srid),
+        st.builds(dt.GeoSpatial, geotype=geotype, nullable=nullable, srid=srid),
     )