overload type_to_sql

[Tristramg]

This fixes when using generated geometry column from schema.rb

ref #369

type_to_sql had been removed in https://github.com/rgeo/activerecord-postgis-adapter/pull/360/files#diff-eeeb5e84f6023e4b440660467863dbed44712213c7c65db4e6fe401a3c219b1cL37-L57 as the adapter knows how to handle limits as strings. But in this situation, it is as a hash

[keithdoggett]

Hey @Tristramg thanks for submitting this PR. I think it looks good, but we should add a test to make sure. I think we may need to add more types to that array as well in case people make columns as t.st_polygon for example.

Here's a sample test that we can add to test/ddl_test.rb

  def test_generated_geometry_column
    klass.connection.create_table(:spatial_models, force: true) do |t|
      t.st_point :coordinates, limit: { srid: 4326 }
      t.virtual :generated_buffer, type: :st_polygon, limit: { srid: 4326 }, as: 'ST_Buffer(coordinates, 10)', stored: true
    end
    klass.reset_column_information
    col = klass.columns.last
    assert_equal(:geometry, col.type)
    assert(col.virtual?)
  end

[keithdoggett]

Looking into this more, I think we need to modify new_column_definition in spatial_table_definition.rb and check if a column with the type :virtual is also a spatial type. Then what you have should work.

Looks like adding this

          col_type = if type.to_sym == :virtual
            options[:type]
          else
            type
          end

at the beginning of new_column_definition works

[Tristramg]

Thank you a lot. You basically wrote the whole commit !

Much better in new_column_definition as it avoids doing the same work twice

[keithdoggett]

Hey @Tristramg thanks for adding that. Can you merge master into this branch so we can add the changes from this PR #376? That should fix the failing tests.

[Tristramg]

Thank you for the reminder. I’ve rebased my fork

…

On Wed, 26 Apr 2023 at 16:20, Keith Doggett ***@***.***> wrote: Hey @Tristramg <https://github.com/Tristramg> thanks for adding that. Can you merge master into this branch so we can add the changes from this PR #376 <#376>? That should fix the failing tests. — Reply to this email directly, view it on GitHub <#377 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAC7S3575LQ7RYT46IR2RLXDEVMTANCNFSM6AAAAAAXG5ABPY> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[keithdoggett]

Looks like it's failing with older versions of postgres since it is probably not supported before v12. We can add a skip statement at the beginning of it with a conditional to check for virtual columns being supported.

skip "Virtual Columns are not supported in this version of PostGIS" unless SpatialModel.connection.supports_virtual_columns?

[Tristramg]

Let’s see if this time is the right one. Thank you for you patience and walking me through all this :)

[keithdoggett]

No problem! Thanks for the contribution! I can get a release out in the next day or 2 with this change.