Skip to content

Commit

Permalink
Arrow/Parquet: fix writing empty point Z with GEOMETRY_ENCODING=GEOAR…
Browse files Browse the repository at this point in the history
…ROW_INTERLEAVED, and test spatial filtering of that encoding

Tests scenario of qgis/QGIS#57228
  • Loading branch information
rouault committed Apr 27, 2024
1 parent a401dcd commit 069d82c
Show file tree
Hide file tree
Showing 3 changed files with 146 additions and 9 deletions.
131 changes: 131 additions & 0 deletions autotest/ogr/ogr_parquet.py
Expand Up @@ -3680,6 +3680,137 @@ def check(lyr):
assert lyr.GetFeatureCount() != 0


###############################################################################
# Check GeoArrow fixed size list / interleaved encoding


@pytest.mark.parametrize(
"wkt",
[
"POINT (1 2)",
"POINT Z (1 2 3)",
"LINESTRING (1 2,3 4)",
"LINESTRING Z (1 2 3,4 5 6)",
"POLYGON ((0 1,2 3,10 20,0 1))",
"POLYGON ((0 0,0 10,10 10,10 0,0 0),(1 1,1 9,9 9,9 1,1 1))",
"POLYGON Z ((0 1 10,2 3 20,10 20 30,0 1 10))",
"MULTIPOINT ((1 2),(3 4))",
"MULTIPOINT Z ((1 2 3),(4 5 6))",
"MULTILINESTRING ((1 2,3 4),(5 6,7 8,9 10))",
"MULTILINESTRING Z ((1 2 3,4 5 6),(7 8 9,10 11 12,13 14 15))",
"MULTIPOLYGON (((0 1,2 3,10 20,0 1)),((100 110,100 120,120 120,100 110)))",
"MULTIPOLYGON (((0 0,0 10,10 10,10 0,0 0),(1 1,1 9,9 9,9 1,1 1)),((100 110,100 120,120 120,100 110)))",
"MULTIPOLYGON Z (((0 1 10,2 3 20,10 20 30,0 1 10)))",
],
)
@pytest.mark.parametrize("covering_bbox", [True, False])
@gdaltest.enable_exceptions()
def test_ogr_parquet_geoarrow_fixed_size_list(tmp_vsimem, wkt, covering_bbox):

geom = ogr.CreateGeometryFromWkt(wkt)

filename = str(tmp_vsimem / "test_ogr_parquet_geoarrow_fixed_size_list.parquet")

ds = ogr.GetDriverByName("Parquet").CreateDataSource(filename)

lyr = ds.CreateLayer(
"test",
geom_type=geom.GetGeometryType(),
options=[
"GEOMETRY_ENCODING=GEOARROW_INTERLEAVED",
"WRITE_COVERING_BBOX=" + ("YES" if covering_bbox else "NO"),
],
)
lyr.CreateField(ogr.FieldDefn("foo"))

# Nominal geometry
f = ogr.Feature(lyr.GetLayerDefn())
f.SetGeometry(geom)
lyr.CreateFeature(f)

# Null geometry
f = ogr.Feature(lyr.GetLayerDefn())
lyr.CreateFeature(f)

# Empty geometry
f = ogr.Feature(lyr.GetLayerDefn())
f.SetGeometry(ogr.Geometry(geom.GetGeometryType()))
lyr.CreateFeature(f)

# Nominal geometry
f = ogr.Feature(lyr.GetLayerDefn())
f.SetGeometry(geom)
lyr.CreateFeature(f)

geom2 = None
if geom.GetGeometryCount() > 1:
geom2 = geom.Clone()
geom2.RemoveGeometry(1)
f = ogr.Feature(lyr.GetLayerDefn())
f.SetGeometry(geom2)
lyr.CreateFeature(f)

ds = None

def check(lyr):
assert lyr.GetGeomType() == geom.GetGeometryType()

f = lyr.GetNextFeature()
ogrtest.check_feature_geometry(f, geom)

f = lyr.GetNextFeature()
if geom.GetGeometryType() in (ogr.wkbPoint, ogr.wkbPoint25D):
assert f.GetGeometryRef().IsEmpty()
else:
assert f.GetGeometryRef() is None

f = lyr.GetNextFeature()
ogrtest.check_feature_geometry(f, ogr.Geometry(geom.GetGeometryType()))

f = lyr.GetNextFeature()
ogrtest.check_feature_geometry(f, geom)

if geom2:
f = lyr.GetNextFeature()
ogrtest.check_feature_geometry(f, geom2)

ds = ogr.Open(filename)
lyr = ds.GetLayer(0)
check(lyr)

