diff --git a/MIGRATION_GUIDE.TXT b/MIGRATION_GUIDE.TXT index c7d011677e4c..f1155642d493 100644 --- a/MIGRATION_GUIDE.TXT +++ b/MIGRATION_GUIDE.TXT @@ -15,6 +15,9 @@ MIGRATION GUIDE FROM GDAL 3.9 to GDAL 3.10 corresponding optional (but recommended to be implemented to reliably detect reading errors) callbacks "error" and "clear_err". +- Python bindings: Band.GetStatistics() and Band.ComputeStatistics() now + return a None value in case of error (when exceptions are not enabled) + MIGRATION GUIDE FROM GDAL 3.8 to GDAL 3.9 ----------------------------------------- diff --git a/autotest/gcore/gdal_stats.py b/autotest/gcore/gdal_stats.py index 523c6459621f..227d268d64de 100755 --- a/autotest/gcore/gdal_stats.py +++ b/autotest/gcore/gdal_stats.py @@ -99,7 +99,7 @@ def test_stats_dont_force(): gdal.Unlink("data/byte.tif.aux.xml") ds = gdal.Open("data/byte.tif") stats = ds.GetRasterBand(1).GetStatistics(0, 0) - assert stats == [0, 0, 0, -1], "did not get expected stats" + assert stats is None ############################################################################### @@ -762,7 +762,7 @@ def test_stats_approx_stats_flag(dt=gdal.GDT_Byte, struct_frmt="B"): approx_ok = 0 force = 0 stats = ds.GetRasterBand(1).GetStatistics(approx_ok, force) - assert stats == [0.0, 0.0, 0.0, -1.0], "did not get expected stats" + assert stats is None approx_ok = 0 force = 1 @@ -824,15 +824,15 @@ def test_stats_clear(): filename = "/vsimem/out.tif" gdal.Translate(filename, "data/byte.tif") ds = gdal.Open(filename) - assert ds.GetRasterBand(1).GetStatistics(False, False) == [0, 0, 0, -1] - assert ds.GetRasterBand(1).ComputeStatistics(False) != [0, 0, 0, -1] + assert ds.GetRasterBand(1).GetStatistics(False, False) is None + assert ds.GetRasterBand(1).ComputeStatistics(False) is not None ds = gdal.Open(filename) - assert ds.GetRasterBand(1).GetStatistics(False, False) != [0, 0, 0, -1] + assert ds.GetRasterBand(1).GetStatistics(False, False) is not None ds.ClearStatistics() ds = gdal.Open(filename) - assert ds.GetRasterBand(1).GetStatistics(False, False) == [0, 0, 0, -1] + assert ds.GetRasterBand(1).GetStatistics(False, False) is None gdal.GetDriverByName("GTiff").Delete(filename) diff --git a/autotest/gcore/pam.py b/autotest/gcore/pam.py index 2b4403123f04..0af14cd16b38 100755 --- a/autotest/gcore/pam.py +++ b/autotest/gcore/pam.py @@ -382,7 +382,7 @@ def test_pam_11(): # Check that we actually have no saved statistics ds = gdal.Open("tmpdirreadonly/byte.tif") stats = ds.GetRasterBand(1).GetStatistics(False, False) - assert stats[3] == -1, "did not expected to have stats at that point" + assert stats is None ds = None # This must be run as an external process so we can override GDAL_PAM_PROXY_DIR diff --git a/autotest/gcore/vrt_read.py b/autotest/gcore/vrt_read.py index 00c068f165b4..6a67274a971d 100755 --- a/autotest/gcore/vrt_read.py +++ b/autotest/gcore/vrt_read.py @@ -2341,7 +2341,7 @@ def test_vrt_read_compute_statistics_mosaic_optimization_src_with_nodata_all(): with gdal.quiet_errors(): vrt_stats = vrt_ds.GetRasterBand(1).ComputeStatistics(False) - assert vrt_stats == [0, 0, 0, 0] + assert vrt_stats is None assert vrt_ds.GetRasterBand(1).GetMetadataItem("STATISTICS_MINIMUM") is None diff --git a/autotest/gdrivers/ecw.py b/autotest/gdrivers/ecw.py index 008057c1c11e..d0407441e7fd 100755 --- a/autotest/gdrivers/ecw.py +++ b/autotest/gdrivers/ecw.py @@ -1479,7 +1479,7 @@ def test_ecw_41(tmp_path): # Check that no statistics is already included in the file assert ds.GetRasterBand(1).GetMinimum() is None assert ds.GetRasterBand(1).GetMaximum() is None - assert ds.GetRasterBand(1).GetStatistics(1, 0) == [0.0, 0.0, 0.0, -1.0] + assert ds.GetRasterBand(1).GetStatistics(1, 0) is None assert ds.GetRasterBand(1).GetDefaultHistogram(force=0) is None # Now compute the stats diff --git a/autotest/gdrivers/ehdr.py b/autotest/gdrivers/ehdr.py index 8165192b8f55..6fb4b75b747f 100755 --- a/autotest/gdrivers/ehdr.py +++ b/autotest/gdrivers/ehdr.py @@ -379,20 +379,20 @@ def test_ehdr_approx_stats_flag(): approx_ok = 1 force = 1 stats = ds.GetRasterBand(1).GetStatistics(approx_ok, force) - assert stats == [0.0, 0.0, 0.0, 0.0], "did not get expected stats" + assert stats == [0.0, 0.0, 0.0, 0.0] md = ds.GetRasterBand(1).GetMetadata() assert "STATISTICS_APPROXIMATE" in md, "did not get expected metadata" approx_ok = 0 force = 0 stats = ds.GetRasterBand(1).GetStatistics(approx_ok, force) - assert stats == [0.0, 0.0, 0.0, -1.0], "did not get expected stats" + assert stats is None ds = gdal.Open(tmpfile, gdal.GA_Update) approx_ok = 0 force = 0 stats = ds.GetRasterBand(1).GetStatistics(approx_ok, force) - assert stats == [0.0, 0.0, 0.0, -1.0], "did not get expected stats" + assert stats is None approx_ok = 0 force = 1 diff --git a/autotest/gdrivers/gti.py b/autotest/gdrivers/gti.py index 13066179ef91..8888103c4b7e 100755 --- a/autotest/gdrivers/gti.py +++ b/autotest/gdrivers/gti.py @@ -34,6 +34,7 @@ import struct import gdaltest +import ogrtest import pytest from osgeo import gdal, ogr @@ -1057,6 +1058,20 @@ def test_gti_rgb_left_right(tmp_vsimem): == "/vsimem/test_gti_rgb_left_right/left.tif" ) + if ogrtest.have_geos(): + (flags, pct) = vrt_ds.GetRasterBand(1).GetDataCoverageStatus( + 0, 0, vrt_ds.RasterXSize, vrt_ds.RasterYSize + ) + assert flags == gdal.GDAL_DATA_COVERAGE_STATUS_DATA and pct == 100.0 + + (flags, pct) = vrt_ds.GetRasterBand(1).GetDataCoverageStatus(1, 2, 3, 4) + assert flags == gdal.GDAL_DATA_COVERAGE_STATUS_DATA and pct == 100.0 + + (flags, pct) = vrt_ds.GetRasterBand(1).GetDataCoverageStatus( + vrt_ds.RasterXSize // 2 - 1, 2, 2, 4 + ) + assert flags == gdal.GDAL_DATA_COVERAGE_STATUS_DATA and pct == 100.0 + def test_gti_overlapping_sources(tmp_vsimem): @@ -1082,6 +1097,12 @@ def test_gti_overlapping_sources(tmp_vsimem): vrt_ds = gdal.Open(index_filename) assert vrt_ds.GetRasterBand(1).Checksum() == 2 + if ogrtest.have_geos(): + (flags, pct) = vrt_ds.GetRasterBand(1).GetDataCoverageStatus( + 0, 0, vrt_ds.RasterXSize, vrt_ds.RasterYSize + ) + assert flags == gdal.GDAL_DATA_COVERAGE_STATUS_DATA and pct == 100.0 + # Test unsupported sort_field_type = OFTBinary index_filename = str(tmp_vsimem / "index.gti.gpkg") sort_values = [None, None] @@ -1319,6 +1340,41 @@ def test_gti_overlapping_sources(tmp_vsimem): assert vrt_ds.GetRasterBand(1).Checksum() == 2, sort_values +def test_gti_gap_between_sources(tmp_vsimem): + + filename1 = str(tmp_vsimem / "one.tif") + ds = gdal.GetDriverByName("GTiff").Create(filename1, 1, 1) + ds.SetGeoTransform([2, 1, 0, 49, 0, -1]) + ds.GetRasterBand(1).Fill(1) + del ds + + filename2 = str(tmp_vsimem / "two.tif") + ds = gdal.GetDriverByName("GTiff").Create(filename2, 1, 1) + ds.SetGeoTransform([4, 1, 0, 49, 0, -1]) + ds.GetRasterBand(1).Fill(2) + del ds + + index_filename = str(tmp_vsimem / "index.gti.gpkg") + index_ds, _ = create_basic_tileindex( + index_filename, [gdal.Open(filename1), gdal.Open(filename2)] + ) + del index_ds + + vrt_ds = gdal.Open(index_filename) + assert vrt_ds.GetRasterBand(1).Checksum() == 3 + + if ogrtest.have_geos(): + (flags, pct) = vrt_ds.GetRasterBand(1).GetDataCoverageStatus( + 0, 0, vrt_ds.RasterXSize, vrt_ds.RasterYSize + ) + assert ( + flags + == gdal.GDAL_DATA_COVERAGE_STATUS_DATA + | gdal.GDAL_DATA_COVERAGE_STATUS_EMPTY + and pct == pytest.approx(100.0 * 2 / 3) + ) + + def test_gti_no_source(tmp_vsimem): index_filename = str(tmp_vsimem / "index.gti.gpkg") @@ -1359,6 +1415,12 @@ def test_gti_no_source(tmp_vsimem): is None ) + if ogrtest.have_geos(): + (flags, pct) = vrt_ds.GetRasterBand(1).GetDataCoverageStatus( + 0, 0, vrt_ds.RasterXSize, vrt_ds.RasterYSize + ) + assert flags == gdal.GDAL_DATA_COVERAGE_STATUS_EMPTY and pct == 0.0 + def test_gti_invalid_source(tmp_vsimem): diff --git a/autotest/gdrivers/netcdf_multidim.py b/autotest/gdrivers/netcdf_multidim.py index bc9ea089c208..71585b3c04f5 100755 --- a/autotest/gdrivers/netcdf_multidim.py +++ b/autotest/gdrivers/netcdf_multidim.py @@ -3979,12 +3979,7 @@ def test(): view = ar.GetView("[0:10,...]") classic_ds = view.AsClassicDataset(1, 0) - assert classic_ds.GetRasterBand(1).GetStatistics(False, False) == [ - 0.0, - 0.0, - 0.0, - -1.0, - ] + assert classic_ds.GetRasterBand(1).GetStatistics(False, False) is None classic_ds.GetRasterBand(1).ComputeStatistics(False) view = ar.GetView("[10:20,...]") @@ -4035,12 +4030,7 @@ def reopen(): ) classic_ds = ar.AsClassicDataset(1, 0) - assert classic_ds.GetRasterBand(1).GetStatistics(False, False) == [ - 0.0, - 0.0, - 0.0, - -1.0, - ] + assert classic_ds.GetRasterBand(1).GetStatistics(False, False) is None rg_subset = rg.SubsetDimensionFromSelection("/x=440750") diff --git a/autotest/gdrivers/nitf.py b/autotest/gdrivers/nitf.py index a24070cbdc97..40909532991e 100755 --- a/autotest/gdrivers/nitf.py +++ b/autotest/gdrivers/nitf.py @@ -1488,8 +1488,7 @@ def test_nitf_36(): ds.GetRasterBand(1).GetMinimum() is None ), "Did not expect to have minimum value at that point." - (_, _, mean, stddev) = ds.GetRasterBand(1).GetStatistics(False, False) - assert stddev < 0, "Did not expect to have statistics at that point." + assert ds.GetRasterBand(1).GetStatistics(False, False) is None (exp_mean, exp_stddev) = (65.4208, 47.254550335) (_, _, mean, stddev) = ds.GetRasterBand(1).GetStatistics(False, True) diff --git a/autotest/gdrivers/vrtprocesseddataset.py b/autotest/gdrivers/vrtprocesseddataset.py index 1c353d170732..11033bf373f7 100755 --- a/autotest/gdrivers/vrtprocesseddataset.py +++ b/autotest/gdrivers/vrtprocesseddataset.py @@ -1181,7 +1181,7 @@ def test_vrtprocesseddataset_serialize(tmp_vsimem): with gdaltest.tempfile(vrt_filename, content): ds = gdal.Open(vrt_filename) np.testing.assert_equal(ds.GetRasterBand(1).ReadAsArray(), np.array([[11, 12]])) - assert ds.GetRasterBand(1).GetStatistics(False, False) == [0.0, 0.0, 0.0, -1.0] + assert ds.GetRasterBand(1).GetStatistics(False, False) is None ds.GetRasterBand(1).ComputeStatistics(False) ds.Close() diff --git a/autotest/ogr/ogr_geom.py b/autotest/ogr/ogr_geom.py index df26bc55037a..39cd29a1db86 100755 --- a/autotest/ogr/ogr_geom.py +++ b/autotest/ogr/ogr_geom.py @@ -982,50 +982,38 @@ def test_ogr_geom_flattenTo2D_triangle(): ############################################################################### +@gdaltest.enable_exceptions() def test_ogr_geom_linestring_limits(): geom = ogr.CreateGeometryFromWkt("LINESTRING EMPTY") assert geom.Length() == 0 - gdal.ErrorReset() - with gdal.quiet_errors(): + with pytest.raises(Exception): geom.GetPoint(-1) - assert gdal.GetLastErrorType() != 0 - gdal.ErrorReset() - with gdal.quiet_errors(): + with pytest.raises(Exception): geom.GetPoint(0) - assert gdal.GetLastErrorType() != 0 - gdal.ErrorReset() - with gdal.quiet_errors(): + with pytest.raises(Exception): geom.GetPoint_2D(-1) - assert gdal.GetLastErrorType() != 0 - gdal.ErrorReset() - with gdal.quiet_errors(): + with pytest.raises(Exception): geom.GetPoint_2D(0) - assert gdal.GetLastErrorType() != 0 - gdal.ErrorReset() - with gdal.quiet_errors(): + with pytest.raises(Exception): geom.SetPoint(-1, 5, 6, 7) - assert gdal.GetLastErrorType() != 0 - gdal.ErrorReset() - with gdal.quiet_errors(): + with pytest.raises(Exception): geom.SetPoint_2D(-1, 5, 6) - assert gdal.GetLastErrorType() != 0 - gdal.ErrorReset() - with gdal.quiet_errors(): - geom.SetPoint(2147000000, 5, 6, 7) - assert gdal.GetLastErrorType() != 0 + with pytest.raises(Exception): + geom.SetPoint((1 << 31) - 2, 5, 6, 7) - gdal.ErrorReset() - with gdal.quiet_errors(): - geom.SetPoint_2D(2147000000, 5, 6) - assert gdal.GetLastErrorType() != 0 + with pytest.raises(Exception): + geom.SetPoint_2D((1 << 31) - 2, 5, 6) + + with pytest.raises(Exception): + geom.SetPoint_2D((1 << 31) - 1, 5, 6) geom = ogr.CreateGeometryFromWkt("LINESTRING(0 0)") assert geom.Length() == 0 diff --git a/autotest/ogr/ogr_mem.py b/autotest/ogr/ogr_mem.py index a308eac2a1c9..0002d2c7487d 100755 --- a/autotest/ogr/ogr_mem.py +++ b/autotest/ogr/ogr_mem.py @@ -1464,6 +1464,18 @@ def test_ogr_mem_write_arrow(): field_def = ogr.FieldDefn("field_stringlist", ogr.OFTStringList) src_lyr.CreateField(field_def) + field_def = ogr.FieldDefn("field_json", ogr.OFTString) + field_def.SetSubType(ogr.OFSTJSON) + src_lyr.CreateField(field_def) + + field_def = ogr.FieldDefn("field_uuid", ogr.OFTString) + field_def.SetSubType(ogr.OFSTUUID) + src_lyr.CreateField(field_def) + + field_def = ogr.FieldDefn("field_with_width", ogr.OFTString) + field_def.SetWidth(10) + src_lyr.CreateField(field_def) + feat_def = src_lyr.GetLayerDefn() src_feature = ogr.Feature(feat_def) src_feature.SetField("field_bool", True) @@ -1485,6 +1497,9 @@ def test_ogr_mem_write_arrow(): src_feature.field_float32list = [1.5, -1.5] src_feature.field_reallist = [123.5, 567.0] src_feature.field_stringlist = ["abc", "def"] + src_feature["field_json"] = '{"foo":"bar"}' + src_feature["field_uuid"] = "INVALID_UUID" + src_feature["field_with_width"] = "foo" src_feature.SetGeometry(ogr.CreateGeometryFromWkt("POINT (1 2)")) src_lyr.CreateFeature(src_feature) @@ -1506,6 +1521,15 @@ def test_ogr_mem_write_arrow(): if schema.GetChild(i).GetName() != "wkb_geometry": dst_lyr.CreateFieldFromArrowSchema(schema.GetChild(i)) + idx = dst_lyr.GetLayerDefn().GetFieldIndex("field_json") + assert dst_lyr.GetLayerDefn().GetFieldDefn(idx).GetSubType() == ogr.OFSTJSON + + idx = dst_lyr.GetLayerDefn().GetFieldIndex("field_uuid") + assert dst_lyr.GetLayerDefn().GetFieldDefn(idx).GetSubType() == ogr.OFSTUUID + + idx = dst_lyr.GetLayerDefn().GetFieldIndex("field_with_width") + assert dst_lyr.GetLayerDefn().GetFieldDefn(idx).GetWidth() == 10 + while True: array = stream.GetNextRecordBatch() if array is None: @@ -2851,6 +2875,24 @@ def test_ogr_mem_write_pyarrow_invalid_dict_index(dict_values): lyr.WritePyArrow(table) +############################################################################### + + +def test_ogr_mem_arrow_json(): + pytest.importorskip("pyarrow") + + ds = ogr.GetDriverByName("Memory").CreateDataSource("") + lyr = ds.CreateLayer("foo") + field_def = ogr.FieldDefn("field_json", ogr.OFTString) + field_def.SetSubType(ogr.OFSTJSON) + lyr.CreateField(field_def) + + stream = lyr.GetArrowStreamAsPyArrow() + md = stream.schema["field_json"].metadata + assert b"ARROW:extension:name" in md + assert md[b"ARROW:extension:name"] == b"arrow.json" + + ############################################################################### # Test Layer.GetDataset() diff --git a/autotest/ogr/ogr_parquet.py b/autotest/ogr/ogr_parquet.py index 8d09e1c861b0..0d3d268cc2a7 100755 --- a/autotest/ogr/ogr_parquet.py +++ b/autotest/ogr/ogr_parquet.py @@ -4044,6 +4044,30 @@ def test_ogr_parquet_read_arrow_json_extension(): assert f["extension_json"] == '{"foo":"bar"}' +############################################################################### +# Test writing a file with the arrow.json extension + + +def test_ogr_parquet_writing_arrow_json_extension(tmp_vsimem): + + outfilename = str(tmp_vsimem / "out.parquet") + with ogr.GetDriverByName("Parquet").CreateDataSource(outfilename) as ds: + lyr = ds.CreateLayer("test") + fld_defn = ogr.FieldDefn("extension_json") + fld_defn.SetSubType(ogr.OFSTJSON) + lyr.CreateField(fld_defn) + f = ogr.Feature(lyr.GetLayerDefn()) + f["extension_json"] = '{"foo":"bar"}' + lyr.CreateFeature(f) + + with gdal.config_option("OGR_PARQUET_READ_GDAL_SCHEMA", "NO"): + ds = ogr.Open(outfilename) + lyr = ds.GetLayer(0) + assert lyr.GetLayerDefn().GetFieldDefn(0).GetSubType() == ogr.OFSTJSON + f = lyr.GetNextFeature() + assert f["extension_json"] == '{"foo":"bar"}' + + ############################################################################### # Test ignored fields with arrow::dataset and bounding box column diff --git a/doc/source/user/raster_data_model.rst b/doc/source/user/raster_data_model.rst index e8e9d5bba92b..4800d7f713c4 100644 --- a/doc/source/user/raster_data_model.rst +++ b/doc/source/user/raster_data_model.rst @@ -136,7 +136,7 @@ The value of the _NAME is the string that can be passed to :cpp:func:`GDALOpen` Drivers which support subdatasets advertise the ``DMD_SUBDATASETS`` capability. This information is reported when the --format and --formats options are passed to the command line utilities. -Currently, drivers which support subdatasets are: ADRG, ECRGTOC, GEORASTER, GTiff, HDF4, HDF5, netCDF, NITF, NTv2, OGDI, PDF, PostGISRaster, Rasterlite, RPFTOC, RS2, TileDB, WCS, and WMS. +Currently, drivers which support subdatasets are: ADRG, ECRGTOC, GEORASTER, GTiff, HDF4, HDF5, netCDF, NITF, NTv2, OGDI, PDF, PostGISRaster, Rasterlite, RPFTOC, RS2, TileDB, WCS, WMS, and Zarr. IMAGE_STRUCTURE Domain ++++++++++++++++++++++ diff --git a/frmts/vrt/gdaltileindexdataset.cpp b/frmts/vrt/gdaltileindexdataset.cpp index 122e7f782b98..97e5b9465127 100644 --- a/frmts/vrt/gdaltileindexdataset.cpp +++ b/frmts/vrt/gdaltileindexdataset.cpp @@ -370,6 +370,9 @@ class GDALTileIndexBand final : public GDALPamRasterBand GSpacing nLineSpace, GDALRasterIOExtraArg *psExtraArg) override; + int IGetDataCoverageStatus(int nXOff, int nYOff, int nXSize, int nYSize, + int nMaskFlagStop, double *pdfDataPct) override; + int GetMaskFlags() override { if (m_poDS->m_poMaskBand && m_poDS->m_poMaskBand.get() != this) @@ -2328,6 +2331,151 @@ CPLErr GDALTileIndexBand::IRasterIO(GDALRWFlag eRWFlag, int nXOff, int nYOff, nPixelSpace, nLineSpace, 0, psExtraArg); } +/************************************************************************/ +/* IGetDataCoverageStatus() */ +/************************************************************************/ + +#ifndef HAVE_GEOS +int GDALTileIndexBand::IGetDataCoverageStatus(int /* nXOff */, int /* nYOff */, + int /* nXSize */, + int /* nYSize */, + int /* nMaskFlagStop */, + double *pdfDataPct) +{ + if (pdfDataPct != nullptr) + *pdfDataPct = -1.0; + return GDAL_DATA_COVERAGE_STATUS_UNIMPLEMENTED | + GDAL_DATA_COVERAGE_STATUS_DATA; +} +#else +int GDALTileIndexBand::IGetDataCoverageStatus(int nXOff, int nYOff, int nXSize, + int nYSize, int nMaskFlagStop, + double *pdfDataPct) +{ + if (pdfDataPct != nullptr) + *pdfDataPct = -1.0; + + const double dfMinX = m_poDS->m_adfGeoTransform[GT_TOPLEFT_X] + + nXOff * m_poDS->m_adfGeoTransform[GT_WE_RES]; + const double dfMaxX = + dfMinX + nXSize * m_poDS->m_adfGeoTransform[GT_WE_RES]; + const double dfMaxY = m_poDS->m_adfGeoTransform[GT_TOPLEFT_Y] + + nYOff * m_poDS->m_adfGeoTransform[GT_NS_RES]; + const double dfMinY = + dfMaxY + nYSize * m_poDS->m_adfGeoTransform[GT_NS_RES]; + m_poDS->m_poLayer->SetSpatialFilterRect(dfMinX, dfMinY, dfMaxX, dfMaxY); + m_poDS->m_poLayer->ResetReading(); + + int nStatus = 0; + + auto poPolyNonCoveredBySources = std::make_unique(); + { + auto poLR = std::make_unique(); + poLR->addPoint(nXOff, nYOff); + poLR->addPoint(nXOff, nYOff + nYSize); + poLR->addPoint(nXOff + nXSize, nYOff + nYSize); + poLR->addPoint(nXOff + nXSize, nYOff); + poLR->addPoint(nXOff, nYOff); + poPolyNonCoveredBySources->addRingDirectly(poLR.release()); + } + while (true) + { + auto poFeature = + std::unique_ptr(m_poDS->m_poLayer->GetNextFeature()); + if (!poFeature) + break; + if (!poFeature->IsFieldSetAndNotNull(m_poDS->m_nLocationFieldIndex)) + { + continue; + } + + const auto poGeom = poFeature->GetGeometryRef(); + if (!poGeom || poGeom->IsEmpty()) + continue; + + OGREnvelope sSourceEnvelope; + poGeom->getEnvelope(&sSourceEnvelope); + + const double dfDstXOff = + std::max(nXOff, (sSourceEnvelope.MinX - + m_poDS->m_adfGeoTransform[GT_TOPLEFT_X]) / + m_poDS->m_adfGeoTransform[GT_WE_RES]); + const double dfDstXOff2 = std::min( + nXOff + nXSize, + (sSourceEnvelope.MaxX - m_poDS->m_adfGeoTransform[GT_TOPLEFT_X]) / + m_poDS->m_adfGeoTransform[GT_WE_RES]); + const double dfDstYOff = + std::max(nYOff, (sSourceEnvelope.MaxY - + m_poDS->m_adfGeoTransform[GT_TOPLEFT_Y]) / + m_poDS->m_adfGeoTransform[GT_NS_RES]); + const double dfDstYOff2 = std::min( + nYOff + nYSize, + (sSourceEnvelope.MinY - m_poDS->m_adfGeoTransform[GT_TOPLEFT_Y]) / + m_poDS->m_adfGeoTransform[GT_NS_RES]); + + // CPLDebug("GTI", "dfDstXOff=%f, dfDstXOff2=%f, dfDstYOff=%f, dfDstYOff2=%f", + // dfDstXOff, dfDstXOff2, dfDstYOff, dfDstXOff2); + + // Check if the AOI is fully inside the source + if (nXOff >= dfDstXOff && nYOff >= dfDstYOff && + nXOff + nXSize <= dfDstXOff2 && nYOff + nYSize <= dfDstYOff2) + { + if (pdfDataPct) + *pdfDataPct = 100.0; + return GDAL_DATA_COVERAGE_STATUS_DATA; + } + + // Check intersection of bounding boxes. + if (dfDstXOff2 > nXOff && dfDstYOff2 > nYOff && + dfDstXOff < nXOff + nXSize && dfDstYOff < nYOff + nYSize) + { + nStatus |= GDAL_DATA_COVERAGE_STATUS_DATA; + if (poPolyNonCoveredBySources) + { + OGRPolygon oPolySource; + auto poLR = std::make_unique(); + poLR->addPoint(dfDstXOff, dfDstYOff); + poLR->addPoint(dfDstXOff, dfDstYOff2); + poLR->addPoint(dfDstXOff2, dfDstYOff2); + poLR->addPoint(dfDstXOff2, dfDstYOff); + poLR->addPoint(dfDstXOff, dfDstYOff); + oPolySource.addRingDirectly(poLR.release()); + auto poRes = std::unique_ptr( + poPolyNonCoveredBySources->Difference(&oPolySource)); + if (poRes && poRes->IsEmpty()) + { + if (pdfDataPct) + *pdfDataPct = 100.0; + return GDAL_DATA_COVERAGE_STATUS_DATA; + } + else if (poRes && poRes->getGeometryType() == wkbPolygon) + { + poPolyNonCoveredBySources.reset( + poRes.release()->toPolygon()); + } + else + { + poPolyNonCoveredBySources.reset(); + } + } + } + if (nMaskFlagStop != 0 && (nStatus & nMaskFlagStop) != 0) + { + return nStatus; + } + } + if (poPolyNonCoveredBySources) + { + if (!poPolyNonCoveredBySources->IsEmpty()) + nStatus |= GDAL_DATA_COVERAGE_STATUS_EMPTY; + if (pdfDataPct) + *pdfDataPct = 100.0 * (1.0 - poPolyNonCoveredBySources->get_Area() / + nXSize / nYSize); + } + return nStatus; +} +#endif // HAVE_GEOS + /************************************************************************/ /* GetMetadataDomainList() */ /************************************************************************/ diff --git a/ogr/ogr_geometry.h b/ogr/ogr_geometry.h index 3a957c5e8e08..42144dce1419 100644 --- a/ogr/ogr_geometry.h +++ b/ogr/ogr_geometry.h @@ -2577,7 +2577,7 @@ class CPL_DLL OGRCurvePolygon : public OGRSurface virtual void assignSpatialReference(const OGRSpatialReference *poSR) override; - virtual OGRErr addRing(OGRCurve *); + virtual OGRErr addRing(const OGRCurve *); virtual OGRErr addRingDirectly(OGRCurve *); OGRErr addRing(std::unique_ptr); diff --git a/ogr/ogrcurvecollection.cpp b/ogr/ogrcurvecollection.cpp index e17429c45d6a..36d6466dd4da 100644 --- a/ogr/ogrcurvecollection.cpp +++ b/ogr/ogrcurvecollection.cpp @@ -31,6 +31,7 @@ #include #include +#include #include #include "ogr_core.h" @@ -155,6 +156,21 @@ OGRErr OGRCurveCollection::addCurveDirectly(OGRGeometry *poGeom, if (bNeedRealloc) { +#if SIZEOF_VOIDP < 8 + if (nCurveCount == std::numeric_limits::max() / + static_cast(sizeof(OGRCurve *))) + { + CPLError(CE_Failure, CPLE_OutOfMemory, "Too many subgeometries"); + return OGRERR_FAILURE; + } +#else + if (nCurveCount == std::numeric_limits::max()) + { + CPLError(CE_Failure, CPLE_AppDefined, "Too many subgeometries"); + return OGRERR_FAILURE; + } +#endif + OGRCurve **papoNewCurves = static_cast(VSI_REALLOC_VERBOSE( papoCurves, sizeof(OGRCurve *) * (nCurveCount + 1))); if (papoNewCurves == nullptr) diff --git a/ogr/ogrcurvepolygon.cpp b/ogr/ogrcurvepolygon.cpp index af8d744d4150..206745d38a0b 100644 --- a/ogr/ogrcurvepolygon.cpp +++ b/ogr/ogrcurvepolygon.cpp @@ -342,7 +342,7 @@ OGRErr OGRCurvePolygon::removeRing(int iIndex, bool bDelete) * @return OGRERR_NONE in case of success */ -OGRErr OGRCurvePolygon::addRing(OGRCurve *poNewRing) +OGRErr OGRCurvePolygon::addRing(const OGRCurve *poNewRing) { OGRCurve *poNewRingCloned = poNewRing->clone(); diff --git a/ogr/ogrgeometrycollection.cpp b/ogr/ogrgeometrycollection.cpp index cd0ea336bad5..0b9c3e90d404 100644 --- a/ogr/ogrgeometrycollection.cpp +++ b/ogr/ogrgeometrycollection.cpp @@ -32,6 +32,7 @@ #include #include +#include #include #include "cpl_conv.h" @@ -71,7 +72,7 @@ OGRGeometryCollection::OGRGeometryCollection(const OGRGeometryCollection &other) { // Do not use addGeometry() as it is virtual. papoGeoms = static_cast( - VSI_CALLOC_VERBOSE(sizeof(void *), other.nGeomCount)); + VSI_CALLOC_VERBOSE(sizeof(OGRGeometry *), other.nGeomCount)); if (papoGeoms) { nGeomCount = other.nGeomCount; @@ -114,9 +115,9 @@ OGRGeometryCollection::operator=(const OGRGeometryCollection &other) OGRGeometry::operator=(other); - for (int i = 0; i < other.nGeomCount; i++) + for (const auto &poSubGeom : other) { - addGeometry(other.papoGeoms[i]); + addGeometry(poSubGeom); } } return *this; @@ -131,7 +132,7 @@ void OGRGeometryCollection::empty() { if (papoGeoms != nullptr) { - for (auto &&poSubGeom : *this) + for (auto &poSubGeom : *this) { delete poSubGeom; } @@ -179,7 +180,7 @@ int OGRGeometryCollection::getDimension() const int nDimension = 0; // FIXME? Not sure if it is really appropriate to take the max in case // of geometries of different dimension. - for (auto &&poSubGeom : *this) + for (const auto &poSubGeom : *this) { int nSubGeomDimension = poSubGeom->getDimension(); if (nSubGeomDimension > nDimension) @@ -199,7 +200,7 @@ int OGRGeometryCollection::getDimension() const void OGRGeometryCollection::flattenTo2D() { - for (auto &&poSubGeom : *this) + for (auto &poSubGeom : *this) { poSubGeom->flattenTo2D(); } @@ -361,10 +362,26 @@ OGRErr OGRGeometryCollection::addGeometryDirectly(OGRGeometry *poNewGeom) if (!isCompatibleSubType(poNewGeom->getGeometryType())) return OGRERR_UNSUPPORTED_GEOMETRY_TYPE; +#if SIZEOF_VOIDP < 8 + if (nGeomCount == std::numeric_limits::max() / + static_cast(sizeof(OGRGeometry *))) + { + CPLError(CE_Failure, CPLE_OutOfMemory, "Too many subgeometries"); + return OGRERR_FAILURE; + } +#else + if (nGeomCount == std::numeric_limits::max()) + { + CPLError(CE_Failure, CPLE_AppDefined, "Too many subgeometries"); + return OGRERR_FAILURE; + } +#endif + HomogenizeDimensionalityWith(poNewGeom); - OGRGeometry **papoNewGeoms = static_cast( - VSI_REALLOC_VERBOSE(papoGeoms, sizeof(void *) * (nGeomCount + 1))); + OGRGeometry **papoNewGeoms = + static_cast(VSI_REALLOC_VERBOSE( + papoGeoms, sizeof(OGRGeometry *) * (nGeomCount + 1))); if (papoNewGeoms == nullptr) return OGRERR_FAILURE; @@ -446,7 +463,7 @@ OGRErr OGRGeometryCollection::removeGeometry(int iGeom, int bDelete) delete papoGeoms[iGeom]; memmove(papoGeoms + iGeom, papoGeoms + iGeom + 1, - sizeof(void *) * (nGeomCount - iGeom - 1)); + sizeof(OGRGeometry *) * (nGeomCount - iGeom - 1)); nGeomCount--; @@ -459,9 +476,9 @@ OGRErr OGRGeometryCollection::removeGeometry(int iGeom, int bDelete) bool OGRGeometryCollection::hasEmptyParts() const { - for (int i = 0; i < nGeomCount; ++i) + for (const auto &poSubGeom : *this) { - if (papoGeoms[i]->IsEmpty() || papoGeoms[i]->hasEmptyParts()) + if (poSubGeom->IsEmpty() || poSubGeom->hasEmptyParts()) return true; } return false; @@ -536,7 +553,7 @@ OGRErr OGRGeometryCollection::importFromWkbInternal( // coverity[tainted_data] papoGeoms = static_cast( - VSI_CALLOC_VERBOSE(sizeof(void *), nGeomCount)); + VSI_CALLOC_VERBOSE(sizeof(OGRGeometry *), nGeomCount)); if (nGeomCount != 0 && papoGeoms == nullptr) { nGeomCount = 0; @@ -886,11 +903,10 @@ std::string OGRGeometryCollection::exportToWktInternal( try { - for (int i = 0; i < nGeomCount; ++i) + for (const auto &poSubGeom : *this) { - OGRGeometry *geom = papoGeoms[i]; OGRErr subgeomErr = OGRERR_NONE; - std::string tempWkt = geom->exportToWkt(opts, &subgeomErr); + std::string tempWkt = poSubGeom->exportToWkt(opts, &subgeomErr); if (subgeomErr != OGRERR_NONE) { if (err) @@ -976,7 +992,7 @@ void OGRGeometryCollection::getEnvelope(OGREnvelope3D *psEnvelope) const bool bExtentSet = false; *psEnvelope = OGREnvelope3D(); - for (auto &&poSubGeom : *this) + for (const auto &poSubGeom : *this) { if (!poSubGeom->IsEmpty()) { @@ -1037,7 +1053,7 @@ OGRErr OGRGeometryCollection::transform(OGRCoordinateTransformation *poCT) { int iGeom = 0; - for (auto &&poSubGeom : *this) + for (auto &poSubGeom : *this) { const OGRErr eErr = poSubGeom->transform(poCT); if (eErr != OGRERR_NONE) @@ -1069,7 +1085,7 @@ OGRErr OGRGeometryCollection::transform(OGRCoordinateTransformation *poCT) void OGRGeometryCollection::closeRings() { - for (auto &&poSubGeom : *this) + for (auto &poSubGeom : *this) { if (OGR_GT_IsSubClassOf(wkbFlatten(poSubGeom->getGeometryType()), wkbCurvePolygon)) @@ -1087,7 +1103,7 @@ void OGRGeometryCollection::closeRings() void OGRGeometryCollection::setCoordinateDimension(int nNewDimension) { - for (auto &&poSubGeom : *this) + for (auto &poSubGeom : *this) { poSubGeom->setCoordinateDimension(nNewDimension); } @@ -1097,7 +1113,7 @@ void OGRGeometryCollection::setCoordinateDimension(int nNewDimension) void OGRGeometryCollection::set3D(OGRBoolean bIs3D) { - for (auto &&poSubGeom : *this) + for (auto &poSubGeom : *this) { poSubGeom->set3D(bIs3D); } @@ -1107,7 +1123,7 @@ void OGRGeometryCollection::set3D(OGRBoolean bIs3D) void OGRGeometryCollection::setMeasured(OGRBoolean bIsMeasured) { - for (auto &&poSubGeom : *this) + for (auto &poSubGeom : *this) { poSubGeom->setMeasured(bIsMeasured); } @@ -1134,7 +1150,7 @@ void OGRGeometryCollection::setMeasured(OGRBoolean bIsMeasured) double OGRGeometryCollection::get_Length() const { double dfLength = 0.0; - for (auto &&poSubGeom : *this) + for (const auto &poSubGeom : *this) { const OGRwkbGeometryType eType = wkbFlatten(poSubGeom->getGeometryType()); @@ -1174,7 +1190,7 @@ double OGRGeometryCollection::get_Length() const double OGRGeometryCollection::get_Area() const { double dfArea = 0.0; - for (auto &&poSubGeom : *this) + for (const auto &poSubGeom : *this) { OGRwkbGeometryType eType = wkbFlatten(poSubGeom->getGeometryType()); if (OGR_GT_IsSurface(eType)) @@ -1234,7 +1250,7 @@ double OGRGeometryCollection::get_GeodesicArea( poSRSOverride = getSpatialReference(); double dfArea = 0.0; - for (auto &&poSubGeom : *this) + for (const auto &poSubGeom : *this) { OGRwkbGeometryType eType = wkbFlatten(poSubGeom->getGeometryType()); if (OGR_GT_IsSurface(eType)) @@ -1275,7 +1291,7 @@ double OGRGeometryCollection::get_GeodesicArea( OGRBoolean OGRGeometryCollection::IsEmpty() const { - for (auto &&poSubGeom : *this) + for (const auto &poSubGeom : *this) { if (poSubGeom->IsEmpty() == FALSE) return FALSE; @@ -1291,7 +1307,7 @@ void OGRGeometryCollection::assignSpatialReference( const OGRSpatialReference *poSR) { OGRGeometry::assignSpatialReference(poSR); - for (auto &&poSubGeom : *this) + for (auto &poSubGeom : *this) { poSubGeom->assignSpatialReference(poSR); } @@ -1303,7 +1319,7 @@ void OGRGeometryCollection::assignSpatialReference( void OGRGeometryCollection::segmentize(double dfMaxLength) { - for (auto &&poSubGeom : *this) + for (auto &poSubGeom : *this) { poSubGeom->segmentize(dfMaxLength); } @@ -1315,7 +1331,7 @@ void OGRGeometryCollection::segmentize(double dfMaxLength) void OGRGeometryCollection::swapXY() { - for (auto &&poSubGeom : *this) + for (auto &poSubGeom : *this) { poSubGeom->swapXY(); } @@ -1345,9 +1361,9 @@ OGRBoolean OGRGeometryCollection::isCompatibleSubType( OGRBoolean OGRGeometryCollection::hasCurveGeometry(int bLookForNonLinear) const { - for (int iGeom = 0; iGeom < nGeomCount; iGeom++) + for (const auto &poSubGeom : *this) { - if (papoGeoms[iGeom]->hasCurveGeometry(bLookForNonLinear)) + if (poSubGeom->hasCurveGeometry(bLookForNonLinear)) return TRUE; } return FALSE; @@ -1361,19 +1377,20 @@ OGRGeometry * OGRGeometryCollection::getLinearGeometry(double dfMaxAngleStepSizeDegrees, const char *const *papszOptions) const { - OGRGeometryCollection *poGC = + auto poGC = std::unique_ptr( OGRGeometryFactory::createGeometry(OGR_GT_GetLinear(getGeometryType())) - ->toGeometryCollection(); - if (poGC == nullptr) + ->toGeometryCollection()); + if (!poGC) return nullptr; poGC->assignSpatialReference(getSpatialReference()); - for (int iGeom = 0; iGeom < nGeomCount; iGeom++) + for (const auto &poSubGeom : *this) { - OGRGeometry *poSubGeom = papoGeoms[iGeom]->getLinearGeometry( + OGRGeometry *poSubGeomNew = poSubGeom->getLinearGeometry( dfMaxAngleStepSizeDegrees, papszOptions); - poGC->addGeometryDirectly(poSubGeom); + if (poGC->addGeometryDirectly(poSubGeomNew) != OGRERR_NONE) + return nullptr; } - return poGC; + return poGC.release(); } /************************************************************************/ @@ -1383,27 +1400,26 @@ OGRGeometryCollection::getLinearGeometry(double dfMaxAngleStepSizeDegrees, OGRGeometry * OGRGeometryCollection::getCurveGeometry(const char *const *papszOptions) const { - OGRGeometryCollection *poGC = + auto poGC = std::unique_ptr( OGRGeometryFactory::createGeometry(OGR_GT_GetCurve(getGeometryType())) - ->toGeometryCollection(); - if (poGC == nullptr) + ->toGeometryCollection()); + if (!poGC) return nullptr; poGC->assignSpatialReference(getSpatialReference()); bool bHasCurveGeometry = false; - for (int iGeom = 0; iGeom < nGeomCount; iGeom++) + for (const auto &poSubGeom : *this) { - OGRGeometry *poSubGeom = - papoGeoms[iGeom]->getCurveGeometry(papszOptions); - if (poSubGeom->hasCurveGeometry()) + OGRGeometry *poSubGeomNew = poSubGeom->getCurveGeometry(papszOptions); + if (poSubGeomNew->hasCurveGeometry()) bHasCurveGeometry = true; - poGC->addGeometryDirectly(poSubGeom); + if (poGC->addGeometryDirectly(poSubGeomNew) != OGRERR_NONE) + return nullptr; } if (!bHasCurveGeometry) { - delete poGC; return clone(); } - return poGC; + return poGC.release(); } /************************************************************************/ diff --git a/ogr/ogrlinestring.cpp b/ogr/ogrlinestring.cpp index 9d59218fb9a4..f3e155eac89f 100644 --- a/ogr/ogrlinestring.cpp +++ b/ogr/ogrlinestring.cpp @@ -530,6 +530,20 @@ void OGRSimpleCurve::setPoint(int iPoint, OGRPoint *poPoint) setPoint(iPoint, poPoint->getX(), poPoint->getY()); } +/************************************************************************/ +/* CheckPointCount() */ +/************************************************************************/ + +static inline bool CheckPointCount(int iPoint) +{ + if (iPoint == std::numeric_limits::max()) + { + CPLError(CE_Failure, CPLE_AppDefined, "Too big point count."); + return false; + } + return true; +} + /************************************************************************/ /* setPoint() */ /************************************************************************/ @@ -557,6 +571,8 @@ void OGRSimpleCurve::setPoint(int iPoint, double xIn, double yIn, double zIn) if (iPoint >= nPointCount) { + if (!CheckPointCount(iPoint)) + return; setNumPoints(iPoint + 1); if (nPointCount < iPoint + 1) return; @@ -598,6 +614,8 @@ void OGRSimpleCurve::setPointM(int iPoint, double xIn, double yIn, double mIn) if (iPoint >= nPointCount) { + if (!CheckPointCount(iPoint)) + return; setNumPoints(iPoint + 1); if (nPointCount < iPoint + 1) return; @@ -643,6 +661,8 @@ void OGRSimpleCurve::setPoint(int iPoint, double xIn, double yIn, double zIn, if (iPoint >= nPointCount) { + if (!CheckPointCount(iPoint)) + return; setNumPoints(iPoint + 1); if (nPointCount < iPoint + 1) return; @@ -684,6 +704,8 @@ void OGRSimpleCurve::setPoint(int iPoint, double xIn, double yIn) { if (iPoint >= nPointCount) { + if (!CheckPointCount(iPoint)) + return; setNumPoints(iPoint + 1); if (nPointCount < iPoint + 1 || paoPoints == nullptr) return; @@ -717,6 +739,8 @@ void OGRSimpleCurve::setZ(int iPoint, double zIn) if (iPoint >= nPointCount) { + if (!CheckPointCount(iPoint)) + return; setNumPoints(iPoint + 1); if (nPointCount < iPoint + 1) return; @@ -750,6 +774,8 @@ void OGRSimpleCurve::setM(int iPoint, double mIn) if (iPoint >= nPointCount) { + if (!CheckPointCount(iPoint)) + return; setNumPoints(iPoint + 1); if (nPointCount < iPoint + 1) return; diff --git a/ogr/ogrsf_frmts/arrow/ogrfeatherlayer.cpp b/ogr/ogrsf_frmts/arrow/ogrfeatherlayer.cpp index a82639d33332..8d867d8bda3c 100644 --- a/ogr/ogrsf_frmts/arrow/ogrfeatherlayer.cpp +++ b/ogr/ogrsf_frmts/arrow/ogrfeatherlayer.cpp @@ -175,7 +175,7 @@ void OGRFeatherLayer::EstablishFeatureDefn() if (field_kv_metadata) { auto extension_name = - field_kv_metadata->Get("ARROW:extension:name"); + field_kv_metadata->Get(ARROW_EXTENSION_NAME_KEY); if (extension_name.ok()) { osExtensionName = *extension_name; diff --git a/ogr/ogrsf_frmts/arrow_common/ograrrowlayer.hpp b/ogr/ogrsf_frmts/arrow_common/ograrrowlayer.hpp index 51b71cf4b332..0ef2b1ec0a05 100644 --- a/ogr/ogrsf_frmts/arrow_common/ograrrowlayer.hpp +++ b/ogr/ogrsf_frmts/arrow_common/ograrrowlayer.hpp @@ -304,16 +304,13 @@ inline bool OGRArrowLayer::MapArrowTypeToOGR( } else if (const auto &field_kv_metadata = field->metadata()) { - auto extension_name = field_kv_metadata->Get("ARROW:extension:name"); + auto extension_name = field_kv_metadata->Get(ARROW_EXTENSION_NAME_KEY); if (extension_name.ok()) { osExtensionName = *extension_name; } } - // Preliminary/in-advance read support for future JSON Canonical Extension - // Cf https://github.com/apache/arrow/pull/41257 and - // https://github.com/apache/arrow/pull/13901 if (!osExtensionName.empty() && osExtensionName != EXTENSION_NAME_ARROW_JSON) { diff --git a/ogr/ogrsf_frmts/arrow_common/ograrrowwriterlayer.hpp b/ogr/ogrsf_frmts/arrow_common/ograrrowwriterlayer.hpp index bb0735fcb1db..20aed6fe967d 100644 --- a/ogr/ogrsf_frmts/arrow_common/ograrrowwriterlayer.hpp +++ b/ogr/ogrsf_frmts/arrow_common/ograrrowwriterlayer.hpp @@ -153,6 +153,7 @@ inline void OGRArrowWriterLayer::CreateSchemaCommon() { const auto poFieldDefn = m_poFeatureDefn->GetFieldDefn(i); std::shared_ptr dt; + const auto eDT = poFieldDefn->GetType(); const auto eSubDT = poFieldDefn->GetSubType(); const auto &osDomainName = poFieldDefn->GetDomainName(); const OGRFieldDomain *poFieldDomain = nullptr; @@ -172,7 +173,7 @@ inline void OGRArrowWriterLayer::CreateSchemaCommon() poFieldDomain = oIter->second.get(); } } - switch (poFieldDefn->GetType()) + switch (eDT) { case OFTInteger: if (eSubDT == OFSTBoolean) @@ -209,7 +210,7 @@ inline void OGRArrowWriterLayer::CreateSchemaCommon() case OFTString: case OFTWideString: - if (eSubDT != OFSTNone || nWidth > 0) + if ((eSubDT != OFSTNone && eSubDT != OFSTJSON) || nWidth > 0) bNeedGDALSchema = true; dt = arrow::utf8(); break; @@ -265,9 +266,18 @@ inline void OGRArrowWriterLayer::CreateSchemaCommon() break; } } - fields.emplace_back(arrow::field(poFieldDefn->GetNameRef(), - std::move(dt), - poFieldDefn->IsNullable())); + + auto field = arrow::field(poFieldDefn->GetNameRef(), std::move(dt), + poFieldDefn->IsNullable()); + if (eDT == OFTString && eSubDT == OFSTJSON) + { + auto kvMetadata = std::make_shared(); + kvMetadata->Append(ARROW_EXTENSION_NAME_KEY, + EXTENSION_NAME_ARROW_JSON); + field = field->WithMetadata(kvMetadata); + } + + fields.emplace_back(std::move(field)); if (poFieldDefn->GetAlternativeNameRef()[0]) bNeedGDALSchema = true; if (!poFieldDefn->GetComment().empty()) @@ -389,7 +399,7 @@ inline void OGRArrowWriterLayer::CreateSchemaCommon() ? field->metadata()->Copy() : std::make_shared(); kvMetadata->Append( - "ARROW:extension:name", + ARROW_EXTENSION_NAME_KEY, GetGeomEncodingAsString(m_aeGeomEncoding[i], false)); field = field->WithMetadata(kvMetadata); } diff --git a/ogr/ogrsf_frmts/generic/ogrlayerarrow.cpp b/ogr/ogrsf_frmts/generic/ogrlayerarrow.cpp index 085610b02356..b6f9a8ba1a61 100644 --- a/ogr/ogrsf_frmts/generic/ogrlayerarrow.cpp +++ b/ogr/ogrsf_frmts/generic/ogrlayerarrow.cpp @@ -419,9 +419,10 @@ int OGRLayer::GetArrowSchema(struct ArrowArrayStream *, psChild->name = CPLStrdup(poFieldDefn->GetNameRef()); if (poFieldDefn->IsNullable()) psChild->flags = ARROW_FLAG_NULLABLE; + const auto eType = poFieldDefn->GetType(); const auto eSubType = poFieldDefn->GetSubType(); const char *item_format = nullptr; - switch (poFieldDefn->GetType()) + switch (eType) { case OFTInteger: { @@ -585,15 +586,18 @@ int OGRLayer::GetArrowSchema(struct ArrowArrayStream *, if (!osComment.empty()) oMetadata.emplace_back(std::pair(MD_GDAL_OGR_COMMENT, osComment)); - if (poFieldDefn->GetSubType() != OFSTNone && - poFieldDefn->GetSubType() != OFSTBoolean && - poFieldDefn->GetSubType() != OFSTFloat32) + if (eType == OFTString && eSubType == OFSTJSON) { oMetadata.emplace_back( - std::pair(MD_GDAL_OGR_SUBTYPE, - OGR_GetFieldSubTypeName(poFieldDefn->GetSubType()))); + std::pair(ARROW_EXTENSION_NAME_KEY, EXTENSION_NAME_ARROW_JSON)); + } + else if (eSubType != OFSTNone && eSubType != OFSTBoolean && + eSubType != OFSTFloat32) + { + oMetadata.emplace_back(std::pair( + MD_GDAL_OGR_SUBTYPE, OGR_GetFieldSubTypeName(eSubType))); } - if (poFieldDefn->GetType() == OFTString && poFieldDefn->GetWidth() > 0) + if (eType == OFTString && poFieldDefn->GetWidth() > 0) { oMetadata.emplace_back(std::pair( MD_GDAL_OGR_WIDTH, CPLSPrintf("%d", poFieldDefn->GetWidth()))); diff --git a/ogr/ogrsf_frmts/osm/ogr_osm.h b/ogr/ogrsf_frmts/osm/ogr_osm.h index 5407175d58a5..7985caca6397 100644 --- a/ogr/ogrsf_frmts/osm/ogr_osm.h +++ b/ogr/ogrsf_frmts/osm/ogr_osm.h @@ -269,8 +269,8 @@ class OGROSMLayer final : public OGRLayer } void SetFieldsFromTags(OGRFeature *poFeature, GIntBig nID, bool bIsWayID, - unsigned int nTags, OSMTag *pasTags, - OSMInfo *psInfo); + unsigned int nTags, const OSMTag *pasTags, + const OSMInfo *psInfo); void SetDeclareInterest(bool bIn) { @@ -522,30 +522,31 @@ class OGROSMDataSource final : public OGRDataSource static const GIntBig FILESIZE_INVALID = -1; GIntBig m_nFileSize = FILESIZE_NOT_INIT; - void CompressWay(bool bIsArea, unsigned int nTags, IndexedKVP *pasTags, - int nPoints, LonLat *pasLonLatPairs, OSMInfo *psInfo, + void CompressWay(bool bIsArea, unsigned int nTags, + const IndexedKVP *pasTags, int nPoints, + const LonLat *pasLonLatPairs, const OSMInfo *psInfo, std::vector &abyCompressedWay); void UncompressWay(int nBytes, const GByte *pabyCompressedWay, bool *pbIsArea, std::vector &asCoords, unsigned int *pnTags, OSMTag *pasTags, OSMInfo *psInfo); - bool ParseConf(char **papszOpenOptions); + bool ParseConf(CSLConstList papszOpenOptions); bool CreateTempDB(); bool SetDBOptions(); void SetCacheSize(); bool CreatePreparedStatements(); void CloseDB(); - bool IndexPoint(OSMNode *psNode); - bool IndexPointSQLite(OSMNode *psNode); + bool IndexPoint(const OSMNode *psNode); + bool IndexPointSQLite(const OSMNode *psNode); bool FlushCurrentSector(); bool FlushCurrentSectorCompressedCase(); bool FlushCurrentSectorNonCompressedCase(); - bool IndexPointCustom(OSMNode *psNode); + bool IndexPointCustom(const OSMNode *psNode); void IndexWay(GIntBig nWayID, bool bIsArea, unsigned int nTags, - IndexedKVP *pasTags, LonLat *pasLonLatPairs, int nPairs, - OSMInfo *psInfo); + const IndexedKVP *pasTags, const LonLat *pasLonLatPairs, + int nPairs, const OSMInfo *psInfo); bool StartTransactionCacheDB(); bool CommitTransactionCacheDB(); @@ -563,12 +564,12 @@ class OGROSMDataSource final : public OGRDataSource unsigned int LookupWays(std::map> &aoMapWays, - OSMRelation *psRelation); + const OSMRelation *psRelation); - OGRGeometry *BuildMultiPolygon(OSMRelation *psRelation, + OGRGeometry *BuildMultiPolygon(const OSMRelation *psRelation, unsigned int *pnTags, OSMTag *pasTags); - OGRGeometry *BuildGeometryCollection(OSMRelation *psRelation, - int bMultiLineString); + OGRGeometry *BuildGeometryCollection(const OSMRelation *psRelation, + bool bMultiLineString); bool TransferToDiskIfNecesserary(); @@ -610,7 +611,7 @@ class OGROSMDataSource final : public OGRDataSource GDALProgressFunc pfnProgress, void *pProgressData) override; - int Open(const char *pszFilename, char **papszOpenOptions); + int Open(const char *pszFilename, CSLConstList papszOpenOptions); int MyResetReading(); bool ParseNextChunk(int nIdxLayer, GDALProgressFunc pfnProgress, @@ -618,9 +619,9 @@ class OGROSMDataSource final : public OGRDataSource OGRErr GetExtent(OGREnvelope *psExtent); int IsInterleavedReading(); - void NotifyNodes(unsigned int nNodes, OSMNode *pasNodes); - void NotifyWay(OSMWay *psWay); - void NotifyRelation(OSMRelation *psRelation); + void NotifyNodes(unsigned int nNodes, const OSMNode *pasNodes); + void NotifyWay(const OSMWay *psWay); + void NotifyRelation(const OSMRelation *psRelation); void NotifyBounds(double dfXMin, double dfYMin, double dfXMax, double dfYMax); diff --git a/ogr/ogrsf_frmts/osm/ogrosmdatasource.cpp b/ogr/ogrsf_frmts/osm/ogrosmdatasource.cpp index a00d31dab42d..79ee6de286fa 100644 --- a/ogr/ogrsf_frmts/osm/ogrosmdatasource.cpp +++ b/ogr/ogrsf_frmts/osm/ogrosmdatasource.cpp @@ -317,21 +317,20 @@ OGROSMDataSource::~OGROSMDataSource() } CPLFree(m_pabySector); - std::map::iterator oIter = m_oMapBuckets.begin(); - for (; oIter != m_oMapBuckets.end(); ++oIter) + for (auto &oIter : m_oMapBuckets) { if (m_bCompressNodes) { int nRem = - oIter->first % (knPAGE_SIZE / BUCKET_SECTOR_SIZE_ARRAY_SIZE); + oIter.first % (knPAGE_SIZE / BUCKET_SECTOR_SIZE_ARRAY_SIZE); if (nRem == 0) - CPLFree(oIter->second.u.panSectorSize); + CPLFree(oIter.second.u.panSectorSize); } else { - int nRem = oIter->first % (knPAGE_SIZE / BUCKET_BITMAP_SIZE); + int nRem = oIter.first % (knPAGE_SIZE / BUCKET_BITMAP_SIZE); if (nRem == 0) - CPLFree(oIter->second.u.pabyBitmap); + CPLFree(oIter.second.u.pabyBitmap); } } } @@ -408,7 +407,7 @@ constexpr GByte abyBitsCount[] = { 4, 5, 5, 6, 5, 6, 6, 7, 3, 4, 4, 5, 4, 5, 5, 6, 4, 5, 5, 6, 5, 6, 6, 7, 4, 5, 5, 6, 5, 6, 6, 7, 5, 6, 6, 7, 6, 7, 7, 8}; -bool OGROSMDataSource::IndexPoint(OSMNode *psNode) +bool OGROSMDataSource::IndexPoint(const OSMNode *psNode) { if (!m_bIndexPoints) return true; @@ -423,7 +422,7 @@ bool OGROSMDataSource::IndexPoint(OSMNode *psNode) /* IndexPointSQLite() */ /************************************************************************/ -bool OGROSMDataSource::IndexPointSQLite(OSMNode *psNode) +bool OGROSMDataSource::IndexPointSQLite(const OSMNode *psNode) { sqlite3_bind_int64(m_hInsertNodeStmt, 1, psNode->nID); @@ -517,7 +516,7 @@ Bucket *OGROSMDataSource::AllocBucket(int iBucket) Bucket *OGROSMDataSource::GetBucket(int nBucketId) { - std::map::iterator oIter = m_oMapBuckets.find(nBucketId); + auto oIter = m_oMapBuckets.find(nBucketId); if (oIter == m_oMapBuckets.end()) { Bucket *psBucket = &m_oMapBuckets[nBucketId]; @@ -638,7 +637,7 @@ bool OGROSMDataSource::FlushCurrentSectorNonCompressedCase() /* IndexPointCustom() */ /************************************************************************/ -bool OGROSMDataSource::IndexPointCustom(OSMNode *psNode) +bool OGROSMDataSource::IndexPointCustom(const OSMNode *psNode) { if (psNode->nID <= m_nPrevNodeId) { @@ -720,7 +719,7 @@ bool OGROSMDataSource::IndexPointCustom(OSMNode *psNode) /* NotifyNodes() */ /************************************************************************/ -void OGROSMDataSource::NotifyNodes(unsigned int nNodes, OSMNode *pasNodes) +void OGROSMDataSource::NotifyNodes(unsigned int nNodes, const OSMNode *pasNodes) { const OGREnvelope *psEnvelope = m_apoLayers[IDX_LYR_POINTS]->GetSpatialFilterEnvelope(); @@ -742,7 +741,7 @@ void OGROSMDataSource::NotifyNodes(unsigned int nNodes, OSMNode *pasNodes) continue; bool bInterestingTag = m_bReportAllNodes; - OSMTag *pasTags = pasNodes[i].pasTags; + const OSMTag *pasTags = pasNodes[i].pasTags; if (!m_bReportAllNodes) { @@ -1025,8 +1024,7 @@ void OGROSMDataSource::LookupNodesCustom() int nOffInBucket = static_cast(id % NODE_PER_BUCKET); int nOffInBucketReduced = nOffInBucket >> NODE_PER_SECTOR_SHIFT; - std::map::const_iterator oIter = - m_oMapBuckets.find(nBucket); + const auto oIter = m_oMapBuckets.find(nBucket); if (oIter == m_oMapBuckets.end()) continue; const Bucket *psBucket = &(oIter->second); @@ -1109,8 +1107,7 @@ void OGROSMDataSource::LookupNodesCustomCompressedCase() if (nOffInBucketReduced != l_nOffInBucketReducedOld) { - std::map::const_iterator oIter = - m_oMapBuckets.find(nBucket); + const auto oIter = m_oMapBuckets.find(nBucket); if (oIter == m_oMapBuckets.end()) { CPLError(CE_Failure, CPLE_AppDefined, @@ -1223,8 +1220,7 @@ void OGROSMDataSource::LookupNodesCustomNonCompressedCase() if (psBucket == nullptr || nBucket != l_nBucketOld) { - std::map::const_iterator oIter = - m_oMapBuckets.find(nBucket); + const auto oIter = m_oMapBuckets.find(nBucket); if (oIter == m_oMapBuckets.end()) { CPLError(CE_Failure, CPLE_AppDefined, @@ -1383,8 +1379,9 @@ static void WriteVarSInt64(GIntBig nSVal, GByte **ppabyData) /************************************************************************/ void OGROSMDataSource::CompressWay(bool bIsArea, unsigned int nTags, - IndexedKVP *pasTags, int nPoints, - LonLat *pasLonLatPairs, OSMInfo *psInfo, + const IndexedKVP *pasTags, int nPoints, + const LonLat *pasLonLatPairs, + const OSMInfo *psInfo, std::vector &abyCompressedWay) { abyCompressedWay.clear(); @@ -1560,9 +1557,9 @@ void OGROSMDataSource::UncompressWay(int nBytes, const GByte *pabyCompressedWay, /************************************************************************/ void OGROSMDataSource::IndexWay(GIntBig nWayID, bool bIsArea, - unsigned int nTags, IndexedKVP *pasTags, - LonLat *pasLonLatPairs, int nPairs, - OSMInfo *psInfo) + unsigned int nTags, const IndexedKVP *pasTags, + const LonLat *pasLonLatPairs, int nPairs, + const OSMInfo *psInfo) { if (!m_bIndexWays) return; @@ -1732,7 +1729,7 @@ void OGROSMDataSource::ProcessWaysBatch() OGRGeometry *poGeom = poLS; const int nPoints = static_cast(m_asLonLatCache.size()); - poLS->setNumPoints(nPoints); + poLS->setNumPoints(nPoints, /*bZeroizeNewContent=*/false); for (int i = 0; i < nPoints; i++) { poLS->setPoint(i, INT_TO_DBL(m_asLonLatCache[i].nLon), @@ -1860,7 +1857,7 @@ bool OGROSMDataSource::IsClosedWayTaggedAsPolygon(unsigned int nTags, /* NotifyWay() */ /************************************************************************/ -void OGROSMDataSource::NotifyWay(OSMWay *psWay) +void OGROSMDataSource::NotifyWay(const OSMWay *psWay) { m_nWaysProcessed++; if (m_nWaysProcessed % 10000 == 0) @@ -2158,7 +2155,7 @@ static void OGROSMNotifyWay(OSMWay *psWay, OSMContext * /* psOSMContext */, unsigned int OGROSMDataSource::LookupWays( std::map> &aoMapWays, - OSMRelation *psRelation) + const OSMRelation *psRelation) { unsigned int nFound = 0; unsigned int iCur = 0; @@ -2224,7 +2221,7 @@ unsigned int OGROSMDataSource::LookupWays( /* BuildMultiPolygon() */ /************************************************************************/ -OGRGeometry *OGROSMDataSource::BuildMultiPolygon(OSMRelation *psRelation, +OGRGeometry *OGROSMDataSource::BuildMultiPolygon(const OSMRelation *psRelation, unsigned int *pnTags, OSMTag *pasTags) { @@ -2260,9 +2257,8 @@ OGRGeometry *OGROSMDataSource::BuildMultiPolygon(OSMRelation *psRelation, return nullptr; } - OGRMultiLineString *poMLS = new OGRMultiLineString(); - OGRGeometry **papoPolygons = static_cast( - CPLMalloc(sizeof(OGRGeometry *) * psRelation->nMembers)); + OGRMultiLineString oMLS; + std::vector apoPolygons(psRelation->nMembers); int nPolys = 0; if (pnTags != nullptr) @@ -2305,7 +2301,7 @@ OGRGeometry *OGROSMDataSource::BuildMultiPolygon(OSMRelation *psRelation, OGRPolygon *poPoly = new OGRPolygon(); OGRLinearRing *poRing = new OGRLinearRing(); poPoly->addRingDirectly(poRing); - papoPolygons[nPolys++] = poPoly; + apoPolygons[nPolys++] = poPoly; poLS = poRing; if (strcmp(psRelation->pasMembers[i].pszRole, "outer") == 0) @@ -2320,11 +2316,11 @@ OGRGeometry *OGROSMDataSource::BuildMultiPolygon(OSMRelation *psRelation, else { poLS = new OGRLineString(); - poMLS->addGeometryDirectly(poLS); + oMLS.addGeometryDirectly(poLS); } const int nPoints = static_cast(m_asLonLatCache.size()); - poLS->setNumPoints(nPoints); + poLS->setNumPoints(nPoints, /*bZeroizeNewContent=*/false); for (int j = 0; j < nPoints; j++) { poLS->setPoint(j, INT_TO_DBL(m_asLonLatCache[j].nLon), @@ -2333,21 +2329,16 @@ OGRGeometry *OGROSMDataSource::BuildMultiPolygon(OSMRelation *psRelation, } } - if (poMLS->getNumGeometries() > 0) + if (oMLS.getNumGeometries() > 0) { - OGRGeometryH hPoly = OGRBuildPolygonFromEdges( - OGRGeometry::ToHandle(poMLS), TRUE, FALSE, 0, nullptr); - if (hPoly != nullptr && OGR_G_GetGeometryType(hPoly) == wkbPolygon) + auto poPolyFromEdges = std::unique_ptr( + OGRGeometry::FromHandle(OGRBuildPolygonFromEdges( + OGRGeometry::ToHandle(&oMLS), TRUE, FALSE, 0, nullptr))); + if (poPolyFromEdges && poPolyFromEdges->getGeometryType() == wkbPolygon) { - OGRPolygon *poSuperPoly = - OGRGeometry::FromHandle(hPoly)->toPolygon(); - const unsigned nRings = 1 + static_cast( - poSuperPoly->getNumInteriorRings()); - for (unsigned int i = 0; i < nRings; i++) + const OGRPolygon *poSuperPoly = poPolyFromEdges->toPolygon(); + for (const OGRLinearRing *poRing : *poSuperPoly) { - OGRLinearRing *poRing = - (i == 0) ? poSuperPoly->getExteriorRing() - : poSuperPoly->getInteriorRing(i - 1); if (poRing != nullptr && poRing->getNumPoints() >= 4 && poRing->getX(0) == poRing->getX(poRing->getNumPoints() - 1) && @@ -2355,34 +2346,32 @@ OGRGeometry *OGROSMDataSource::BuildMultiPolygon(OSMRelation *psRelation, { OGRPolygon *poPoly = new OGRPolygon(); poPoly->addRing(poRing); - papoPolygons[nPolys++] = poPoly; + apoPolygons[nPolys++] = poPoly; } } } - - OGR_G_DestroyGeometry(hPoly); } - delete poMLS; - OGRGeometry *poRet = nullptr; + std::unique_ptr poRet; if (nPolys > 0) { int bIsValidGeometry = FALSE; const char *apszOptions[2] = {"METHOD=DEFAULT", nullptr}; - OGRGeometry *poGeom = OGRGeometryFactory::organizePolygons( - papoPolygons, nPolys, &bIsValidGeometry, apszOptions); + auto poGeom = + std::unique_ptr(OGRGeometryFactory::organizePolygons( + apoPolygons.data(), nPolys, &bIsValidGeometry, apszOptions)); - if (poGeom != nullptr && poGeom->getGeometryType() == wkbPolygon) + if (poGeom && poGeom->getGeometryType() == wkbPolygon) { - OGRMultiPolygon *poMulti = new OGRMultiPolygon(); - poMulti->addGeometryDirectly(poGeom); - poGeom = poMulti; + auto poMulti = std::make_unique(); + poMulti->addGeometryDirectly(poGeom.release()); + poGeom = std::move(poMulti); } - if (poGeom != nullptr && poGeom->getGeometryType() == wkbMultiPolygon) + if (poGeom && poGeom->getGeometryType() == wkbMultiPolygon) { - poRet = poGeom; + poRet = std::move(poGeom); } else { @@ -2390,35 +2379,33 @@ OGRGeometry *OGROSMDataSource::BuildMultiPolygon(OSMRelation *psRelation, "Relation " CPL_FRMT_GIB ": Geometry has incompatible type : %s", psRelation->nID, - poGeom != nullptr - ? OGR_G_GetGeometryName(OGRGeometry::ToHandle(poGeom)) - : "null"); - delete poGeom; + poGeom ? OGR_G_GetGeometryName( + OGRGeometry::ToHandle(poGeom.get())) + : "null"); } } - CPLFree(papoPolygons); - // cppcheck-suppress constVariableReference for (auto &oIter : aoMapWays) CPLFree(oIter.second.second); - return poRet; + return poRet.release(); } /************************************************************************/ /* BuildGeometryCollection() */ /************************************************************************/ -OGRGeometry *OGROSMDataSource::BuildGeometryCollection(OSMRelation *psRelation, - int bMultiLineString) +OGRGeometry * +OGROSMDataSource::BuildGeometryCollection(const OSMRelation *psRelation, + bool bMultiLineString) { std::map> aoMapWays; LookupWays(aoMapWays, psRelation); - OGRGeometryCollection *poColl = (bMultiLineString) - ? new OGRMultiLineString() - : new OGRGeometryCollection(); + std::unique_ptr poColl = + bMultiLineString ? std::make_unique() + : std::make_unique(); for (unsigned int i = 0; i < psRelation->nMembers; i++) { @@ -2460,7 +2447,7 @@ OGRGeometry *OGROSMDataSource::BuildGeometryCollection(OSMRelation *psRelation, } const int nPoints = static_cast(m_asLonLatCache.size()); - poLS->setNumPoints(nPoints); + poLS->setNumPoints(nPoints, /*bZeroizeNewContent=*/false); for (int j = 0; j < nPoints; j++) { poLS->setPoint(j, INT_TO_DBL(m_asLonLatCache[j].nLon), @@ -2471,22 +2458,21 @@ OGRGeometry *OGROSMDataSource::BuildGeometryCollection(OSMRelation *psRelation, if (poColl->getNumGeometries() == 0) { - delete poColl; - poColl = nullptr; + poColl.reset(); } // cppcheck-suppress constVariableReference for (auto &oIter : aoMapWays) CPLFree(oIter.second.second); - return poColl; + return poColl.release(); } /************************************************************************/ /* NotifyRelation() */ /************************************************************************/ -void OGROSMDataSource::NotifyRelation(OSMRelation *psRelation) +void OGROSMDataSource::NotifyRelation(const OSMRelation *psRelation) { if (!m_asWayFeaturePairs.empty()) ProcessWaysBatch(); @@ -2669,7 +2655,8 @@ void OGROSMDataSource::ProcessPolygonsStandalone() poPoly->addRingDirectly(poRing); OGRLineString *poLS = poRing; - poLS->setNumPoints(static_cast(m_asLonLatCache.size())); + poLS->setNumPoints(static_cast(m_asLonLatCache.size()), + /*bZeroizeNewContent=*/false); for (int j = 0; j < static_cast(m_asLonLatCache.size()); j++) { poLS->setPoint(j, INT_TO_DBL(m_asLonLatCache[j].nLon), @@ -2738,7 +2725,8 @@ static void OGROSMNotifyBounds(double dfXMin, double dfYMin, double dfXMax, /* Open() */ /************************************************************************/ -int OGROSMDataSource::Open(const char *pszFilename, char **papszOpenOptionsIn) +int OGROSMDataSource::Open(const char *pszFilename, + CSLConstList papszOpenOptionsIn) { m_pszName = CPLStrdup(pszFilename); @@ -2850,7 +2838,7 @@ int OGROSMDataSource::Open(const char *pszFilename, char **papszOpenOptionsIn) VSI_MALLOC_VERBOSE(MAX_ACCUMULATED_NODES * sizeof(GIntBig))); try { - m_asWayFeaturePairs.resize(MAX_DELAYED_FEATURES); + m_asWayFeaturePairs.reserve(MAX_DELAYED_FEATURES); } catch (const std::exception &) { @@ -2874,6 +2862,8 @@ int OGROSMDataSource::Open(const char *pszFilename, char **papszOpenOptionsIn) m_nMaxSizeForInMemoryDBInMB = atoi(CSLFetchNameValueDef( papszOpenOptionsIn, "MAX_TMPFILE_SIZE", CPLGetConfigOption("OSM_MAX_TMPFILE_SIZE", "100"))); + if (m_nMaxSizeForInMemoryDBInMB == 0) + m_nMaxSizeForInMemoryDBInMB = 1; GIntBig nSize = static_cast(m_nMaxSizeForInMemoryDBInMB) * 1024 * 1024; if (nSize < 0 || @@ -2904,14 +2894,13 @@ int OGROSMDataSource::Open(const char *pszFilename, char **papszOpenOptionsIn) } CPLPushErrorHandler(CPLQuietErrorHandler); - bool bSuccess = - VSIFSeekL(m_fpNodes, static_cast(nSize * 3 / 4), - SEEK_SET) == 0; + const bool bSuccess = + VSIFTruncateL(m_fpNodes, + static_cast(nSize * 3 / 4)) == 0; CPLPopErrorHandler(); if (bSuccess) { - VSIFSeekL(m_fpNodes, 0, SEEK_SET); VSIFTruncateL(m_fpNodes, 0); } else @@ -2953,7 +2942,7 @@ int OGROSMDataSource::Open(const char *pszFilename, char **papszOpenOptionsIn) CPLString osInterestLayers = GetInterestLayersForDSName(GetName()); if (!osInterestLayers.empty()) { - delete ExecuteSQL(osInterestLayers, nullptr, nullptr); + ReleaseResultSet(ExecuteSQL(osInterestLayers, nullptr, nullptr)); } } return bRet; @@ -2990,14 +2979,14 @@ bool OGROSMDataSource::CreateTempDB() VSILFILE *fp = VSIFOpenL(m_osTmpDBName, "wb"); if (fp) { - GIntBig nSize = - static_cast(m_nMaxSizeForInMemoryDBInMB) * 1024 * 1024; + vsi_l_offset nSize = + static_cast(m_nMaxSizeForInMemoryDBInMB) * 1024 * + 1024; if (m_bCustomIndexing && m_bInMemoryNodesFile) nSize = nSize / 4; CPLPushErrorHandler(CPLQuietErrorHandler); - bSuccess = - VSIFSeekL(fp, static_cast(nSize), SEEK_SET) == 0; + bSuccess = VSIFTruncateL(fp, nSize) == 0; CPLPopErrorHandler(); if (bSuccess) @@ -3371,13 +3360,12 @@ bool OGROSMDataSource::CommitTransactionCacheDB() void OGROSMDataSource::AddComputedAttributes( int iCurLayer, const std::vector &oAttributes) { - for (size_t i = 0; i < oAttributes.size(); i++) + for (const auto &oAttribute : oAttributes) { - if (!oAttributes[i].osSQL.empty()) + if (!oAttribute.osSQL.empty()) { - m_apoLayers[iCurLayer]->AddComputedAttribute(oAttributes[i].osName, - oAttributes[i].eType, - oAttributes[i].osSQL); + m_apoLayers[iCurLayer]->AddComputedAttribute( + oAttribute.osName, oAttribute.eType, oAttribute.osSQL); } } } @@ -3386,7 +3374,7 @@ void OGROSMDataSource::AddComputedAttributes( /* ParseConf() */ /************************************************************************/ -bool OGROSMDataSource::ParseConf(char **papszOpenOptionsIn) +bool OGROSMDataSource::ParseConf(CSLConstList papszOpenOptionsIn) { const char *pszFilename = CSLFetchNameValueDef(papszOpenOptionsIn, "CONFIG_FILE", @@ -3859,21 +3847,20 @@ int OGROSMDataSource::MyResetReading() memset(m_pabySector, 0, SECTOR_SIZE); - std::map::iterator oIter = m_oMapBuckets.begin(); - for (; oIter != m_oMapBuckets.end(); ++oIter) + for (auto &oIter : m_oMapBuckets) { - Bucket *psBucket = &(oIter->second); - psBucket->nOff = -1; + Bucket &sBucket = oIter.second; + sBucket.nOff = -1; if (m_bCompressNodes) { - if (psBucket->u.panSectorSize) - memset(psBucket->u.panSectorSize, 0, + if (sBucket.u.panSectorSize) + memset(sBucket.u.panSectorSize, 0, BUCKET_SECTOR_SIZE_ARRAY_SIZE); } else { - if (psBucket->u.pabyBitmap) - memset(psBucket->u.pabyBitmap, 0, BUCKET_BITMAP_SIZE); + if (sBucket.u.pabyBitmap) + memset(sBucket.u.pabyBitmap, 0, BUCKET_BITMAP_SIZE); } } } @@ -4489,12 +4476,9 @@ OGRLayer *OGROSMDataSource::ExecuteSQL(const char *pszSQLCommand, if (pszDialect != nullptr && EQUAL(pszDialect, "SQLITE")) { - std::set oSetLayers = - OGRSQLiteGetReferencedLayers(pszSQLCommand); - std::set::iterator oIter = oSetLayers.begin(); - for (; oIter != oSetLayers.end(); ++oIter) + const auto oSetLayers = OGRSQLiteGetReferencedLayers(pszSQLCommand); + for (const LayerDesc &oLayerDesc : oSetLayers) { - const LayerDesc &oLayerDesc = *oIter; if (oLayerDesc.osDSName.empty()) { if (bLayerAlreadyAdded) diff --git a/ogr/ogrsf_frmts/osm/ogrosmlayer.cpp b/ogr/ogrsf_frmts/osm/ogrosmlayer.cpp index aec8754b49a5..c590c98c18aa 100644 --- a/ogr/ogrsf_frmts/osm/ogrosmlayer.cpp +++ b/ogr/ogrsf_frmts/osm/ogrosmlayer.cpp @@ -576,7 +576,8 @@ static const char *GetValueOfTag(const char *pszKeyToSearch, unsigned int nTags, void OGROSMLayer::SetFieldsFromTags(OGRFeature *poFeature, GIntBig nID, bool bIsWayID, unsigned int nTags, - OSMTag *pasTags, OSMInfo *psInfo) + const OSMTag *pasTags, + const OSMInfo *psInfo) { if (!bIsWayID) { diff --git a/ogr/ogrsf_frmts/parquet/ogrparquetlayer.cpp b/ogr/ogrsf_frmts/parquet/ogrparquetlayer.cpp index e0c33f578788..d001813141e0 100644 --- a/ogr/ogrsf_frmts/parquet/ogrparquetlayer.cpp +++ b/ogr/ogrsf_frmts/parquet/ogrparquetlayer.cpp @@ -212,7 +212,7 @@ bool OGRParquetLayerBase::DealWithGeometryColumn( std::string osExtensionName; if (field_kv_metadata) { - auto extension_name = field_kv_metadata->Get("ARROW:extension:name"); + auto extension_name = field_kv_metadata->Get(ARROW_EXTENSION_NAME_KEY); if (extension_name.ok()) { osExtensionName = *extension_name; diff --git a/port/cpl_vsi_mem.cpp b/port/cpl_vsi_mem.cpp index a4ccaedd9266..5cfd8be55714 100644 --- a/port/cpl_vsi_mem.cpp +++ b/port/cpl_vsi_mem.cpp @@ -139,7 +139,6 @@ class VSIMemHandle final : public VSIVirtualHandle bool bUpdate = false; bool bEOF = false; bool m_bError = false; - bool bExtendFileAtNextWrite = false; VSIMemHandle() = default; ~VSIMemHandle() override; @@ -266,13 +265,21 @@ bool VSIMemFile::SetLength(vsi_l_offset nNewLength) return false; } - const vsi_l_offset nNewAlloc = (nNewLength + nNewLength / 10) + 5000; + // If the first allocation is 1 MB or above, just take that value + // as the one to allocate + // Otherwise slightly reserve more to avoid too frequent reallocations. + const vsi_l_offset nNewAlloc = + (nAllocLength == 0 && nNewLength >= 1024 * 1024) + ? nNewLength + : nNewLength + nNewLength / 10 + 5000; GByte *pabyNewData = nullptr; if (static_cast(static_cast(nNewAlloc)) == nNewAlloc) { pabyNewData = static_cast( - VSIRealloc(pabyData, static_cast(nNewAlloc))); + nAllocLength == 0 + ? VSICalloc(1, static_cast(nNewAlloc)) + : VSIRealloc(pabyData, static_cast(nNewAlloc))); } if (pabyNewData == nullptr) { @@ -283,9 +290,14 @@ bool VSIMemFile::SetLength(vsi_l_offset nNewLength) return false; } - // Clear the new allocated part of the buffer. - memset(pabyNewData + nAllocLength, 0, - static_cast(nNewAlloc - nAllocLength)); + if (nAllocLength > 0) + { + // Clear the new allocated part of the buffer (only needed if + // there was already reserved memory, otherwise VSICalloc() has + // zeroized it already) + memset(pabyNewData + nAllocLength, 0, + static_cast(nNewAlloc - nAllocLength)); + } pabyData = pabyNewData; nAllocLength = nNewAlloc; @@ -350,7 +362,6 @@ int VSIMemHandle::Seek(vsi_l_offset nOffset, int nWhence) nLength = poFile->nLength; } - bExtendFileAtNextWrite = false; if (nWhence == SEEK_CUR) { if (nOffset > INT_MAX) @@ -375,14 +386,6 @@ int VSIMemHandle::Seek(vsi_l_offset nOffset, int nWhence) bEOF = false; - if (m_nOffset > nLength) - { - if (bUpdate) // Writable files are zero-extended by seek past end. - { - bExtendFileAtNextWrite = true; - } - } - return 0; } @@ -495,14 +498,6 @@ size_t VSIMemHandle::Write(const void *pBuffer, size_t nSize, size_t nCount) const size_t nBytesToWrite = nSize * nCount; - if (bExtendFileAtNextWrite) - { - bExtendFileAtNextWrite = false; - CPL_EXCLUSIVE_LOCK oLock(poFile->m_oMutex); - if (!poFile->SetLength(nOffset)) - return 0; - } - { CPL_EXCLUSIVE_LOCK oLock(poFile->m_oMutex); @@ -578,8 +573,6 @@ int VSIMemHandle::Truncate(vsi_l_offset nNewSize) return -1; } - bExtendFileAtNextWrite = false; - CPL_EXCLUSIVE_LOCK oLock(poFile->m_oMutex); if (poFile->SetLength(nNewSize)) return 0; diff --git a/swig/include/python/typemaps_python.i b/swig/include/python/typemaps_python.i index ef5276f3cc78..1a20779192c1 100644 --- a/swig/include/python/typemaps_python.i +++ b/swig/include/python/typemaps_python.i @@ -225,6 +225,15 @@ TYPEMAP_ARGOUT_ARGOUT_ARRAY_IS_VALID(6) /* %typemap(out) IF_ERROR_RETURN_NONE */ } +%typemap(ret) IF_ERROR_RETURN_NONE +{ + /* %typemap(ret) IF_ERROR_RETURN_NONE */ + if ($1 != CE_None ) { + Py_XDECREF( $result ); + $result = Py_None; + Py_INCREF($result); + } +} %import "ogr_error_map.i" diff --git a/swig/python/gdal-utils/osgeo_utils/samples/gdalinfo.py b/swig/python/gdal-utils/osgeo_utils/samples/gdalinfo.py index ed65ac82fb08..326b21ed6d2a 100644 --- a/swig/python/gdal-utils/osgeo_utils/samples/gdalinfo.py +++ b/swig/python/gdal-utils/osgeo_utils/samples/gdalinfo.py @@ -402,9 +402,8 @@ def main(argv=None): print(line) stats = hBand.GetStatistics(bApproxStats, bStats) - # Dirty hack to recognize if stats are valid. If invalid, the returned - # stddev is negative - if stats[3] >= 0.0: + # Before GDAL 3.10, a negative value for stddev indicated an error + if stats is not None and stats[3] >= 0.0: print( " Minimum=%.3f, Maximum=%.3f, Mean=%.3f, StdDev=%.3f" % (stats[0], stats[1], stats[2], stats[3]) diff --git a/swig/python/modify_cpp_files.cmake b/swig/python/modify_cpp_files.cmake index d5850c04abc9..af6668b0189e 100644 --- a/swig/python/modify_cpp_files.cmake +++ b/swig/python/modify_cpp_files.cmake @@ -33,12 +33,6 @@ string(REPLACE "obj = PyUnicode_AsUTF8String(obj);" "obj = PyUnicode_AsUTF8String(obj); if (!obj) return SWIG_TypeError;" _CONTENTS "${_CONTENTS}") -if("${FILE}" MATCHES "gdal_wrap.cpp") - string(REGEX REPLACE "result = \\(CPLErr\\)([^;]+)(\\;)" - [[CPL_IGNORE_RET_VAL(result = (CPLErr)\1)\2]] - _CONTENTS "${_CONTENTS}") -endif() - string(REPLACE "PyObject *resultobj = 0;" "PyObject *resultobj = 0; int bLocalUseExceptionsCode = GetUseExceptions();" _CONTENTS "${_CONTENTS}")