Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GPKG / SQLite dialect: improve detection of geometry columns when first row is NULL (fixes #8587) #8589

Merged
merged 1 commit into from
Oct 27, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Binary file not shown.
Binary file not shown.
25 changes: 25 additions & 0 deletions autotest/ogr/ogr_gpkg.py
Original file line number Diff line number Diff line change
Expand Up @@ -9421,3 +9421,28 @@ def test_ogr_gpkg_write_arrow_fallback_types(tmp_vsimem):
assert f["int64list"] == "[ 12345678901234, 2 ]"
assert f["reallist"] == "[ 1.5, 2.5 ]"
assert f.GetGeometryRef().ExportToIsoWkt() == "POINT (1 2)"


###############################################################################
# Test a SQL request with the geometry in the first row being null


def test_ogr_gpkg_sql_first_geom_null():

ds = ogr.Open("data/gpkg/first_geometry_null.gpkg")
if not _has_spatialite_4_3_or_later(ds):
pytest.skip("spatialite missing")
with ds.ExecuteSQL("SELECT ST_Buffer(geom,0.1) FROM test") as sql_lyr:
assert sql_lyr.GetGeometryColumn() == "ST_Buffer(geom,0.1)"
with ds.ExecuteSQL("SELECT ST_Buffer(geom,0.1), * FROM test") as sql_lyr:
assert sql_lyr.GetGeometryColumn() == "ST_Buffer(geom,0.1)"
with ds.ExecuteSQL("SELECT ST_Buffer(geom,0.5) AS geom FROM test") as sql_lyr:
assert sql_lyr.GetGeometryColumn() == "geom"
sql_lyr.GetNextFeature()
f = sql_lyr.GetNextFeature()
assert f.GetGeometryRef().GetGeometryType() == ogr.wkbPolygon
with ds.ExecuteSQL("SELECT ST_Buffer(geom,0.5) AS geom, * FROM test") as sql_lyr:
assert sql_lyr.GetGeometryColumn() == "geom"
sql_lyr.GetNextFeature()
f = sql_lyr.GetNextFeature()
assert f.GetGeometryRef().GetGeometryType() == ogr.wkbPolygon
13 changes: 13 additions & 0 deletions autotest/ogr/ogr_sqlite.py
Original file line number Diff line number Diff line change
Expand Up @@ -3954,3 +3954,16 @@ def test_ogr_sqlite_delete(sqlite_test_db):

lyr = sqlite_test_db.GetLayer("tpoly")
assert lyr is None


###############################################################################
# Test a SQL request with the geometry in the first row being null


def test_ogr_sql_sql_first_geom_null(require_spatialite):

ds = ogr.Open("data/sqlite/first_geometry_null.db")
with ds.ExecuteSQL("SELECT ST_Buffer(geom,0.1) FROM test") as sql_lyr:
assert sql_lyr.GetGeometryColumn() == "ST_Buffer(geom,0.1)"
with ds.ExecuteSQL("SELECT ST_Buffer(geom,0.1), * FROM test") as sql_lyr:
assert sql_lyr.GetGeometryColumn() == "ST_Buffer(geom,0.1)"
10 changes: 10 additions & 0 deletions doc/source/drivers/vector/gpkg.rst
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,16 @@ binary encoding must be done.

Starting with Spatialite 4.3, CastAutomagic is no longer needed.

Note that due to the loose typing mechanism of SQLite, if a geometry expression
returns a NULL value for the first row, this will generally cause OGR not to
recognize the column as a geometry column. It might be then useful to sort
the results by making sure that non-null geometries are returned first:

::

ogrinfo poly.gpkg -sql "SELECT * FROM (SELECT ST_Buffer(geom,5) AS geom, * FROM poly) ORDER BY geom IS NULL ASC"


Transaction support
-------------------

Expand Down
9 changes: 9 additions & 0 deletions doc/source/user/sql_sqlite_dialect.rst
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,15 @@ returns:
EAS_ID (Real) = 170
area (Real) = 5268.8125

Note that due to the loose typing mechanism of SQLite, if a geometry expression
returns a NULL value for the first row, this will generally cause OGR not to
recognize the column as a geometry column. It might be then useful to sort
the results by making sure that non-null geometries are returned first:

::

ogrinfo test.shp -sql "SELECT * FROM (SELECT ST_Buffer(geometry,5) AS geometry FROM test) ORDER BY geometry IS NULL ASC" -dialect sqlite

OGR datasource SQL functions
++++++++++++++++++++++++++++

Expand Down
46 changes: 38 additions & 8 deletions ogr/ogrsf_frmts/gpkg/ogrgeopackagelayer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -947,6 +947,7 @@ void OGRGeoPackageLayer::BuildFeatureDefn(const char *pszLayerName,
}
#endif

bool bGeometryColumnGuessed = false;
for (int iCol = 0; iCol < nRawColumns; iCol++)
{
OGRFieldDefn oField(SQLUnescape(sqlite3_column_name(hStmt, iCol)),
Expand Down Expand Up @@ -983,14 +984,19 @@ void OGRGeoPackageLayer::BuildFeatureDefn(const char *pszLayerName,
OGRLayer *poLayer = m_poDS->GetLayerByName(pszTableName);
if (poLayer != nullptr)
{
if (m_poFeatureDefn->GetGeomFieldCount() == 0 &&
EQUAL(pszOriginName, poLayer->GetGeometryColumn()))
if (EQUAL(pszOriginName, poLayer->GetGeometryColumn()))
{
OGRGeomFieldDefn oGeomField(
poLayer->GetLayerDefn()->GetGeomFieldDefn(0));
oGeomField.SetName(oField.GetNameRef());
m_poFeatureDefn->AddGeomFieldDefn(&oGeomField);
m_iGeomCol = iCol;
if (bGeometryColumnGuessed ||
m_poFeatureDefn->GetGeomFieldCount() == 0)
{
if (bGeometryColumnGuessed)
m_poFeatureDefn->DeleteGeomFieldDefn(0);
OGRGeomFieldDefn oGeomField(
poLayer->GetLayerDefn()->GetGeomFieldDefn(0));
oGeomField.SetName(oField.GetNameRef());
m_poFeatureDefn->AddGeomFieldDefn(&oGeomField);
m_iGeomCol = iCol;
}
continue;
}
else if (EQUAL(pszOriginName, poLayer->GetFIDColumn()) &&
Expand Down Expand Up @@ -1021,14 +1027,38 @@ void OGRGeoPackageLayer::BuildFeatureDefn(const char *pszLayerName,
#endif

const int nColType = sqlite3_column_type(hStmt, iCol);
if (m_pszFidColumn == nullptr && nColType == SQLITE_INTEGER &&
if (m_poFeatureDefn->GetGeomFieldCount() == 0 &&
m_pszFidColumn == nullptr && nColType == SQLITE_INTEGER &&
EQUAL(oField.GetNameRef(), "FID"))
{
m_pszFidColumn = CPLStrdup(oField.GetNameRef());
m_iFIDCol = iCol;
continue;
}

// Heuristics to help for https://github.com/OSGeo/gdal/issues/8587
if (nColType == SQLITE_NULL && m_iGeomCol < 0
#ifdef SQLITE_HAS_COLUMN_METADATA
&& !pszTableName && !pszOriginName
#endif
)
{
bool bIsLikelyGeomColName = EQUAL(oField.GetNameRef(), "geom") ||
EQUAL(oField.GetNameRef(), "geometry");
bool bIsGeomFunction = false;
if (!bIsLikelyGeomColName)
bIsGeomFunction = OGRSQLiteIsSpatialFunctionReturningGeometry(
oField.GetNameRef());
if (bIsLikelyGeomColName || bIsGeomFunction)
{
bGeometryColumnGuessed = bIsLikelyGeomColName;
OGRGeomFieldDefn oGeomField(oField.GetNameRef(), wkbUnknown);
m_poFeatureDefn->AddGeomFieldDefn(&oGeomField);
m_iGeomCol = iCol;
continue;
}
}

const char *pszDeclType = sqlite3_column_decltype(hStmt, iCol);

// Recognize a geometry column from trying to build the geometry
Expand Down
12 changes: 12 additions & 0 deletions ogr/ogrsf_frmts/sqlite/ogrsqlitelayer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -606,6 +606,18 @@ void OGRSQLiteLayer::BuildFeatureDefn(const char *pszLayerName, bool bIsSelect,
}
}
}
else if (paosGeomCols == nullptr && nColType == SQLITE_NULL &&
!STARTS_WITH_CI(pszFieldName, "Gpkg") &&
!STARTS_WITH_CI(pszFieldName, "AsGPB(") &&
!STARTS_WITH_CI(pszFieldName, "CastAutomagic(") &&
OGRSQLiteIsSpatialFunctionReturningGeometry(pszFieldName))
{
auto poGeomFieldDefn =
cpl::make_unique<OGRSQLiteGeomFieldDefn>(pszFieldName, iCol);
poGeomFieldDefn->m_eGeomFormat = OSGF_SpatiaLite;
m_poFeatureDefn->AddGeomFieldDefn(std::move(poGeomFieldDefn));
continue;
}

// SpatialLite / Gaia
if (paosGeomCols == nullptr && EQUAL(pszFieldName, "GaiaGeometry") &&
Expand Down
142 changes: 142 additions & 0 deletions ogr/ogrsf_frmts/sqlite/ogrsqliteutility.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -598,3 +598,145 @@ bool OGRSQLiteRTreeRequiresTrustedSchemaOn()
}();
return b;
}

/************************************************************************/
/* OGRSQLiteIsSpatialFunctionReturningGeometry() */
/************************************************************************/

bool OGRSQLiteIsSpatialFunctionReturningGeometry(const char *pszName)
{
const char *const apszFunctions[] = {
"SetSRID(",
"IsValidDetail(",
"Boundary(",
"Envelope(",
"ST_Expand(",
"ST_Reverse(",
"ST_ForceLHR(",
"ST_ForcePolygonCW(",
"ST_ForcePolygonCCW(",
"SanitizeGeometry(",
"EnsureClosedRings(",
"RemoveRepeatedPoints(",
"CastToPoint(",
"CastToLinestring(",
"CastToPolygon(",
"CastToMultiPoint(",
"CastToMultiLinestring(",
"CastToMultiPolygon(",
"CastToGeometryCollection(",
"CastToMulti(",
"ST_Multi(",
"CastToSingle(",
"CastToXY(",
"CastToXYZ(",
"CastToXYM(",
"CastToXYZM(",
"StartPoint(",
"ST_EndPoint(",
"PointOnSurface(",
"Simplify(",
"ST_Generalize(",
"SimplifyPreserveTopology(",
"PointN(",
"AddPoint(",
"SetPoint(",
"SetStartPoint(",
"SetEndPoint(",
"RemovePoint(",
"Centroid(",
"ExteriorRing(",
"InteriorRingN(",
"GeometryN(",
"ST_AddMeasure(",
"ST_Locate_Along_Measure(",
"ST_LocateAlong(",
"ST_Locate_Between_Measures(",
"ST_LocateBetween(",
"ST_TrajectoryInterpolarePoint(",
"Intersection(",
"Difference(",
"GUnion(",
"ST_Union(", // UNION is not a valid function name
"SymDifference(",
"Buffer(",
"ConvexHull(",
"OffsetCurve(",
"SingleSidedBuffer(",
"SharedPaths(",
"Line_Interpolate_Point(",
"Line_Interpolate_Equidistant_Points(",
"Line_Substring(",
"ClosestPoint(",
"ShortestLine(",
"Snap(",
"Collect(",
"LineMerge(",
"BuildArea(",
"Polygonize(",
"MakePolygon(",
"UnaryUnion(",
"UnaryUnion(",
"DrapeLine(",
"DrapeLineExceptions(",
"DissolveSegments(",
"DissolvePoints(",
"LinesFromRings(",
"LinesCutAtNodes(",
"RingsCutAtNodes(",
"CollectionExtract(",
"ExtractMultiPoint(",
"ExtractMultiLinestring(",
"ExtractMultiPolygon(",
"DelaunayTriangulation(",
"VoronojDiagram(",
"ConcaveHull(",
"MakeValid(",
"MakeValidDiscarded(",
"Segmentize(",
"Split(",
"SplitLeft(",
"SplitRight(",
"SnapAndSplit(",
"Project(",
"SnapToGrid(",
"ST_Node(",
"SelfIntersections(",
"ST_Subdivide(",
"Transform(",
"TransformXY(",
"TransformXYZ(",
"ShiftCoords(",
"ShiftCoordinates(",
"ST_Translate(",
"ST_Shift_Longitude(",
"NormalizeLonLat(",
"ScaleCoords(",
"ScaleCoordinates(",
"RotateCoords(",
"RotateCoordinates(",
"ReflectCoords(",
"ReflectCoordinates(",
"SwapCoords(",
"SwapCoordinates(",
"ATM_Transform(",
"gpkgMakePoint(",
"gpkgMakePointZ(",
"gpkgMakePointZM(",
"gpkgMakePointM(",
"AsGPB(",
"GeomFromGPB(",
"CastAutomagic(",
};
for (const char *pszFunction : apszFunctions)
{
if (STARTS_WITH_CI(pszName, pszFunction) ||
(!STARTS_WITH_CI(pszFunction, "ST_") &&
STARTS_WITH_CI(pszName, "ST_") &&
STARTS_WITH_CI(pszName + strlen("ST_"), pszFunction)))
{
return true;
}
}
return false;
}
2 changes: 2 additions & 0 deletions ogr/ogrsf_frmts/sqlite/ogrsqliteutility.h
Original file line number Diff line number Diff line change
Expand Up @@ -100,4 +100,6 @@ std::set<std::string> SQLGetUniqueFieldUCConstraints(

bool OGRSQLiteRTreeRequiresTrustedSchemaOn();

bool OGRSQLiteIsSpatialFunctionReturningGeometry(const char *pszName);

#endif // OGR_SQLITEUTILITY_H_INCLUDED
Loading