# Check that ignoring attribute fields doesn't impact geometry reading
ds = ogr.Open(filename)
lyr = ds.GetLayer(0)
lyr.SetIgnoredFields(["foo"])
check(lyr)

ds = ogr.Open(filename)
lyr = ds.GetLayer(0)
minx, maxx, miny, maxy = geom.GetEnvelope()

lyr.SetSpatialFilter(geom)
assert lyr.GetFeatureCount() == (3 if geom.GetGeometryCount() > 1 else 2)

lyr.SetSpatialFilterRect(maxx + 1, miny, maxx + 2, maxy)
assert lyr.GetFeatureCount() == 0

lyr.SetSpatialFilterRect(minx, maxy + 1, maxx, maxy + 2)
assert lyr.GetFeatureCount() == 0

lyr.SetSpatialFilterRect(minx - 2, miny, minx - 1, maxy)
assert lyr.GetFeatureCount() == 0

lyr.SetSpatialFilterRect(minx, miny - 2, maxx, miny - 1)
assert lyr.GetFeatureCount() == 0
if (
minx != miny
and maxx != maxy
and ogr.GT_Flatten(geom.GetGeometryType()) != ogr.wkbMultiPoint
):
lyr.SetSpatialFilterRect(minx + 0.1, miny + 0.1, maxx - 0.1, maxy - 0.1)
assert lyr.GetFeatureCount() != 0


###############################################################################
# Test reading a file with an extension on a regular field not registered with
# PyArrow
Expand Down
2 changes: 1 addition & 1 deletion ogr/ogrsf_frmts/arrow_common/ograrrowlayer.hpp
Expand Up @@ -833,7 +833,7 @@ static bool IsPointType(const std::shared_ptr<arrow::DataType> &type,
bHasZOut = false;
bHasMOut = true;
}
else if (osValueFieldName == "xyz")
else /* if (osValueFieldName == "xyz" || osValueFieldName == "element") */
{
bHasMOut = false;
bHasZOut = true;
Expand Down
22 changes: 14 additions & 8 deletions ogr/ogrsf_frmts/arrow_common/ograrrowwriterlayer.hpp
Expand Up @@ -1261,10 +1261,10 @@ inline OGRErr OGRArrowWriterLayer::BuildGeometry(OGRGeometry *poGeom,
std::numeric_limits<double>::quiet_NaN()));
OGR_ARROW_RETURN_OGRERR_NOT_OK(poValueBuilder->Append(
std::numeric_limits<double>::quiet_NaN()));
if (OGR_GT_HasZ(eGType))
if (bHasZ)
OGR_ARROW_RETURN_OGRERR_NOT_OK(poValueBuilder->Append(
std::numeric_limits<double>::quiet_NaN()));
if (OGR_GT_HasM(eGType))
if (bHasM)
OGR_ARROW_RETURN_OGRERR_NOT_OK(poValueBuilder->Append(
std::numeric_limits<double>::quiet_NaN()));
}
Expand Down Expand Up @@ -1364,20 +1364,26 @@ inline OGRErr OGRArrowWriterLayer::BuildGeometry(OGRGeometry *poGeom,
std::numeric_limits<double>::quiet_NaN()));
OGR_ARROW_RETURN_OGRERR_NOT_OK(poValueBuilder->Append(
std::numeric_limits<double>::quiet_NaN()));
if (bHasZ)
OGR_ARROW_RETURN_OGRERR_NOT_OK(poValueBuilder->Append(
std::numeric_limits<double>::quiet_NaN()));
if (bHasM)
OGR_ARROW_RETURN_OGRERR_NOT_OK(poValueBuilder->Append(
std::numeric_limits<double>::quiet_NaN()));
}
else
{
OGR_ARROW_RETURN_OGRERR_NOT_OK(
poValueBuilder->Append(poPoint->getX()));
OGR_ARROW_RETURN_OGRERR_NOT_OK(
poValueBuilder->Append(poPoint->getY()));
if (bHasZ)
OGR_ARROW_RETURN_OGRERR_NOT_OK(
poValueBuilder->Append(poPoint->getZ()));
if (bHasM)
OGR_ARROW_RETURN_OGRERR_NOT_OK(
poValueBuilder->Append(poPoint->getM()));
}
if (bHasZ)
OGR_ARROW_RETURN_OGRERR_NOT_OK(
poValueBuilder->Append(poPoint->getZ()));
if (bHasM)
OGR_ARROW_RETURN_OGRERR_NOT_OK(
poValueBuilder->Append(poPoint->getM()));
break;
}

Expand Down

0 comments on commit 069d82c

Please sign in to comment.