sql error on spatial index creation (new geo fields on existing data types)

This bug goes beyond the geocode module--this SQL error shows up whenever you add a new geo field to a content type with existing nodes.

MySQL can't create a spatial index on a field if the field contains invalid geometry data--like if the field is empty. Two things combine to cause the bug in this issue:

• When you add a new CCK field that handles multiple values to an node type, CCK creates a row with blank values for each existing node of that type.
• All Geo fields handle multiple values, which (intentionally) forces CCK to keep them in their own tables.

So when you add a new Geo field to a content type with existing nodes, CCK creates rows with blank values in the geospatial column. Then Geo tries to create an index on that geospatial column, which now has invalid geometry data in it--MySQL needs WKB, CCK populates the fields with ''.

I'm not sure about the best way to solve this--should rows with '' instead of WKB in the geospatial data column just be arbitrarily dropped before trying to create a spatial index? (@allie)

"I'm not sure about the best way to solve this--should rows with '' instead of WKB in the geospatial data column just be arbitrarily dropped before trying to create a spatial index? (@allie)"

I can't think of a case where having '' in any pending-wkb column would be useful, so my first inclination is to let it drop.

Alternatively, perhaps we can stop telling CCK that there's a geo column at all? When I was constructing the field module, I was hoping to outsource more of the field manipulation to CCK instead of managing it myself. In the 'database columns' op of hook_field I define a blob field like so:

      $spec = array(
        'wkt' => array(
          'type' => 'text',
          'size' => 'big',
          'not null' => FALSE,
          'description' => t('Well-known-text format'),
        ),
        'geo' => array(
          'type' => 'blob', // This column will be replaced in _geo_field_init.
          'geo_type' => $field['geo_type'],
          'not null' => TRUE,
          'description' => t('Geometry field'),
        ),
      );

But eventually I found myself implementing the INSERT/UPDATE, and then SELECT portions on my own, so it's not even useful for that column to exist as far as CCK is concerned. Thus, perhaps it's safe to remove geo from the 'database columns' definition and then remove this line from _geo_field_init:
db_drop_field($ret, $table, $column);

This begs the question of why we're bothering to let CCK store anything at all ( the wkt, lat and lon columns are arguably disposable ). I was hoping to stay as CCK-compatible as possible in case we could re-outsource everything later, but if this creates more problems than it fixes, perhaps we simply manage the table on its own?

@allie -- One parallel from other modules that provide complex field functionality is that CCK is only trusted with an id anyway, rather than the full field data; for example, in location.module (the example I am most familiar with) all the bits and pieces of addresses stored by Location are kept in the {location} table with a location id, while cck stores only the location id. This would also address some of the issues around geo's current behavior of forcing cck fields to multiple values.

@ansorg -- it looks like extracting lat/lon from EXIF data is supported when using imagefield rather than filefield... the only geocoding linked to filefields at this point seems to be GPS track data from .gpx files (as you have noticed :)

@ansorg -- I meant the "imagefield" module, see http://drupal.org/project/imagefield

@ansorg -- well, it looks like the imagefield cck field type is "filefield", while the Geocode module expects it to be "imagefield". This should be changed on line 18 of geocode/includes/modules/imagefield.inc:

/**
 * Implementation of hook_geocode_handler_info() on behalf of imagefield.
 */
function imagefield_geocode_handler_info() {
  $handlers = array();
  // This functionality requires PHP's exif module
  if (function_exists('exif_read_data')) {
    $handlers['geocode_imagefield'] = array(
      'title' => t('Image field'),
      'module' => 'geocode',
      'file' => 'imagefield.inc',
      'file path' => drupal_get_path('module', 'geocode') .'/includes/modules',
      'return types' => array(
        'geo' => array('point'),
      ),
      'field types' => array('imagefield'), // <-- should be 'filefield'
    );
  }
  return $handlers;
}
 

er, this part of the issue probably belongs over in Geocode's queue... http://drupal.org/project/issues/geocode :)

I'm optimistically marking this as fixed by http://drupal.org/cvs?commit=223658 .

If you encounter other issues, please open a new ticket, as I'm having a hard time groking how all the geocode hook stuff is related to Geo, and whether or not Geo needs to be doing anything accordingly :)

Thanks!

@ansorg

This solution enabled the Image field option of the Geocode Handle:
but the Return value: option still shows linestring... is there anything else to do to change it?

Thanks