diff --git a/CHANGELOG.md b/CHANGELOG.md index 9a453f6a3..a2fa8c594 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,7 +13,9 @@ For a complete change history, see the SVN log. - GDAL: respect nodata for paletted/colormapped images (#1160) -- PostGIS: the primary key, for tables containing one, is now auto-detected allowing for globally unique feature id values (#804) +- PostGIS: Added a new option called 'autodetect_key_field' (by default false) that if true will + trigger autodetection of a given tables' primary key allowing for feature.id() to represent + globally unique ids. This option has no effect if the user has not manually supplied the 'key_field' option. (#804) - Cairo: Add full rendering support for markers to match AGG renderer functionality (#1071) diff --git a/plugins/input/postgis/postgis_datasource.cpp b/plugins/input/postgis/postgis_datasource.cpp index 6ff78bcd5..59434803a 100644 --- a/plugins/input/postgis/postgis_datasource.cpp +++ b/plugins/input/postgis/postgis_datasource.cpp @@ -107,7 +107,7 @@ void postgis_datasource::bind() const boost::optional initial_size = params_.get("initial_size", 1); boost::optional max_size = params_.get("max_size", 10); - boost::optional require_key = params_.get("require_key", false); + boost::optional autodetect_key_field = params_.get("autodetect_key_field", false); ConnectionManager* mgr = ConnectionManager::instance(); mgr->registerPool(creator_, *initial_size, *max_size); @@ -230,7 +230,7 @@ void postgis_datasource::bind() const } // detect primary key - if (key_field_.empty()) + if (*autodetect_key_field && key_field_.empty()) { std::ostringstream s; s << "SELECT a.attname, a.attnum, t.typname, t.typname in ('int2','int4','int8') " @@ -267,34 +267,42 @@ void postgis_datasource::bind() const { key_field_ = std::string(key_field_string); #ifdef MAPNIK_DEBUG - std::clog << "PostGIS Plugin: auto-detected key field of '" << key_field_ << "' on table '" << geometry_table_ << "'\n"; + std::clog << "PostGIS Plugin: auto-detected key field of '" + << key_field_ << "' on table '" + << geometry_table_ << "'\n"; #endif } } - else // warn with odd cases like numeric primary key + else { - std::clog << "PostGIS Plugin: Warning: '" << rs_key->getValue(0) << "' on table '" << geometry_table_ << "' is not a valid integer primary key field\n"; + // throw for cases like a numeric primary key, which is invalid + // as it should be floating point (int numerics are useless) + std::ostringstream err; + err << "PostGIS Plugin: Error: '" + << rs_key->getValue(0) + << "' on table '" + << geometry_table_ + << "' is not a valid integer primary key field\n"; + throw mapnik::datasource_exception(err.str()); } } else if (result_rows > 1) { - std::clog << "PostGIS Plugin: warning, multi column primary key detected but is not supported\n"; + std::ostringstream err; + err << "PostGIS Plugin: Error: '" + << "multi column primary key detected but is not supported"; + throw mapnik::datasource_exception(err.str()); } } -#ifdef MAPNIK_DEBUG - else - { - std::clog << "Postgis Plugin: no primary key could be detected for '" << geometry_table_ << "'\n"; - } -#endif rs_key->close(); } // if a globally unique key field/primary key is required // but still not known at this point, then throw - if (*require_key && key_field_.empty()) + if (*autodetect_key_field && key_field_.empty()) { - throw mapnik::datasource_exception(std::string("PostGIS Plugin: Error: primary key required for table '") + + throw mapnik::datasource_exception(std::string("PostGIS Plugin: Error: primary key required") + + " but could not be detected for table '" + geometry_table_ + "', please supply 'key_field' option to specify field to use for primary key"); } diff --git a/tests/python_tests/postgis_test.py b/tests/python_tests/postgis_test.py index 128d4c998..9c8604f3d 100644 --- a/tests/python_tests/postgis_test.py +++ b/tests/python_tests/postgis_test.py @@ -119,6 +119,11 @@ INSERT INTO "tableWithMixedCase"(geom) values (ST_MakePoint(1,0)); INSERT INTO "tableWithMixedCase"(geom) values (ST_MakePoint(1,1)); ''' +insert_table_7 = ''' +CREATE TABLE test6(first_id int4, second_id int4,PRIMARY KEY (first_id,second_id), geom geometry); +INSERT INTO test6(first_id, second_id, geom) values (0, 0, GeomFromEWKT('SRID=4326;POINT(0 0)')); +''' + def postgis_setup(): call('dropdb %s' % MAPNIK_TEST_DBNAME,silent=True) call('createdb -T %s %s' % (POSTGIS_TEMPLATE_DBNAME,MAPNIK_TEST_DBNAME),silent=False) @@ -130,6 +135,7 @@ def postgis_setup(): call('''psql -q %s -c "%s"''' % (MAPNIK_TEST_DBNAME,insert_table_4),silent=False) call('''psql -q %s -c "%s"''' % (MAPNIK_TEST_DBNAME,insert_table_5),silent=False) call("""psql -q %s -c '%s'""" % (MAPNIK_TEST_DBNAME,insert_table_6),silent=False) + call('''psql -q %s -c "%s"''' % (MAPNIK_TEST_DBNAME,insert_table_7),silent=False) def postgis_takedown(): pass @@ -235,7 +241,8 @@ if 'postgis' in mapnik.DatasourceCache.instance().plugin_names() \ def test_auto_detection_of_unique_feature_id_32_bit(): ds = mapnik.PostGIS(dbname=MAPNIK_TEST_DBNAME,table='test2', - geometry_field='geom') + geometry_field='geom', + autodetect_key_field=True) fs = ds.featureset() eq_(fs.next()['manual_id'],0) eq_(fs.next()['manual_id'],1) @@ -255,7 +262,7 @@ if 'postgis' in mapnik.DatasourceCache.instance().plugin_names() \ def test_auto_detection_will_fail_since_no_primary_key(): ds = mapnik.PostGIS(dbname=MAPNIK_TEST_DBNAME,table='test3', geometry_field='geom', - require_key=False) + autodetect_key_field=False) fs = ds.featureset() feat = fs.next() eq_(feat['manual_id'],0) @@ -281,13 +288,13 @@ if 'postgis' in mapnik.DatasourceCache.instance().plugin_names() \ def test_auto_detection_will_fail_and_should_throw(): ds = mapnik.PostGIS(dbname=MAPNIK_TEST_DBNAME,table='test3', geometry_field='geom', - require_key=True) + autodetect_key_field=True) fs = ds.featureset() def test_auto_detection_of_unique_feature_id_64_bit(): ds = mapnik.PostGIS(dbname=MAPNIK_TEST_DBNAME,table='test4', geometry_field='geom', - require_key=False) + autodetect_key_field=True) fs = ds.featureset() eq_(fs.next()['manual_id'],0) eq_(fs.next()['manual_id'],1) @@ -304,11 +311,73 @@ if 'postgis' in mapnik.DatasourceCache.instance().plugin_names() \ eq_(fs.next().id(),2147483647) eq_(fs.next().id(),-2147483648) + def test_disabled_auto_detection_and_subquery(): + ds = mapnik.PostGIS(dbname=MAPNIK_TEST_DBNAME,table='''(select geom, 'a'::varchar as name from test2) as t''', + geometry_field='geom', + autodetect_key_field=False) + fs = ds.featureset() + feat = fs.next() + eq_(feat.id(),1) + eq_(feat['name'],'a') + feat = fs.next() + eq_(feat.id(),2) + eq_(feat['name'],'a') + feat = fs.next() + eq_(feat.id(),3) + eq_(feat['name'],'a') + feat = fs.next() + eq_(feat.id(),4) + eq_(feat['name'],'a') + feat = fs.next() + eq_(feat.id(),5) + eq_(feat['name'],'a') + feat = fs.next() + eq_(feat.id(),6) + eq_(feat['name'],'a') + + def test_auto_detection_and_subquery_including_key(): + ds = mapnik.PostGIS(dbname=MAPNIK_TEST_DBNAME,table='''(select geom, manual_id from test2) as t''', + geometry_field='geom', + autodetect_key_field=True) + fs = ds.featureset() + eq_(fs.next()['manual_id'],0) + eq_(fs.next()['manual_id'],1) + eq_(fs.next()['manual_id'],1000) + eq_(fs.next()['manual_id'],-1000) + eq_(fs.next()['manual_id'],2147483647) + eq_(fs.next()['manual_id'],-2147483648) + + fs = ds.featureset() + eq_(fs.next().id(),0) + eq_(fs.next().id(),1) + eq_(fs.next().id(),1000) + eq_(fs.next().id(),-1000) + eq_(fs.next().id(),2147483647) + eq_(fs.next().id(),-2147483648) + + @raises(RuntimeError) + def test_auto_detection_of_invalid_numeric_primary_key(): + ds = mapnik.PostGIS(dbname=MAPNIK_TEST_DBNAME,table='''(select geom, manual_id::numeric from test2) as t''', + geometry_field='geom', + autodetect_key_field=True) + + @raises(RuntimeError) + def test_auto_detection_of_invalid_multiple_keys(): + ds = mapnik.PostGIS(dbname=MAPNIK_TEST_DBNAME,table='''test6''', + geometry_field='geom', + autodetect_key_field=True) + + @raises(RuntimeError) + def test_auto_detection_of_invalid_multiple_keys_subquery(): + ds = mapnik.PostGIS(dbname=MAPNIK_TEST_DBNAME,table='''(select first_id,second_id,geom from test6) as t''', + geometry_field='geom', + autodetect_key_field=True) + def test_manually_specified_feature_id_field(): ds = mapnik.PostGIS(dbname=MAPNIK_TEST_DBNAME,table='test4', geometry_field='geom', key_field='manual_id', - require_key=True) + autodetect_key_field=True) fs = ds.featureset() eq_(fs.next()['manual_id'],0) eq_(fs.next()['manual_id'],1) @@ -328,7 +397,7 @@ if 'postgis' in mapnik.DatasourceCache.instance().plugin_names() \ def test_numeric_type_feature_id_field(): ds = mapnik.PostGIS(dbname=MAPNIK_TEST_DBNAME,table='test5', geometry_field='geom', - require_key=False) + autodetect_key_field=False) fs = ds.featureset() eq_(fs.next()['manual_id'],-1) eq_(fs.next()['manual_id'],1) @@ -340,7 +409,7 @@ if 'postgis' in mapnik.DatasourceCache.instance().plugin_names() \ def test_querying_table_with_mixed_case(): ds = mapnik.PostGIS(dbname=MAPNIK_TEST_DBNAME,table='"tableWithMixedCase"', geometry_field='geom', - require_key=True) + autodetect_key_field=True) fs = ds.featureset() eq_(fs.next().id(),1) eq_(fs.next().id(),2) @@ -351,7 +420,7 @@ if 'postgis' in mapnik.DatasourceCache.instance().plugin_names() \ def test_querying_subquery_with_mixed_case(): ds = mapnik.PostGIS(dbname=MAPNIK_TEST_DBNAME,table='(SeLeCt * FrOm "tableWithMixedCase") as MixedCaseQuery', geometry_field='geom', - require_key=True) + autodetect_key_field=True) fs = ds.featureset() eq_(fs.next().id(),1) eq_(fs.next().id(),2) @@ -363,7 +432,7 @@ if 'postgis' in mapnik.DatasourceCache.instance().plugin_names() \ ds = mapnik.PostGIS(dbname=MAPNIK_TEST_DBNAME,table=''' (SeLeCt * FrOm "tableWithMixedCase" where geom && !bbox! ) as MixedCaseQuery''', geometry_field='geom', - require_key=True) + autodetect_key_field=True) fs = ds.featureset() eq_(fs.next().id(),1) eq_(fs.next().id(),2) @@ -375,7 +444,7 @@ if 'postgis' in mapnik.DatasourceCache.instance().plugin_names() \ ds = mapnik.PostGIS(dbname=MAPNIK_TEST_DBNAME,table=''' (SeLeCt * FrOm "tableWithMixedCase" where ST_Intersects(geom,!bbox!) ) as MixedCaseQuery''', geometry_field='geom', - require_key=True) + autodetect_key_field=True) fs = ds.featureset() eq_(fs.next().id(),1) eq_(fs.next().id(),2) @@ -387,4 +456,5 @@ if 'postgis' in mapnik.DatasourceCache.instance().plugin_names() \ if __name__ == "__main__": setup() + #test_auto_detection_and_subquery() [eval(run)() for run in dir() if 'test_' in run]