Conversation
| SpatialModel.lease_connection.create_enum(:point_type, ["point", "line_string", "polygon"]) | ||
| SpatialModel.lease_connection.create_table(:spatial_models, force: true) do |t| |
There was a problem hiding this comment.
Is it properly cleanup afterwards ? If so, how ?
There was a problem hiding this comment.
i don't know - i copied this from the other examples, can you point me to where any explicit cleanup happens?
| t.column "latlon", :st_point, srid: 3785 | ||
| t.column "latlon_geo", :st_point, srid: 4326, geographic: true | ||
| t.column "default_latlon", :st_point, srid: 0, default: "POINT(0.0 0.0)" |
There was a problem hiding this comment.
these were copied from create_model below, happy to remove them if you think it's necessary.
| @@ -12,7 +12,9 @@ def initialize(name, default, sql_type_metadata = nil, null = true, | |||
| serial: nil, generated: nil, spatial: nil, identity: nil) | |||
| @sql_type_metadata = sql_type_metadata | |||
| @geographic = !!(sql_type_metadata.sql_type =~ /geography\(/i) | |||
| if spatial | |||
| if sql_type_metadata.type == :enum | |||
There was a problem hiding this comment.
I feel like we should not be there if we are handling enums. Otherwise this also means that sql_type_metadata is set for every column, spatial or not.
I believe we should verify how the step before that one is working to make sure this is the correct place (e.g. who is initialising this)
There was a problem hiding this comment.
afaict this is by design.
this is initialized from #new_column_from_field which overrides the rails implementation and returns a SpatialColumn (instead of a PostgreSQL::Column) for all columns.
i'm not sure what would happen if some fields were one type and some were another but it seems out of scope for this pr.
|
@modosc, I did not forget the PR. I am prioritizing rails 8.1 compatibility though. |
no worries - lmk if there's anything i can do to help on that side. |
|
Hey @modosc, could you update your PR and add an entry to the History.md file please ? I also am not 100% sure about how this is handled yet. May I work on it as well or would you rather keep ownership ? If I may, I'd like access to your repo as a contributor! In any case, I'll work on the activerecord-cockroachdb-adapter version of it now hence we'll be able to compare in the end :) |
modosc
force-pushed
the
fix-enum-types
branch
from
1a0c52e to
3a3d248
Compare
|
@BuonOmo i just rebased it. i can't give you access because it's originating from my work org. |
|
Ok so what we need:
What I'm unsure about:
Let's work on that together then :) |
2 participants
fixes #432