fix(datatypes): proper handling of srid in geospatial datatypes

[ncclementi]

• Closes bug: Loading geometry from postgres with SRID gives error #9465

This is still broken, some how the srid value is not being passed properly to _from_sqlglot_GEOMETRY, I think it's because I need to change something in from_ibis too, but not sure how to handle passing the param.

I'm constructing well, the ibis_type in the test, I see

pp ibis_type
GeoSpatial(geotype='geometry', srid=1234, nullable=True)

But when this is passed to SqlglotType.from_ibis(ibis_type) I'm missing somthing that I don't quite pin point yet.

Any comments are welcome

[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,

 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.

[ncclementi]

Is this to build the two separate strategies? If so, pretty neat : )

[cpcloud]

Yep!

[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!

[cpcloud]

@ncclementi This LGTM if you want to give it another look!

[ncclementi]

Small question, we don't need these ones any more because we will always git this path
method := getattr(cls, f"_from_sqlglot_{typecode.name}", None) in to_ibis() now?

[cpcloud]

Yes, that's the reason, but it was that way before this PR too.

[ncclementi]

LGTM, thanks for taking this across the line, looks like it got a bit gnarly.

I left a comment with one question, but it's mostly for my understanding.

[cpcloud]

Thanks @ncclementi! Merging.