From 99b1c0e7a909fd56b7e36cb650fcf096fc833549 Mon Sep 17 00:00:00 2001 From: Even Rouault Date: Tue, 4 Feb 2025 10:52:55 +0100 Subject: [PATCH 1/6] Zarr V3: update with latest Zarr V3 specification - Rename 'endian' codec to 'bytes' - Always generate a "codecs" array with the "bytes" codec in it. On reading, be tolerant with its absence but raise a warning as it being deprecated - On reading, raise a warning on implicit groups, as being deprecated - Support the 'zstd' codec, which isn't yet standardized, but is the default one used by Python zarr v3.0.2 Not handled: 'crc32c' and 'sharding' codecs --- .../data/zarr/v3/test.zr3/ar/zarr.json | 3 +- .../zarr/v3/test.zr3/marvin/android/zarr.json | 3 +- .../v3/test.zr3/marvin/paranoid/zarr.json | 4 + .../v3/test_deprecated_no_codecs.zr3/ar/c/0 | 1 + .../ar/zarr.json | 23 ++ .../marvin/android/0.0 | 1 + .../marvin/android/zarr.json | 22 ++ .../marvin/paranoid/DO_NOT_REMOVE_ME | 0 .../marvin/zarr.json | 7 + .../test_deprecated_no_codecs.zr3/zarr.json | 7 + autotest/gdrivers/zarr_driver.py | 55 ++- doc/source/drivers/raster/zarr.rst | 3 + frmts/zarr/zarr.h | 99 +++--- frmts/zarr/zarr_v3_codec.cpp | 326 +++++++++++------- frmts/zarr/zarr_v3_group.cpp | 27 +- fuzzers/build_seed_corpus.sh | 3 +- port/cpl_compressor.cpp | 24 +- 17 files changed, 407 insertions(+), 201 deletions(-) create mode 100644 autotest/gdrivers/data/zarr/v3/test.zr3/marvin/paranoid/zarr.json create mode 100644 autotest/gdrivers/data/zarr/v3/test_deprecated_no_codecs.zr3/ar/c/0 create mode 100644 autotest/gdrivers/data/zarr/v3/test_deprecated_no_codecs.zr3/ar/zarr.json create mode 100644 autotest/gdrivers/data/zarr/v3/test_deprecated_no_codecs.zr3/marvin/android/0.0 create mode 100644 autotest/gdrivers/data/zarr/v3/test_deprecated_no_codecs.zr3/marvin/android/zarr.json rename autotest/gdrivers/data/zarr/v3/{test.zr3 => test_deprecated_no_codecs.zr3}/marvin/paranoid/DO_NOT_REMOVE_ME (100%) create mode 100644 autotest/gdrivers/data/zarr/v3/test_deprecated_no_codecs.zr3/marvin/zarr.json create mode 100644 autotest/gdrivers/data/zarr/v3/test_deprecated_no_codecs.zr3/zarr.json diff --git a/autotest/gdrivers/data/zarr/v3/test.zr3/ar/zarr.json b/autotest/gdrivers/data/zarr/v3/test.zr3/ar/zarr.json index 8dec6c80d052..aa255257d709 100644 --- a/autotest/gdrivers/data/zarr/v3/test.zr3/ar/zarr.json +++ b/autotest/gdrivers/data/zarr/v3/test.zr3/ar/zarr.json @@ -19,5 +19,6 @@ "separator":"\/" } }, - "fill_value": 255 + "fill_value": 255, + "codecs": [{"name":"bytes"}] } diff --git a/autotest/gdrivers/data/zarr/v3/test.zr3/marvin/android/zarr.json b/autotest/gdrivers/data/zarr/v3/test.zr3/marvin/android/zarr.json index 1e423fa8f742..7ccfe257d12d 100644 --- a/autotest/gdrivers/data/zarr/v3/test.zr3/marvin/android/zarr.json +++ b/autotest/gdrivers/data/zarr/v3/test.zr3/marvin/android/zarr.json @@ -18,5 +18,6 @@ "chunk_key_encoding":{ "name":"v2" }, - "fill_value":255 + "fill_value":255, + "codecs": [{"name":"bytes"}] } diff --git a/autotest/gdrivers/data/zarr/v3/test.zr3/marvin/paranoid/zarr.json b/autotest/gdrivers/data/zarr/v3/test.zr3/marvin/paranoid/zarr.json new file mode 100644 index 000000000000..c9c096a15b06 --- /dev/null +++ b/autotest/gdrivers/data/zarr/v3/test.zr3/marvin/paranoid/zarr.json @@ -0,0 +1,4 @@ +{ + "zarr_format":3, + "node_type":"group" +} diff --git a/autotest/gdrivers/data/zarr/v3/test_deprecated_no_codecs.zr3/ar/c/0 b/autotest/gdrivers/data/zarr/v3/test_deprecated_no_codecs.zr3/ar/c/0 new file mode 100644 index 000000000000..71bd63e62027 --- /dev/null +++ b/autotest/gdrivers/data/zarr/v3/test_deprecated_no_codecs.zr3/ar/c/0 @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/autotest/gdrivers/data/zarr/v3/test_deprecated_no_codecs.zr3/ar/zarr.json b/autotest/gdrivers/data/zarr/v3/test_deprecated_no_codecs.zr3/ar/zarr.json new file mode 100644 index 000000000000..8dec6c80d052 --- /dev/null +++ b/autotest/gdrivers/data/zarr/v3/test_deprecated_no_codecs.zr3/ar/zarr.json @@ -0,0 +1,23 @@ +{ + "zarr_format":3, + "node_type":"array", + "shape":[ + 2 + ], + "data_type":"uint8", + "chunk_grid":{ + "name":"regular", + "configuration":{ + "chunk_shape":[ + 2 + ] + } + }, + "chunk_key_encoding":{ + "name":"default", + "configuration":{ + "separator":"\/" + } + }, + "fill_value": 255 +} diff --git a/autotest/gdrivers/data/zarr/v3/test_deprecated_no_codecs.zr3/marvin/android/0.0 b/autotest/gdrivers/data/zarr/v3/test_deprecated_no_codecs.zr3/marvin/android/0.0 new file mode 100644 index 000000000000..b0a3118514f3 --- /dev/null +++ b/autotest/gdrivers/data/zarr/v3/test_deprecated_no_codecs.zr3/marvin/android/0.0 @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/autotest/gdrivers/data/zarr/v3/test_deprecated_no_codecs.zr3/marvin/android/zarr.json b/autotest/gdrivers/data/zarr/v3/test_deprecated_no_codecs.zr3/marvin/android/zarr.json new file mode 100644 index 000000000000..1e423fa8f742 --- /dev/null +++ b/autotest/gdrivers/data/zarr/v3/test_deprecated_no_codecs.zr3/marvin/android/zarr.json @@ -0,0 +1,22 @@ +{ + "zarr_format":3, + "node_type":"array", + "shape":[ + 5, + 4 + ], + "data_type":"uint8", + "chunk_grid":{ + "name":"regular", + "configuration":{ + "chunk_shape":[ + 5, + 4 + ] + } + }, + "chunk_key_encoding":{ + "name":"v2" + }, + "fill_value":255 +} diff --git a/autotest/gdrivers/data/zarr/v3/test.zr3/marvin/paranoid/DO_NOT_REMOVE_ME b/autotest/gdrivers/data/zarr/v3/test_deprecated_no_codecs.zr3/marvin/paranoid/DO_NOT_REMOVE_ME similarity index 100% rename from autotest/gdrivers/data/zarr/v3/test.zr3/marvin/paranoid/DO_NOT_REMOVE_ME rename to autotest/gdrivers/data/zarr/v3/test_deprecated_no_codecs.zr3/marvin/paranoid/DO_NOT_REMOVE_ME diff --git a/autotest/gdrivers/data/zarr/v3/test_deprecated_no_codecs.zr3/marvin/zarr.json b/autotest/gdrivers/data/zarr/v3/test_deprecated_no_codecs.zr3/marvin/zarr.json new file mode 100644 index 000000000000..410a0e31303f --- /dev/null +++ b/autotest/gdrivers/data/zarr/v3/test_deprecated_no_codecs.zr3/marvin/zarr.json @@ -0,0 +1,7 @@ +{ + "zarr_format":3, + "node_type":"group", + "attributes":{ + "foo":"bar" + } +} \ No newline at end of file diff --git a/autotest/gdrivers/data/zarr/v3/test_deprecated_no_codecs.zr3/zarr.json b/autotest/gdrivers/data/zarr/v3/test_deprecated_no_codecs.zr3/zarr.json new file mode 100644 index 000000000000..164d1a096862 --- /dev/null +++ b/autotest/gdrivers/data/zarr/v3/test_deprecated_no_codecs.zr3/zarr.json @@ -0,0 +1,7 @@ +{ + "zarr_format":3, + "node_type":"group", + "attributes":{ + "root_foo":"bar" + } +} \ No newline at end of file diff --git a/autotest/gdrivers/zarr_driver.py b/autotest/gdrivers/zarr_driver.py index e3292285a128..a0587b58e9b4 100644 --- a/autotest/gdrivers/zarr_driver.py +++ b/autotest/gdrivers/zarr_driver.py @@ -982,11 +982,13 @@ def test_zarr_read_ARRAY_DIMENSIONS(use_zmetadata, filename): assert len(rg.GetDimensions()) == 2 +@pytest.mark.parametrize( + "ds_name", ["data/zarr/v3/test_deprecated_no_codecs.zr3", "data/zarr/v3/test.zr3"] +) @pytest.mark.parametrize("use_get_names", [True, False]) -def test_zarr_read_v3(use_get_names): +def test_zarr_read_v3(ds_name, use_get_names): - filename = "data/zarr/v3/test.zr3" - ds = gdal.OpenEx(filename, gdal.OF_MULTIDIM_RASTER) + ds = gdal.OpenEx(ds_name, gdal.OF_MULTIDIM_RASTER) assert ds is not None rg = ds.GetRootGroup() assert rg.GetName() == "/" @@ -1930,21 +1932,28 @@ def create(): @pytest.mark.parametrize( "compressor,options,expected_json", [ - ["NONE", [], None], + ["NONE", [], [{"name": "bytes", "configuration": {"endian": "little"}}]], [ "gzip", [], - [{"name": "gzip", "configuration": {"level": 6}}], + [ + {"name": "bytes", "configuration": {"endian": "little"}}, + {"name": "gzip", "configuration": {"level": 6}}, + ], ], [ "gzip", ["GZIP_LEVEL=1"], - [{"name": "gzip", "configuration": {"level": 1}}], + [ + {"name": "bytes", "configuration": {"endian": "little"}}, + {"name": "gzip", "configuration": {"level": 1}}, + ], ], [ "blosc", [], [ + {"name": "bytes", "configuration": {"endian": "little"}}, { "name": "blosc", "configuration": { @@ -1954,7 +1963,7 @@ def create(): "typesize": 1, "blocksize": 0, }, - } + }, ], ], [ @@ -1966,6 +1975,7 @@ def create(): "BLOSC_BLOCKSIZE=2", ], [ + {"name": "bytes", "configuration": {"endian": "little"}}, { "name": "blosc", "configuration": { @@ -1974,7 +1984,23 @@ def create(): "shuffle": "noshuffle", "blocksize": 2, }, - } + }, + ], + ], + [ + "zstd", + ["ZSTD_LEVEL=20"], + [ + {"name": "bytes", "configuration": {"endian": "little"}}, + {"name": "zstd", "configuration": {"level": 20, "checksum": False}}, + ], + ], + [ + "zstd", + ["ZSTD_CHECKSUM=YES"], + [ + {"name": "bytes", "configuration": {"endian": "little"}}, + {"name": "zstd", "configuration": {"level": 13, "checksum": True}}, ], ], ], @@ -2032,28 +2058,28 @@ def read(): [ [ ["@ENDIAN=little"], - [{"configuration": {"endian": "little"}, "name": "endian"}], + [{"configuration": {"endian": "little"}, "name": "bytes"}], ], - [["@ENDIAN=big"], [{"configuration": {"endian": "big"}, "name": "endian"}]], + [["@ENDIAN=big"], [{"configuration": {"endian": "big"}, "name": "bytes"}]], [ ["@ENDIAN=little", "CHUNK_MEMORY_LAYOUT=F"], [ {"name": "transpose", "configuration": {"order": "F"}}, - {"configuration": {"endian": "little"}, "name": "endian"}, + {"configuration": {"endian": "little"}, "name": "bytes"}, ], ], [ ["@ENDIAN=big", "CHUNK_MEMORY_LAYOUT=F"], [ {"name": "transpose", "configuration": {"order": "F"}}, - {"configuration": {"endian": "big"}, "name": "endian"}, + {"configuration": {"endian": "big"}, "name": "bytes"}, ], ], [ ["@ENDIAN=big", "CHUNK_MEMORY_LAYOUT=F", "COMPRESS=GZIP"], [ {"name": "transpose", "configuration": {"order": "F"}}, - {"name": "endian", "configuration": {"endian": "big"}}, + {"name": "bytes", "configuration": {"endian": "big"}}, {"name": "gzip", "configuration": {"level": 6}}, ], ], @@ -3180,6 +3206,7 @@ def create(): assert "codecs" in j assert j["codecs"] == [ {"name": "transpose", "configuration": {"order": "F"}}, + {"name": "bytes", "configuration": {"endian": "little"}}, {"name": "gzip", "configuration": {"level": 6}}, ] @@ -5512,7 +5539,7 @@ def test_zarr_driver_copy_files(format): assert gdal.Open(filename) assert gdal.VSIStatL(newfilename) - print(gdal.ReadDirRecursive(newfilename)) + # print(gdal.ReadDirRecursive(newfilename)) assert gdal.Open(newfilename) finally: diff --git a/doc/source/drivers/raster/zarr.rst b/doc/source/drivers/raster/zarr.rst index 9f636a6813cb..06dbc93a9bab 100644 --- a/doc/source/drivers/raster/zarr.rst +++ b/doc/source/drivers/raster/zarr.rst @@ -348,6 +348,9 @@ with ``ARRAY:`` using :program:`gdalmdimtranslate`): Compression method. + For FORMAT=ZARR_V3, only ``NONE``, ``BLOSC``, ``GZIP`` and ``ZSTD`` are + supported. + - .. co:: FILTER :choices: NONE, DELTA :default: NONE diff --git a/frmts/zarr/zarr.h b/frmts/zarr/zarr.h index 8cd675b46dd1..3285da6be97b 100644 --- a/frmts/zarr/zarr.h +++ b/frmts/zarr/zarr.h @@ -1207,25 +1207,24 @@ class ZarrV3Codec CPL_NON_FINAL }; /************************************************************************/ -/* ZarrV3CodecGZip */ +/* ZarrV3CodecAbstractCompressor */ /************************************************************************/ -// Implements https://zarr-specs.readthedocs.io/en/latest/v3/codecs/gzip/v1.0.html -class ZarrV3CodecGZip final : public ZarrV3Codec +class ZarrV3CodecAbstractCompressor CPL_NON_FINAL : public ZarrV3Codec { + protected: CPLStringList m_aosCompressorOptions{}; const CPLCompressor *m_pDecompressor = nullptr; const CPLCompressor *m_pCompressor = nullptr; - ZarrV3CodecGZip(const ZarrV3CodecGZip &) = delete; - ZarrV3CodecGZip &operator=(const ZarrV3CodecGZip &) = delete; - - public: - static constexpr const char *NAME = "gzip"; + explicit ZarrV3CodecAbstractCompressor(const std::string &osName); - ZarrV3CodecGZip(); - ~ZarrV3CodecGZip() override; + ZarrV3CodecAbstractCompressor(const ZarrV3CodecAbstractCompressor &) = + delete; + ZarrV3CodecAbstractCompressor & + operator=(const ZarrV3CodecAbstractCompressor &) = delete; + public: IOType GetInputType() const override { return IOType::BYTES; @@ -1236,6 +1235,24 @@ class ZarrV3CodecGZip final : public ZarrV3Codec return IOType::BYTES; } + bool Encode(const ZarrByteVectorQuickResize &abySrc, + ZarrByteVectorQuickResize &abyDst) const override; + bool Decode(const ZarrByteVectorQuickResize &abySrc, + ZarrByteVectorQuickResize &abyDst) const override; +}; + +/************************************************************************/ +/* ZarrV3CodecGZip */ +/************************************************************************/ + +// Implements https://zarr-specs.readthedocs.io/en/latest/v3/codecs/gzip/v1.0.html +class ZarrV3CodecGZip final : public ZarrV3CodecAbstractCompressor +{ + public: + static constexpr const char *NAME = "gzip"; + + ZarrV3CodecGZip(); + static CPLJSONObject GetConfiguration(int nLevel); bool @@ -1244,11 +1261,6 @@ class ZarrV3CodecGZip final : public ZarrV3Codec ZarrArrayMetadata &oOutputArrayMetadata) override; std::unique_ptr Clone() const override; - - bool Encode(const ZarrByteVectorQuickResize &abySrc, - ZarrByteVectorQuickResize &abyDst) const override; - bool Decode(const ZarrByteVectorQuickResize &abySrc, - ZarrByteVectorQuickResize &abyDst) const override; }; /************************************************************************/ @@ -1256,30 +1268,12 @@ class ZarrV3CodecGZip final : public ZarrV3Codec /************************************************************************/ // Implements https://zarr-specs.readthedocs.io/en/latest/v3/codecs/blosc/v1.0.html -class ZarrV3CodecBlosc final : public ZarrV3Codec +class ZarrV3CodecBlosc final : public ZarrV3CodecAbstractCompressor { - CPLStringList m_aosCompressorOptions{}; - const CPLCompressor *m_pDecompressor = nullptr; - const CPLCompressor *m_pCompressor = nullptr; - - ZarrV3CodecBlosc(const ZarrV3CodecBlosc &) = delete; - ZarrV3CodecBlosc &operator=(const ZarrV3CodecBlosc &) = delete; - public: static constexpr const char *NAME = "blosc"; ZarrV3CodecBlosc(); - ~ZarrV3CodecBlosc() override; - - IOType GetInputType() const override - { - return IOType::BYTES; - } - - IOType GetOutputType() const override - { - return IOType::BYTES; - } static CPLJSONObject GetConfiguration(const char *cname, int clevel, const char *shuffle, int typesize, @@ -1291,27 +1285,43 @@ class ZarrV3CodecBlosc final : public ZarrV3Codec ZarrArrayMetadata &oOutputArrayMetadata) override; std::unique_ptr Clone() const override; +}; - bool Encode(const ZarrByteVectorQuickResize &abySrc, - ZarrByteVectorQuickResize &abyDst) const override; - bool Decode(const ZarrByteVectorQuickResize &abySrc, - ZarrByteVectorQuickResize &abyDst) const override; +/************************************************************************/ +/* ZarrV3CodecZstd */ +/************************************************************************/ + +// Implements https://github.com/zarr-developers/zarr-specs/pull/256 +class ZarrV3CodecZstd final : public ZarrV3CodecAbstractCompressor +{ + public: + static constexpr const char *NAME = "zstd"; + + ZarrV3CodecZstd(); + + static CPLJSONObject GetConfiguration(int level, bool checksum); + + bool + InitFromConfiguration(const CPLJSONObject &configuration, + const ZarrArrayMetadata &oInputArrayMetadata, + ZarrArrayMetadata &oOutputArrayMetadata) override; + + std::unique_ptr Clone() const override; }; /************************************************************************/ -/* ZarrV3CodecEndian */ +/* ZarrV3CodecBytes */ /************************************************************************/ -// Implements https://zarr-specs.readthedocs.io/en/latest/v3/codecs/endian/v1.0.html -class ZarrV3CodecEndian final : public ZarrV3Codec +// Implements https://zarr-specs.readthedocs.io/en/latest/v3/codecs/bytes/v1.0.html +class ZarrV3CodecBytes final : public ZarrV3Codec { bool m_bLittle = true; public: - static constexpr const char *NAME = "endian"; + static constexpr const char *NAME = "bytes"; - ZarrV3CodecEndian(); - ~ZarrV3CodecEndian() override; + ZarrV3CodecBytes(); IOType GetInputType() const override { @@ -1374,7 +1384,6 @@ class ZarrV3CodecTranspose final : public ZarrV3Codec static constexpr const char *NAME = "transpose"; ZarrV3CodecTranspose(); - ~ZarrV3CodecTranspose() override; IOType GetInputType() const override { diff --git a/frmts/zarr/zarr_v3_codec.cpp b/frmts/zarr/zarr_v3_codec.cpp index 739d623da555..b2ba0418e9ef 100644 --- a/frmts/zarr/zarr_v3_codec.cpp +++ b/frmts/zarr/zarr_v3_codec.cpp @@ -29,18 +29,76 @@ ZarrV3Codec::ZarrV3Codec(const std::string &osName) : m_osName(osName) ZarrV3Codec::~ZarrV3Codec() = default; /************************************************************************/ -/* ZarrV3CodecGZip() */ +/* ZarrV3CodecAbstractCompressor() */ +/************************************************************************/ + +ZarrV3CodecAbstractCompressor::ZarrV3CodecAbstractCompressor( + const std::string &osName) + : ZarrV3Codec(osName) +{ +} + +/************************************************************************/ +/* ZarrV3CodecAbstractCompressor::Encode() */ +/************************************************************************/ + +bool ZarrV3CodecAbstractCompressor::Encode( + const ZarrByteVectorQuickResize &abySrc, + ZarrByteVectorQuickResize &abyDst) const +{ + abyDst.resize(abyDst.capacity()); + void *pOutputData = abyDst.data(); + size_t nOutputSize = abyDst.size(); + bool bRet = m_pCompressor->pfnFunc( + abySrc.data(), abySrc.size(), &pOutputData, &nOutputSize, + m_aosCompressorOptions.List(), m_pCompressor->user_data); + if (bRet) + { + abyDst.resize(nOutputSize); + } + else if (nOutputSize > abyDst.size()) + { + CPLError(CE_Failure, CPLE_AppDefined, + "%s codec:Encode(): output buffer too small", + m_osName.c_str()); + } + return bRet; +} + +/************************************************************************/ +/* ZarrV3CodecAbstractCompressor::Decode() */ /************************************************************************/ -ZarrV3CodecGZip::ZarrV3CodecGZip() : ZarrV3Codec(NAME) +bool ZarrV3CodecAbstractCompressor::Decode( + const ZarrByteVectorQuickResize &abySrc, + ZarrByteVectorQuickResize &abyDst) const { + abyDst.resize(abyDst.capacity()); + void *pOutputData = abyDst.data(); + size_t nOutputSize = abyDst.size(); + bool bRet = m_pDecompressor->pfnFunc(abySrc.data(), abySrc.size(), + &pOutputData, &nOutputSize, nullptr, + m_pDecompressor->user_data); + if (bRet) + { + abyDst.resize(nOutputSize); + } + else if (nOutputSize > abyDst.size()) + { + CPLError(CE_Failure, CPLE_AppDefined, + "%s codec:Decode(): output buffer too small", + m_osName.c_str()); + } + return bRet; } /************************************************************************/ -/* ~ZarrV3CodecGZip() */ +/* ZarrV3CodecGZip() */ /************************************************************************/ -ZarrV3CodecGZip::~ZarrV3CodecGZip() = default; +ZarrV3CodecGZip::ZarrV3CodecGZip() : ZarrV3CodecAbstractCompressor(NAME) +{ +} /************************************************************************/ /* GetConfiguration() */ @@ -136,68 +194,130 @@ std::unique_ptr ZarrV3CodecGZip::Clone() const } /************************************************************************/ -/* ZarrV3CodecGZip::Encode() */ +/* ZarrV3CodecZstd() */ /************************************************************************/ -bool ZarrV3CodecGZip::Encode(const ZarrByteVectorQuickResize &abySrc, - ZarrByteVectorQuickResize &abyDst) const +ZarrV3CodecZstd::ZarrV3CodecZstd() : ZarrV3CodecAbstractCompressor(NAME) { - abyDst.resize(abyDst.capacity()); - void *pOutputData = abyDst.data(); - size_t nOutputSize = abyDst.size(); - bool bRet = m_pCompressor->pfnFunc( - abySrc.data(), abySrc.size(), &pOutputData, &nOutputSize, - m_aosCompressorOptions.List(), m_pCompressor->user_data); - if (bRet) - { - abyDst.resize(nOutputSize); - } - else if (nOutputSize > abyDst.size()) - { - CPLError(CE_Failure, CPLE_AppDefined, - "ZarrV3CodecGZip::Encode(): output buffer too small"); - } - return bRet; } /************************************************************************/ -/* ZarrV3CodecGZip::Decode() */ +/* GetConfiguration() */ /************************************************************************/ -bool ZarrV3CodecGZip::Decode(const ZarrByteVectorQuickResize &abySrc, - ZarrByteVectorQuickResize &abyDst) const +/* static */ CPLJSONObject ZarrV3CodecZstd::GetConfiguration(int nLevel, + bool checksum) { - abyDst.resize(abyDst.capacity()); - void *pOutputData = abyDst.data(); - size_t nOutputSize = abyDst.size(); - bool bRet = m_pDecompressor->pfnFunc(abySrc.data(), abySrc.size(), - &pOutputData, &nOutputSize, nullptr, - m_pDecompressor->user_data); - if (bRet) + CPLJSONObject oConfig; + oConfig.Add("level", nLevel); + oConfig.Add("checksum", checksum); + return oConfig; +} + +/************************************************************************/ +/* ZarrV3CodecZstd::InitFromConfiguration() */ +/************************************************************************/ + +bool ZarrV3CodecZstd::InitFromConfiguration( + const CPLJSONObject &configuration, + const ZarrArrayMetadata &oInputArrayMetadata, + ZarrArrayMetadata &oOutputArrayMetadata) +{ + m_pCompressor = CPLGetCompressor("zstd"); + m_pDecompressor = CPLGetDecompressor("zstd"); + if (!m_pCompressor || !m_pDecompressor) { - abyDst.resize(nOutputSize); + CPLError(CE_Failure, CPLE_AppDefined, "zstd compressor not available"); + return false; } - else if (nOutputSize > abyDst.size()) + + m_oConfiguration = configuration.Clone(); + m_oInputArrayMetadata = oInputArrayMetadata; + // byte->byte codec + oOutputArrayMetadata = oInputArrayMetadata; + + int nLevel = 13; + bool bChecksum = false; + + if (configuration.IsValid()) { - CPLError(CE_Failure, CPLE_AppDefined, - "ZarrV3CodecGZip::Decode(): output buffer too small"); + if (configuration.GetType() != CPLJSONObject::Type::Object) + { + CPLError(CE_Failure, CPLE_AppDefined, + "Codec zstd: configuration is not an object"); + return false; + } + + for (const auto &oChild : configuration.GetChildren()) + { + if (oChild.GetName() != "level" && oChild.GetName() != "checksum") + { + CPLError( + CE_Failure, CPLE_AppDefined, + "Codec zstd: configuration contains a unhandled member: %s", + oChild.GetName().c_str()); + return false; + } + } + + const auto oLevel = configuration.GetObj("level"); + if (oLevel.IsValid()) + { + if (oLevel.GetType() != CPLJSONObject::Type::Integer) + { + CPLError(CE_Failure, CPLE_AppDefined, + "Codec zstd: level is not an integer"); + return false; + } + nLevel = oLevel.ToInteger(); + if (nLevel < 0 || nLevel > 22) + { + CPLError(CE_Failure, CPLE_AppDefined, + "Codec zstd: invalid value for level: %d", nLevel); + return false; + } + } + + const auto oChecksum = configuration.GetObj("checksum"); + if (oChecksum.IsValid()) + { + if (oChecksum.GetType() != CPLJSONObject::Type::Boolean) + { + CPLError(CE_Failure, CPLE_AppDefined, + "Codec zstd: checksum is not a boolean"); + return false; + } + bChecksum = oChecksum.ToBool(); + } } - return bRet; + + m_aosCompressorOptions.SetNameValue("LEVEL", CPLSPrintf("%d", nLevel)); + if (bChecksum) + m_aosCompressorOptions.SetNameValue("CKECKSUM", "YES"); + + return true; } /************************************************************************/ -/* ZarrV3CodecBlosc() */ +/* ZarrV3CodecZstd::Clone() */ /************************************************************************/ -ZarrV3CodecBlosc::ZarrV3CodecBlosc() : ZarrV3Codec(NAME) +std::unique_ptr ZarrV3CodecZstd::Clone() const { + auto psClone = std::make_unique(); + ZarrArrayMetadata oOutputArrayMetadata; + psClone->InitFromConfiguration(m_oConfiguration, m_oInputArrayMetadata, + oOutputArrayMetadata); + return psClone; } /************************************************************************/ -/* ~ZarrV3CodecBlosc() */ +/* ZarrV3CodecBlosc() */ /************************************************************************/ -ZarrV3CodecBlosc::~ZarrV3CodecBlosc() = default; +ZarrV3CodecBlosc::ZarrV3CodecBlosc() : ZarrV3CodecAbstractCompressor(NAME) +{ +} /************************************************************************/ /* GetConfiguration() */ @@ -355,74 +475,18 @@ std::unique_ptr ZarrV3CodecBlosc::Clone() const } /************************************************************************/ -/* ZarrV3CodecBlosc::Encode() */ -/************************************************************************/ - -bool ZarrV3CodecBlosc::Encode(const ZarrByteVectorQuickResize &abySrc, - ZarrByteVectorQuickResize &abyDst) const -{ - abyDst.resize(abyDst.capacity()); - void *pOutputData = abyDst.data(); - size_t nOutputSize = abyDst.size(); - bool bRet = m_pCompressor->pfnFunc( - abySrc.data(), abySrc.size(), &pOutputData, &nOutputSize, - m_aosCompressorOptions.List(), m_pCompressor->user_data); - if (bRet) - { - abyDst.resize(nOutputSize); - } - else if (nOutputSize > abyDst.size()) - { - CPLError(CE_Failure, CPLE_AppDefined, - "ZarrV3CodecBlosc::Encode(): output buffer too small"); - } - return bRet; -} - -/************************************************************************/ -/* ZarrV3CodecBlosc::Decode() */ -/************************************************************************/ - -bool ZarrV3CodecBlosc::Decode(const ZarrByteVectorQuickResize &abySrc, - ZarrByteVectorQuickResize &abyDst) const -{ - abyDst.resize(abyDst.capacity()); - void *pOutputData = abyDst.data(); - size_t nOutputSize = abyDst.size(); - bool bRet = m_pDecompressor->pfnFunc(abySrc.data(), abySrc.size(), - &pOutputData, &nOutputSize, nullptr, - m_pDecompressor->user_data); - if (bRet) - { - abyDst.resize(nOutputSize); - } - else if (nOutputSize > abyDst.size()) - { - CPLError(CE_Failure, CPLE_AppDefined, - "ZarrV3CodecBlosc::Decode(): output buffer too small"); - } - return bRet; -} - -/************************************************************************/ -/* ZarrV3CodecEndian() */ +/* ZarrV3CodecBytes() */ /************************************************************************/ -ZarrV3CodecEndian::ZarrV3CodecEndian() : ZarrV3Codec(NAME) +ZarrV3CodecBytes::ZarrV3CodecBytes() : ZarrV3Codec(NAME) { } -/************************************************************************/ -/* ~ZarrV3CodecEndian() */ -/************************************************************************/ - -ZarrV3CodecEndian::~ZarrV3CodecEndian() = default; - /************************************************************************/ /* GetConfiguration() */ /************************************************************************/ -/* static */ CPLJSONObject ZarrV3CodecEndian::GetConfiguration(bool bLittle) +/* static */ CPLJSONObject ZarrV3CodecBytes::GetConfiguration(bool bLittle) { CPLJSONObject oConfig; oConfig.Add("endian", bLittle ? "little" : "big"); @@ -430,10 +494,10 @@ ZarrV3CodecEndian::~ZarrV3CodecEndian() = default; } /************************************************************************/ -/* ZarrV3CodecEndian::InitFromConfiguration() */ +/* ZarrV3CodecBytes::InitFromConfiguration() */ /************************************************************************/ -bool ZarrV3CodecEndian::InitFromConfiguration( +bool ZarrV3CodecBytes::InitFromConfiguration( const CPLJSONObject &configuration, const ZarrArrayMetadata &oInputArrayMetadata, ZarrArrayMetadata &oOutputArrayMetadata) @@ -490,12 +554,12 @@ bool ZarrV3CodecEndian::InitFromConfiguration( } /************************************************************************/ -/* ZarrV3CodecEndian::Clone() */ +/* ZarrV3CodecBytes::Clone() */ /************************************************************************/ -std::unique_ptr ZarrV3CodecEndian::Clone() const +std::unique_ptr ZarrV3CodecBytes::Clone() const { - auto psClone = std::make_unique(); + auto psClone = std::make_unique(); ZarrArrayMetadata oOutputArrayMetadata; psClone->InitFromConfiguration(m_oConfiguration, m_oInputArrayMetadata, oOutputArrayMetadata); @@ -503,11 +567,11 @@ std::unique_ptr ZarrV3CodecEndian::Clone() const } /************************************************************************/ -/* ZarrV3CodecEndian::Encode() */ +/* ZarrV3CodecBytes::Encode() */ /************************************************************************/ -bool ZarrV3CodecEndian::Encode(const ZarrByteVectorQuickResize &abySrc, - ZarrByteVectorQuickResize &abyDst) const +bool ZarrV3CodecBytes::Encode(const ZarrByteVectorQuickResize &abySrc, + ZarrByteVectorQuickResize &abyDst) const { CPLAssert(!IsNoOp()); @@ -566,11 +630,11 @@ bool ZarrV3CodecEndian::Encode(const ZarrByteVectorQuickResize &abySrc, } /************************************************************************/ -/* ZarrV3CodecEndian::Decode() */ +/* ZarrV3CodecBytes::Decode() */ /************************************************************************/ -bool ZarrV3CodecEndian::Decode(const ZarrByteVectorQuickResize &abySrc, - ZarrByteVectorQuickResize &abyDst) const +bool ZarrV3CodecBytes::Decode(const ZarrByteVectorQuickResize &abySrc, + ZarrByteVectorQuickResize &abyDst) const { return Encode(abySrc, abyDst); } @@ -583,12 +647,6 @@ ZarrV3CodecTranspose::ZarrV3CodecTranspose() : ZarrV3Codec(NAME) { } -/************************************************************************/ -/* ~ZarrV3CodecTranspose() */ -/************************************************************************/ - -ZarrV3CodecTranspose::~ZarrV3CodecTranspose() = default; - /************************************************************************/ /* IsNoOp() */ /************************************************************************/ @@ -930,26 +988,33 @@ bool ZarrV3CodecSequence::InitFromJson(const CPLJSONObject &oCodecs) ZarrV3Codec::IOType eLastType = ZarrV3Codec::IOType::ARRAY; std::string osLastCodec; -#if !CPL_IS_LSB const auto InsertImplicitEndianCodecIfNeeded = - [this, &oInputArrayMetadata, &eLastType, &osLastCodec]() + [ +#if !CPL_IS_LSB + this, +#endif + &oInputArrayMetadata, &eLastType, &osLastCodec]() { - // Insert a little endian codec if we are on a big endian target if (eLastType == ZarrV3Codec::IOType::ARRAY && oInputArrayMetadata.oElt.nativeSize > 1) { - auto poEndianCodec = std::make_unique(); + CPLError(CE_Warning, CPLE_AppDefined, + "'bytes' codec missing. Assuming little-endian storage, " + "but such tolerance may be removed in future versions"); + auto poEndianCodec = std::make_unique(); ZarrArrayMetadata oOutputArrayMetadata; poEndianCodec->InitFromConfiguration( - ZarrV3CodecEndian::GetConfiguration(true), oInputArrayMetadata, + ZarrV3CodecBytes::GetConfiguration(true), oInputArrayMetadata, oOutputArrayMetadata); oInputArrayMetadata = oOutputArrayMetadata; eLastType = poEndianCodec->GetOutputType(); osLastCodec = poEndianCodec->GetName(); +#if !CPL_IS_LSB + // Insert a little endian codec if we are on a big endian target m_apoCodecs.emplace_back(std::move(poEndianCodec)); +#endif } }; -#endif for (const auto &oCodec : oCodecsArray) { @@ -964,8 +1029,11 @@ bool ZarrV3CodecSequence::InitFromJson(const CPLJSONObject &oCodecs) poCodec = std::make_unique(); else if (osName == "blosc") poCodec = std::make_unique(); - else if (osName == "endian") - poCodec = std::make_unique(); + else if (osName == "zstd") + poCodec = std::make_unique(); + else if (osName == "bytes" || + osName == "endian" /* endian is the old name */) + poCodec = std::make_unique(); else if (osName == "transpose") poCodec = std::make_unique(); else @@ -985,12 +1053,10 @@ bool ZarrV3CodecSequence::InitFromJson(const CPLJSONObject &oCodecs) return false; } } -#if !CPL_IS_LSB else { InsertImplicitEndianCodecIfNeeded(); } -#endif ZarrArrayMetadata oOutputArrayMetadata; if (!poCodec->InitFromConfiguration(oCodec["configuration"], @@ -1007,16 +1073,14 @@ bool ZarrV3CodecSequence::InitFromJson(const CPLJSONObject &oCodecs) m_apoCodecs.emplace_back(std::move(poCodec)); } -#if !CPL_IS_LSB InsertImplicitEndianCodecIfNeeded(); -#endif m_oCodecArray = oCodecs.Clone(); return true; } /************************************************************************/ -/* ZarrV3CodecEndian::AllocateBuffer() */ +/* ZarrV3CodecBytes::AllocateBuffer() */ /************************************************************************/ bool ZarrV3CodecSequence::AllocateBuffer(ZarrByteVectorQuickResize &abyBuffer) diff --git a/frmts/zarr/zarr_v3_group.cpp b/frmts/zarr/zarr_v3_group.cpp index 4a340778a3be..e149df5b84dc 100644 --- a/frmts/zarr/zarr_v3_group.cpp +++ b/frmts/zarr/zarr_v3_group.cpp @@ -248,6 +248,11 @@ ZarrV3Group::OpenZarrGroup(const std::string &osName, CSLConstList) const // Implicit group if (VSIStatL(osSubDir.c_str(), &sStat) == 0 && VSI_ISDIR(sStat.st_mode)) { + // Note: Python zarr v3.0.2 still generates implicit groups + // See https://github.com/zarr-developers/zarr-python/issues/2794 + CPLError(CE_Warning, CPLE_AppDefined, + "Support for Zarr V3 implicit group is now deprecated, and " + "may be removed in a future version"); auto poSubGroup = ZarrV3Group::Create(m_poSharedResource, GetFullName(), osName, osSubDir); poSubGroup->m_poParent = @@ -580,13 +585,13 @@ std::shared_ptr ZarrV3Group::CreateMDArray( oCodecs.Add(oCodec); } - // Not documented - const char *pszEndian = CSLFetchNameValue(papszOptions, "@ENDIAN"); - if (pszEndian) + // Not documented option, but 'bytes' codec is required + const char *pszEndian = + CSLFetchNameValueDef(papszOptions, "@ENDIAN", "little"); { CPLJSONObject oCodec; - oCodec.Add("name", "endian"); - oCodec.Add("configuration", ZarrV3CodecEndian::GetConfiguration( + oCodec.Add("name", "bytes"); + oCodec.Add("configuration", ZarrV3CodecBytes::GetConfiguration( EQUAL(pszEndian, "little"))); oCodecs.Add(oCodec); } @@ -655,6 +660,18 @@ std::shared_ptr ZarrV3Group::CreateMDArray( typesize, blocksize)); oCodecs.Add(oCodec); } + else if (EQUAL(pszCompressor, "ZSTD")) + { + CPLJSONObject oCodec; + oCodec.Add("name", "zstd"); + const char *pszLevel = + CSLFetchNameValueDef(papszOptions, "ZSTD_LEVEL", "13"); + const bool bChecksum = CPLTestBool( + CSLFetchNameValueDef(papszOptions, "ZSTD_CHECKSUM", "FALSE")); + oCodec.Add("configuration", ZarrV3CodecZstd::GetConfiguration( + atoi(pszLevel), bChecksum)); + oCodecs.Add(oCodec); + } else if (!EQUAL(pszCompressor, "NONE")) { CPLError(CE_Failure, CPLE_AppDefined, diff --git a/fuzzers/build_seed_corpus.sh b/fuzzers/build_seed_corpus.sh index 291fc011250d..f107e010e1c2 100755 --- a/fuzzers/build_seed_corpus.sh +++ b/fuzzers/build_seed_corpus.sh @@ -334,6 +334,7 @@ rm -f $OUT/zarr_fuzzer_seed_corpus.zip CUR_DIR=$PWD cd $(dirname $0)/../autotest/gdrivers/data/zarr for dirname in *.zarr v3/*.zr3; do + CUR_DIR2=$PWD cd $dirname { filelist=$(find . -type f) @@ -343,7 +344,7 @@ for dirname in *.zarr v3/*.zr3; do cat $f done } > $CUR_DIR/$(basename $dirname).tar - cd .. + cd $CUR_DIR2 done cd $CUR_DIR zip -r $OUT/zarr_fuzzer_seed_corpus.zip ./*.zarr.tar ./*.zr3.tar >/dev/null diff --git a/port/cpl_compressor.cpp b/port/cpl_compressor.cpp index 5f7a8b2b38b2..f721868020ed 100644 --- a/port/cpl_compressor.cpp +++ b/port/cpl_compressor.cpp @@ -373,7 +373,6 @@ static bool CPLZSTDCompressor(const void *input_data, size_t input_size, if (output_data != nullptr && *output_data != nullptr && output_size != nullptr && *output_size != 0) { - const int level = atoi(CSLFetchNameValueDef(options, "LEVEL", "13")); ZSTD_CCtx *ctx = ZSTD_createCCtx(); if (ctx == nullptr) { @@ -381,8 +380,24 @@ static bool CPLZSTDCompressor(const void *input_data, size_t input_size, return false; } - size_t ret = ZSTD_compressCCtx(ctx, *output_data, *output_size, - input_data, input_size, level); + const int level = atoi(CSLFetchNameValueDef(options, "LEVEL", "13")); + if (ZSTD_isError( + ZSTD_CCtx_setParameter(ctx, ZSTD_c_compressionLevel, level))) + { + CPLError(CE_Failure, CPLE_AppDefined, "Invalid compression level"); + ZSTD_freeCCtx(ctx); + *output_size = 0; + return false; + } + + if (CPLTestBool(CSLFetchNameValueDef(options, "CHECKSUM", "NO"))) + { + CPL_IGNORE_RET_VAL( + ZSTD_CCtx_setParameter(ctx, ZSTD_c_checksumFlag, 1)); + } + + size_t ret = ZSTD_compress2(ctx, *output_data, *output_size, input_data, + input_size); ZSTD_freeCCtx(ctx); if (ZSTD_isError(ret)) { @@ -1370,6 +1385,9 @@ static void CPLAddBuiltinCompressors() "OPTIONS=" " "; const char *const apszMetadata[] = {pszOptions, nullptr}; sComp.papszMetadata = apszMetadata; From 4c2e68a7617f7d6abef0eae928ba0eeed6f59dad Mon Sep 17 00:00:00 2001 From: Even Rouault Date: Tue, 4 Feb 2025 23:04:00 +0100 Subject: [PATCH 2/6] LIBKML: fix error when creating a Id field of type integer Fixes #11773 --- autotest/ogr/ogr_libkml.py | 22 ++++++++++++++ ogr/ogrsf_frmts/libkml/ogr_libkml.h | 9 +++--- .../libkml/ogrlibkmldatasource.cpp | 16 +++++----- ogr/ogrsf_frmts/libkml/ogrlibkmllayer.cpp | 29 ++++++++++--------- 4 files changed, 51 insertions(+), 25 deletions(-) diff --git a/autotest/ogr/ogr_libkml.py b/autotest/ogr/ogr_libkml.py index e0880763d8e5..ce0cacc66058 100755 --- a/autotest/ogr/ogr_libkml.py +++ b/autotest/ogr/ogr_libkml.py @@ -2341,3 +2341,25 @@ def ogr_libkml_non_editable(): lyr = ds.GetLayer(0) assert lyr.TestCapability(ogr.OLCRandomWrite) == 0 assert lyr.TestCapability(ogr.OLCDeleteFeature) == 0 + + +############################################################################### +# + + +def test_ogr_libkml_create_field_id_integer(tmp_vsimem): + + filename = str(tmp_vsimem / "test.kml") + with ogr.GetDriverByName("LIBKML").CreateDataSource(filename) as ds: + lyr = ds.CreateLayer("test") + lyr.CreateField(ogr.FieldDefn("id", ogr.OFTInteger)) + lyr.CreateField(ogr.FieldDefn("name")) + f = ogr.Feature(lyr.GetLayerDefn()) + f["id"] = 1 + f.SetGeometry(ogr.CreateGeometryFromWkt("POINT (1 2)")) + lyr.CreateFeature(f) + + with gdal.OpenEx(filename, gdal.OF_VECTOR | gdal.OF_UPDATE) as ds: + lyr = ds.GetLayer(0) + f = lyr.GetNextFeature() + assert f["id"] == "1" diff --git a/ogr/ogrsf_frmts/libkml/ogr_libkml.h b/ogr/ogrsf_frmts/libkml/ogr_libkml.h index 8d6fa8c0a172..48265111122e 100644 --- a/ogr/ogrsf_frmts/libkml/ogr_libkml.h +++ b/ogr/ogrsf_frmts/libkml/ogr_libkml.h @@ -32,7 +32,8 @@ CPLString OGRLIBKMLGetSanitizedNCName(const char *pszName); class OGRLIBKMLLayer final : public OGRLayer, public OGRGetNextFeatureThroughRaw { - int bUpdate = false; + bool m_bNew = false; + bool bUpdate = false; int nFeatures = 0; int iFeature = 0; @@ -84,7 +85,7 @@ class OGRLIBKMLLayer final : public OGRLayer, OGRLIBKMLDataSource *poOgrDS, kmldom::ElementPtr poKmlRoot, kmldom::ContainerPtr poKmlContainer, kmldom::UpdatePtr poKmlUpdate, const char *pszFileName, - int bNew, int bUpdate); + bool bNew, bool bUpdate); virtual ~OGRLIBKMLLayer(); void ResetReading() override @@ -265,7 +266,7 @@ class OGRLIBKMLDataSource final : public GDALDataset void SetStyleTableDirectly(OGRStyleTable *poStyleTable) override; void SetStyleTable(OGRStyleTable *poStyleTable) override; - int Open(const char *pszFilename, int bUpdate); + int Open(const char *pszFilename, bool bUpdate); int Create(const char *pszFilename, char **papszOptions); CPLErr FlushCache(bool bAtClosing) override; @@ -357,7 +358,7 @@ class OGRLIBKMLDataSource final : public GDALDataset AddLayer(const char *pszLayerName, OGRwkbGeometryType eGType, const OGRSpatialReference *poSRS, OGRLIBKMLDataSource *poOgrDS, kmldom::ElementPtr poKmlRoot, kmldom::ContainerPtr poKmlContainer, - const char *pszFileName, int bNew, int bUpdate, int nGuess); + const char *pszFileName, bool bNew, bool bUpdate, int nGuess); }; #endif diff --git a/ogr/ogrsf_frmts/libkml/ogrlibkmldatasource.cpp b/ogr/ogrsf_frmts/libkml/ogrlibkmldatasource.cpp index 66111ab7d51e..eb5f074627b7 100644 --- a/ogr/ogrsf_frmts/libkml/ogrlibkmldatasource.cpp +++ b/ogr/ogrsf_frmts/libkml/ogrlibkmldatasource.cpp @@ -846,7 +846,7 @@ OGRLIBKMLLayer *OGRLIBKMLDataSource::AddLayer( const char *pszLayerName, OGRwkbGeometryType eGType, const OGRSpatialReference *poSRS, OGRLIBKMLDataSource *poOgrDS, ElementPtr poKmlRoot, ContainerPtr poKmlContainer, const char *pszFileName, - int bNew, int bUpdateIn, int nGuess) + bool bNew, bool bUpdateIn, int nGuess) { // Build unique layer name CPLString osUniqueLayername(pszLayerName); @@ -947,7 +947,7 @@ int OGRLIBKMLDataSource::ParseLayers(ContainerPtr poKmlContainer, bool bRecurse) /***** create the layer *****/ AddLayer(oKmlFeatName.c_str(), wkbUnknown, nullptr, this, - nullptr, AsContainer(poKmlFeat), "", FALSE, bUpdate, + nullptr, AsContainer(poKmlFeat), "", false, bUpdate, static_cast(nKmlFeatures)); /***** check if any features are another layer *****/ @@ -1293,7 +1293,7 @@ int OGRLIBKMLDataSource::OpenKmz(const char *pszFilename, int bUpdateIn) AddLayer(osLayerName.c_str(), wkbUnknown, nullptr, this, std::move(poKmlLyrRoot), poKmlLyrContainer, - oKmlHref.get_path().c_str(), FALSE, bUpdateIn, + oKmlHref.get_path().c_str(), false, bUpdateIn, static_cast(nKmlFeatures)); /***** check if any features are another layer *****/ @@ -1335,7 +1335,7 @@ int OGRLIBKMLDataSource::OpenKmz(const char *pszFilename, int bUpdateIn) AddLayer(layername_default.c_str(), wkbUnknown, nullptr, this, std::move(poKmlDocKmlRoot), poKmlContainer, pszFilename, - FALSE, bUpdateIn, 1); + false, bUpdateIn, 1); } ParseLayers(std::move(poKmlContainer), true); @@ -1458,7 +1458,7 @@ int OGRLIBKMLDataSource::OpenDir(const char *pszFilename, int bUpdateIn) /***** create the layer *****/ AddLayer(osLayerName.c_str(), wkbUnknown, nullptr, this, std::move(poKmlRoot), poKmlContainer, osFilePath.c_str(), - FALSE, bUpdateIn, nFiles); + false, bUpdateIn, nFiles); /***** check if any features are another layer *****/ ParseLayers(std::move(poKmlContainer), true); @@ -1514,7 +1514,7 @@ static bool CheckIsKMZ(const char *pszFilename) return bFoundKML; } -int OGRLIBKMLDataSource::Open(const char *pszFilename, int bUpdateIn) +int OGRLIBKMLDataSource::Open(const char *pszFilename, bool bUpdateIn) { bUpdate = CPL_TO_BOOL(bUpdateIn); @@ -2171,7 +2171,7 @@ OGRLIBKMLLayer *OGRLIBKMLDataSource::CreateLayerKml( /***** create the layer *****/ OGRLIBKMLLayer *poOgrLayer = AddLayer(pszLayerName, eGType, poSRS, this, nullptr, - poKmlLayerContainer, "", TRUE, bUpdate, 1); + poKmlLayerContainer, "", true, bUpdate, 1); /***** add the layer name as a *****/ if (poKmlLayerContainer) @@ -2239,7 +2239,7 @@ OGRLIBKMLLayer *OGRLIBKMLDataSource::CreateLayerKmz( OGRLIBKMLLayer *poOgrLayer = AddLayer(pszLayerName, eGType, poSRS, this, nullptr, poKmlDocument, CPLFormFilenameSafe(nullptr, pszLayerName, ".kml").c_str(), - TRUE, bUpdate, 1); + true, bUpdate, 1); /***** add the layer name as a *****/ if (!m_poKmlUpdate) diff --git a/ogr/ogrsf_frmts/libkml/ogrlibkmllayer.cpp b/ogr/ogrsf_frmts/libkml/ogrlibkmllayer.cpp index 4590451e32c9..49b73dda2223 100644 --- a/ogr/ogrsf_frmts/libkml/ogrlibkmllayer.cpp +++ b/ogr/ogrsf_frmts/libkml/ogrlibkmllayer.cpp @@ -105,8 +105,8 @@ OGRLIBKMLLayer::OGRLIBKMLLayer( const char *pszLayerName, OGRwkbGeometryType eGType, const OGRSpatialReference *poSRSIn, OGRLIBKMLDataSource *poOgrDS, ElementPtr poKmlRoot, ContainerPtr poKmlContainer, UpdatePtr poKmlUpdate, - const char *pszFileName, int bNew, int bUpdateIn) - : bUpdate(CPL_TO_BOOL(bUpdateIn)), m_pszName(CPLStrdup(pszLayerName)), + const char *pszFileName, bool bNew, bool bUpdateIn) + : m_bNew(bNew), bUpdate(bUpdateIn), m_pszName(CPLStrdup(pszLayerName)), m_pszFileName(CPLStrdup(pszFileName)), m_poKmlLayer(std::move(poKmlContainer)), // Store the layers container. m_poKmlLayerRoot( @@ -536,19 +536,22 @@ OGRErr OGRLIBKMLLayer::ICreateFeature(OGRFeature *poOgrFeat) if (!bUpdate) return OGRERR_UNSUPPORTED_OPERATION; - const int idxIdField = - m_poOgrFeatureDefn->GetFieldIndex(m_oFieldConfig.idfield); - if (idxIdField >= 0 && poOgrFeat->IsFieldSet(idxIdField)) + if (!m_bNew) { - ScanAllFeatures(); - - if (cpl::contains(m_oMapKmlIdToOGRId, - poOgrFeat->GetFieldAsString(idxIdField))) + const int idxIdField = + m_poOgrFeatureDefn->GetFieldIndex(m_oFieldConfig.idfield); + if (idxIdField >= 0 && poOgrFeat->IsFieldSet(idxIdField)) { - CPLError(CE_Failure, CPLE_AppDefined, - "A feature with id %s already exists", - poOgrFeat->GetFieldAsString(idxIdField)); - return OGRERR_FAILURE; + ScanAllFeatures(); + + if (cpl::contains(m_oMapKmlIdToOGRId, + poOgrFeat->GetFieldAsString(idxIdField))) + { + CPLError(CE_Failure, CPLE_AppDefined, + "A feature with id %s already exists", + poOgrFeat->GetFieldAsString(idxIdField)); + return OGRERR_FAILURE; + } } } From eb09f0e008fbfdcefc66c7bdd900909e1425a0a7 Mon Sep 17 00:00:00 2001 From: Even Rouault Date: Fri, 7 Feb 2025 18:33:39 +0100 Subject: [PATCH 3/6] Python bindings: add syntaxic sugar to get/set algorithm arguments --- autotest/utilities/test_gdalalg_info.py | 12 +- autotest/utilities/test_gdalalg_pipeline.py | 5 +- .../utilities/test_gdalalg_raster_convert.py | 5 +- .../utilities/test_gdalalg_raster_edit.py | 6 +- .../utilities/test_gdalalg_raster_info.py | 34 ++-- .../utilities/test_gdalalg_raster_mosaic.py | 162 ++++++++---------- .../utilities/test_gdalalg_raster_overview.py | 38 ++-- .../utilities/test_gdalalg_raster_pipeline.py | 24 ++- .../utilities/test_gdalalg_vector_clip.py | 104 ++++++----- .../utilities/test_gdalalg_vector_convert.py | 11 +- .../utilities/test_gdalalg_vector_filter.py | 6 +- .../utilities/test_gdalalg_vector_info.py | 18 +- .../utilities/test_gdalalg_vector_pipeline.py | 30 ++-- swig/include/python/gdal_python.i | 92 +++++++++- 14 files changed, 298 insertions(+), 249 deletions(-) diff --git a/autotest/utilities/test_gdalalg_info.py b/autotest/utilities/test_gdalalg_info.py index 94d493a67e7d..1f3d157ac4b0 100755 --- a/autotest/utilities/test_gdalalg_info.py +++ b/autotest/utilities/test_gdalalg_info.py @@ -17,8 +17,7 @@ def get_info_alg(): - reg = gdal.GetGlobalAlgorithmRegistry() - return reg.InstantiateAlg("info") + return gdal.GetGlobalAlgorithmRegistry()["info"] @pytest.mark.parametrize( @@ -31,7 +30,7 @@ def get_info_alg(): def test_gdalalg_info_on_raster(args): info = get_info_alg() assert info.ParseRunAndFinalize(args) - output_string = info.GetActualAlgorithm().GetArg("output-string").Get() + output_string = info["output-string"] assert output_string.startswith("Driver: GTiff/GeoTIFF") @@ -57,7 +56,7 @@ def test_gdalalg_info_on_raster_invalid_arg(): def test_gdalalg_info_on_vector(args): info = get_info_alg() assert info.ParseRunAndFinalize(args) - output_string = info.GetActualAlgorithm().GetArg("output-string").Get() + output_string = info["output-string"] assert output_string.startswith("INFO: Open of") @@ -75,8 +74,7 @@ def test_gdalalg_info_invalid_arg(): def test_gdalalg_info_run_cannot_be_run(): info = get_info_alg() - ds = gdal.GetDriverByName("MEM").Create("", 1, 1) - info.GetArg("input").SetDataset(ds) + info["input"] = gdal.GetDriverByName("MEM").Create("", 1, 1) with pytest.raises(Exception, match="method should not be called directly"): info.Run() @@ -108,6 +106,6 @@ def test_gdalalg_info_mixed_raster_vector_with_invalid_arg(tmp_vsimem): def test_gdalalg_info_mixed_run_without_arg(tmp_vsimem): info = get_info_alg() - info.GetArg("input").Get().SetName("data/utmsmall.tif") + info["input"] = "data/utmsmall.tif" with pytest.raises(Exception, match="should not be called directly"): assert info.Run() diff --git a/autotest/utilities/test_gdalalg_pipeline.py b/autotest/utilities/test_gdalalg_pipeline.py index 6e3f80b898b4..5f0468b421ee 100755 --- a/autotest/utilities/test_gdalalg_pipeline.py +++ b/autotest/utilities/test_gdalalg_pipeline.py @@ -17,8 +17,7 @@ def get_pipeline_alg(): - reg = gdal.GetGlobalAlgorithmRegistry() - return reg.InstantiateAlg("pipeline") + return gdal.GetGlobalAlgorithmRegistry()["pipeline"] def test_gdalalg_pipeline_read_and_write(tmp_vsimem): @@ -44,6 +43,6 @@ def my_progress(pct, msg, user_data): def test_gdalalg_pipeline_mixed_run_without_arg(tmp_vsimem): pipeline = get_pipeline_alg() - pipeline.GetArg("input").Get().SetDataset(gdal.OpenEx("../ogr/data/poly.shp")) + pipeline["input"] = gdal.OpenEx("../ogr/data/poly.shp") with pytest.raises(Exception, match="should not be called directly"): assert pipeline.Run() diff --git a/autotest/utilities/test_gdalalg_raster_convert.py b/autotest/utilities/test_gdalalg_raster_convert.py index a774972f1645..739ad35eebd0 100755 --- a/autotest/utilities/test_gdalalg_raster_convert.py +++ b/autotest/utilities/test_gdalalg_raster_convert.py @@ -53,7 +53,7 @@ def test_gdalalg_raster_convert_to_mem(): convert = get_convert_alg() assert convert.ParseCommandLineArguments(["--of=MEM", "data/utmsmall.tif", ""]) assert convert.Run() - out_ds = convert.GetArg("output").Get().GetDataset() + out_ds = convert["output"].GetDataset() assert out_ds.GetRasterBand(1).Checksum() == 50054 @@ -82,8 +82,7 @@ def test_gdalalg_raster_convert_append(tmp_vsimem): def test_gdalalg_raster_convert_error_output_already_set(): convert = get_convert_alg() - ds = gdal.GetDriverByName("MEM").Create("", 1, 1) - convert.GetArg("output").Get().SetDataset(ds) + convert["output"] = gdal.GetDriverByName("MEM").Create("", 1, 1) assert convert.ParseCommandLineArguments(["data/utmsmall.tif"]) with pytest.raises( Exception, diff --git a/autotest/utilities/test_gdalalg_raster_edit.py b/autotest/utilities/test_gdalalg_raster_edit.py index d85556a8f36a..0f24976fd4f9 100755 --- a/autotest/utilities/test_gdalalg_raster_edit.py +++ b/autotest/utilities/test_gdalalg_raster_edit.py @@ -17,9 +17,7 @@ def get_edit_alg(): - reg = gdal.GetGlobalAlgorithmRegistry() - raster = reg.InstantiateAlg("raster") - return raster.InstantiateSubAlgorithm("edit") + return gdal.GetGlobalAlgorithmRegistry()["raster"]["edit"] def test_gdalalg_raster_edit_read_only(tmp_vsimem): @@ -28,7 +26,7 @@ def test_gdalalg_raster_edit_read_only(tmp_vsimem): gdal.FileFromMemBuffer(tmp_filename, open("../gcore/data/byte.tif", "rb").read()) pipeline = get_edit_alg() - pipeline.GetArg("dataset").Set(gdal.OpenEx(tmp_filename)) + pipeline["dataset"] = gdal.OpenEx(tmp_filename) with pytest.raises( Exception, match="edit: Dataset should be opened in update mode" ): diff --git a/autotest/utilities/test_gdalalg_raster_info.py b/autotest/utilities/test_gdalalg_raster_info.py index 18a8857c9cd7..f4a10b88333c 100755 --- a/autotest/utilities/test_gdalalg_raster_info.py +++ b/autotest/utilities/test_gdalalg_raster_info.py @@ -19,9 +19,7 @@ def get_info_alg(): - reg = gdal.GetGlobalAlgorithmRegistry() - raster = reg.InstantiateAlg("raster") - return raster.InstantiateSubAlgorithm("info") + return gdal.GetGlobalAlgorithmRegistry()["raster"]["info"] def test_gdalalg_raster_info_stdout(): @@ -41,7 +39,7 @@ def test_gdalalg_raster_info_stdout(): def test_gdalalg_raster_info(): info = get_info_alg() assert info.ParseRunAndFinalize(["--format=text", "data/utmsmall.tif"]) - output_string = info.GetArg("output-string").Get() + output_string = info["output-string"] assert output_string.startswith("Driver: GTiff/GeoTIFF") @@ -50,7 +48,7 @@ def test_gdalalg_raster_info_mm_checksum(): assert info.ParseRunAndFinalize( ["--format=text", "--mm", "--checksum", "data/utmsmall.tif"] ) - output_string = info.GetArg("output-string").Get() + output_string = info["output-string"] assert " Computed Min/Max=0.000,255.000" in output_string assert "Checksum=" in output_string @@ -58,9 +56,9 @@ def test_gdalalg_raster_info_mm_checksum(): def test_gdalalg_raster_info_stats(): info = get_info_alg() ds = gdal.Translate("", "../gcore/data/byte.tif", format="MEM") - info.GetArg("input").SetDataset(ds) + info["input"] = ds assert info.ParseRunAndFinalize(["--stats"]) - output_string = info.GetArg("output-string").Get() + output_string = info["output-string"] j = json.loads(output_string) assert "stdDev" in j["bands"][0] @@ -68,9 +66,9 @@ def test_gdalalg_raster_info_stats(): def test_gdalalg_raster_info_approx_stats(): info = get_info_alg() ds = gdal.Translate("", "../gcore/data/byte.tif", format="MEM") - info.GetArg("input").SetDataset(ds) + info["input"] = ds assert info.ParseRunAndFinalize(["--approx-stats"]) - output_string = info.GetArg("output-string").Get() + output_string = info["output-string"] j = json.loads(output_string) assert "stdDev" in j["bands"][0] @@ -78,9 +76,9 @@ def test_gdalalg_raster_info_approx_stats(): def test_gdalalg_raster_info_hist(): info = get_info_alg() ds = gdal.Translate("", "../gcore/data/byte.tif", format="MEM") - info.GetArg("input").SetDataset(ds) + info["input"] = ds assert info.ParseRunAndFinalize(["--hist"]) - output_string = info.GetArg("output-string").Get() + output_string = info["output-string"] j = json.loads(output_string) assert "histogram" in j["bands"][0] @@ -88,7 +86,7 @@ def test_gdalalg_raster_info_hist(): def test_gdalalg_raster_info_no_options(): info = get_info_alg() ds = gdal.Translate("", "../gcore/data/byte.tif", format="MEM") - info.GetArg("input").SetDataset(ds) + info["input"] = ds assert info.ParseRunAndFinalize( ["--no-gcp", "--no-md", "--no-ct", "--no-fl", "--no-nodata", "--no-mask"] ) @@ -98,9 +96,9 @@ def test_gdalalg_raster_info_list_mdd(): info = get_info_alg() ds = gdal.Translate("", "../gcore/data/byte.tif", format="MEM") ds.SetMetadataItem("foo", "bar", "MY_DOMAIN") - info.GetArg("input").SetDataset(ds) + info["input"] = ds assert info.ParseRunAndFinalize(["--list-mdd"]) - output_string = info.GetArg("output-string").Get() + output_string = info["output-string"] j = json.loads(output_string) assert "MY_DOMAIN" in j["metadata"]["metadataDomains"] @@ -109,9 +107,9 @@ def test_gdalalg_raster_info_mdd_all(): info = get_info_alg() ds = gdal.Translate("", "../gcore/data/byte.tif", format="MEM") ds.SetMetadataItem("foo", "bar", "MY_DOMAIN") - info.GetArg("input").SetDataset(ds) + info["input"] = ds assert info.ParseRunAndFinalize(["--mdd=all"]) - output_string = info.GetArg("output-string").Get() + output_string = info["output-string"] j = json.loads(output_string) assert j["metadata"] == { "": {"AREA_OR_POINT": "Area"}, @@ -128,7 +126,7 @@ def test_gdalalg_raster_info_list_subdataset(): assert info.ParseRunAndFinalize( ["--input=../gcore/data/tiff_with_subifds.tif", "--subdataset=2"] ) - output_string = info.GetArg("output-string").Get() + output_string = info["output-string"] j = json.loads(output_string) assert j["description"] == "GTIFF_DIR:2:../gcore/data/tiff_with_subifds.tif" @@ -149,7 +147,7 @@ def test_gdalalg_raster_info_list_subdataset_error_cannot_open_subdataset(): ds = gdal.GetDriverByName("MEM").Create("", 1, 1) ds.SetMetadataItem("SUBDATASET_1_DESC", "desc", "SUBDATASETS") ds.SetMetadataItem("SUBDATASET_1_NAME", "i_do_not_exist", "SUBDATASETS") - info.GetArg("input").SetDataset(ds) + info["input"] = ds with pytest.raises( Exception, match="i_do_not_exist", diff --git a/autotest/utilities/test_gdalalg_raster_mosaic.py b/autotest/utilities/test_gdalalg_raster_mosaic.py index 414d871a9f07..4100b53aef3c 100755 --- a/autotest/utilities/test_gdalalg_raster_mosaic.py +++ b/autotest/utilities/test_gdalalg_raster_mosaic.py @@ -17,27 +17,23 @@ def get_mosaic_alg(): - reg = gdal.GetGlobalAlgorithmRegistry() - raster = reg.InstantiateAlg("raster") - return raster.InstantiateSubAlgorithm("mosaic") + return gdal.GetGlobalAlgorithmRegistry()["raster"]["mosaic"] def test_gdalalg_raster_mosaic_from_dataset_handle(): alg = get_mosaic_alg() - alg.GetArg("input").Set( - [ - gdal.Translate( - "", "../gcore/data/byte.tif", options="-f MEM -srcwin 0 0 10 20" - ), - gdal.Translate( - "", "../gcore/data/byte.tif", options="-f MEM -srcwin 10 0 10 20" - ), - ] - ) - alg.GetArg("output").Set("") + alg["input"] = [ + gdal.Translate( + "", "../gcore/data/byte.tif", options="-f MEM -srcwin 0 0 10 20" + ), + gdal.Translate( + "", "../gcore/data/byte.tif", options="-f MEM -srcwin 10 0 10 20" + ), + ] + alg["output"] = "" assert alg.Run() - ds = alg.GetArg("output").Get().GetDataset() + ds = alg["output"].GetDataset() assert ds.RasterXSize == 20 assert ds.RasterYSize == 20 assert ds.GetGeoTransform() == pytest.approx( @@ -49,10 +45,10 @@ def test_gdalalg_raster_mosaic_from_dataset_handle(): def test_gdalalg_raster_mosaic_from_dataset_name(): alg = get_mosaic_alg() - alg.GetArg("input").Set(["../gcore/data/byte.tif"]) - alg.GetArg("output").Set("") + alg["input"] = ["../gcore/data/byte.tif"] + alg["output"] = "" assert alg.Run() - ds = alg.GetArg("output").Get().GetDataset() + ds = alg["output"].GetDataset() assert ds.RasterXSize == 20 assert ds.RasterYSize == 20 assert ds.GetGeoTransform() == pytest.approx( @@ -97,7 +93,7 @@ def test_gdalalg_raster_mosaic_bbox(): ] ) assert alg.Run() - ds = alg.GetArg("output").Get().GetDataset() + ds = alg["output"].GetDataset() assert ds.RasterXSize == 18 assert ds.RasterYSize == 18 assert ds.GetGeoTransform() == pytest.approx( @@ -114,11 +110,11 @@ def test_gdalalg_raster_mosaic_resolution_average(): src2_ds.SetGeoTransform([3, 0.5, 0, 49, 0, -0.5]) alg = get_mosaic_alg() - alg.GetArg("input").Set([src1_ds, src2_ds]) - alg.GetArg("resolution").Set("average") - alg.GetArg("output").Set("") + alg["input"] = [src1_ds, src2_ds] + alg["resolution"] = "average" + alg["output"] = "" assert alg.Run() - ds = alg.GetArg("output").Get().GetDataset() + ds = alg["output"].GetDataset() assert ds.RasterXSize == 3 assert ds.RasterYSize == 1 assert ds.GetGeoTransform() == pytest.approx((2.0, 0.75, 0.0, 49.0, 0.0, -0.75)) @@ -132,11 +128,11 @@ def test_gdalalg_raster_mosaic_resolution_highest(): src2_ds.SetGeoTransform([3, 0.5, 0, 49, 0, -0.5]) alg = get_mosaic_alg() - alg.GetArg("input").Set([src1_ds, src2_ds]) - alg.GetArg("output").Set("") - alg.GetArg("resolution").Set("highest") + alg["input"] = [src1_ds, src2_ds] + alg["output"] = "" + alg["resolution"] = "highest" assert alg.Run() - ds = alg.GetArg("output").Get().GetDataset() + ds = alg["output"].GetDataset() assert ds.RasterXSize == 4 assert ds.RasterYSize == 2 assert ds.GetGeoTransform() == pytest.approx((2.0, 0.5, 0.0, 49.0, 0.0, -0.5)) @@ -150,11 +146,11 @@ def test_gdalalg_raster_mosaic_resolution_lowest(): src2_ds.SetGeoTransform([3, 0.5, 0, 49, 0, -0.5]) alg = get_mosaic_alg() - alg.GetArg("input").Set([src1_ds, src2_ds]) - alg.GetArg("output").Set("") - alg.GetArg("resolution").Set("lowest") + alg["input"] = [src1_ds, src2_ds] + alg["output"] = "" + alg["resolution"] = "lowest" assert alg.Run() - ds = alg.GetArg("output").Get().GetDataset() + ds = alg["output"].GetDataset() assert ds.RasterXSize == 2 assert ds.RasterYSize == 1 assert ds.GetGeoTransform() == pytest.approx((2.0, 1.0, 0.0, 49.0, 0.0, -1.0)) @@ -168,11 +164,11 @@ def test_gdalalg_raster_mosaic_resolution_custom(): src2_ds.SetGeoTransform([3, 0.5, 0, 49, 0, -0.5]) alg = get_mosaic_alg() - alg.GetArg("input").Set([src1_ds, src2_ds]) - alg.GetArg("output").Set("") - alg.GetArg("resolution").Set("0.5,1") + alg["input"] = [src1_ds, src2_ds] + alg["output"] = "" + alg["resolution"] = "0.5,1" assert alg.Run() - ds = alg.GetArg("output").Get().GetDataset() + ds = alg["output"].GetDataset() assert ds.RasterXSize == 4 assert ds.RasterYSize == 1 assert ds.GetGeoTransform() == pytest.approx((2.0, 0.5, 0.0, 49.0, 0.0, -1.0)) @@ -186,12 +182,12 @@ def test_gdalalg_raster_mosaic_target_aligned_pixels(): src2_ds.SetGeoTransform([3, 0.5, 0, 49, 0, -0.5]) alg = get_mosaic_alg() - alg.GetArg("input").Set([src1_ds, src2_ds]) - alg.GetArg("output").Set("") - alg.GetArg("resolution").Set("0.3,0.6") - alg.GetArg("target-aligned-pixels").Set(True) + alg["input"] = [src1_ds, src2_ds] + alg["output"] = "" + alg["resolution"] = "0.3,0.6" + alg["target-aligned-pixels"] = True assert alg.Run() - ds = alg.GetArg("output").Get().GetDataset() + ds = alg["output"].GetDataset() assert ds.RasterXSize == 8 assert ds.RasterYSize == 2 assert ds.GetGeoTransform() == pytest.approx((1.8, 0.3, 0.0, 49.2, 0.0, -0.6)) @@ -205,8 +201,8 @@ def test_gdalalg_raster_mosaic_resolution_same_default(): src2_ds.SetGeoTransform([3, 0.5, 0, 49, 0, -0.5]) alg = get_mosaic_alg() - alg.GetArg("input").Set([src1_ds, src2_ds]) - alg.GetArg("output").Set("") + alg["input"] = [src1_ds, src2_ds] + alg["output"] = "" with pytest.raises( Exception, match="whereas previous sources have resolution", @@ -221,19 +217,19 @@ def test_gdalalg_raster_mosaic_resolution_invalid(): Exception, match="resolution: two comma separated positive values should be provided, or 'same', 'average', 'highest' or 'lowest'", ): - alg.GetArg("resolution").Set("invalid") + alg["resolution"] = "invalid" with pytest.raises( Exception, match="resolution: two comma separated positive values should be provided, or 'same', 'average', 'highest' or 'lowest'", ): - alg.GetArg("resolution").Set("0.5") + alg["resolution"] = "0.5" with pytest.raises( Exception, match="resolution: two comma separated positive values should be provided, or 'same', 'average', 'highest' or 'lowest'", ): - alg.GetArg("resolution").Set("-0.5,-0.5") + alg["resolution"] = "-0.5,-0.5" def test_gdalalg_raster_mosaic_srcnodata_dstnodata(): @@ -243,12 +239,12 @@ def test_gdalalg_raster_mosaic_srcnodata_dstnodata(): src1_ds.GetRasterBand(1).Fill(1) alg = get_mosaic_alg() - alg.GetArg("input").Set([src1_ds]) - alg.GetArg("output").Set("") - alg.GetArg("srcnodata").Set([1]) - alg.GetArg("dstnodata").Set([2]) + alg["input"] = [src1_ds] + alg["output"] = "" + alg["srcnodata"] = [1] + alg["dstnodata"] = [2] assert alg.Run() - ds = alg.GetArg("output").Get().GetDataset() + ds = alg["output"].GetDataset() assert ds.GetRasterBand(1).Checksum() == 2 assert ds.GetRasterBand(1).GetNoDataValue() == 2 @@ -260,13 +256,13 @@ def test_gdalalg_raster_mosaic_hidenodata(): src1_ds.GetRasterBand(1).Fill(1) alg = get_mosaic_alg() - alg.GetArg("input").Set([src1_ds]) - alg.GetArg("output").Set("") - alg.GetArg("srcnodata").Set([1]) - alg.GetArg("dstnodata").Set([2]) - alg.GetArg("hidenodata").Set(True) + alg["input"] = [src1_ds] + alg["output"] = "" + alg["srcnodata"] = [1] + alg["dstnodata"] = [2] + alg["hidenodata"] = True assert alg.Run() - ds = alg.GetArg("output").Get().GetDataset() + ds = alg["output"].GetDataset() assert ds.GetRasterBand(1).Checksum() == 2 assert ds.GetRasterBand(1).GetNoDataValue() is None @@ -274,25 +270,11 @@ def test_gdalalg_raster_mosaic_hidenodata(): def test_gdalalg_raster_mosaic_addalpha(): alg = get_mosaic_alg() - alg.GetArg("input").Set(["../gcore/data/byte.tif"]) - alg.GetArg("output").Set("") - alg.GetArg("addalpha").Set(True) - assert alg.Run() - ds = alg.GetArg("output").Get().GetDataset() - assert ds.RasterCount == 2 - assert ds.GetRasterBand(1).Checksum() == 4672 - assert ds.GetRasterBand(2).GetColorInterpretation() == gdal.GCI_AlphaBand - assert ds.GetRasterBand(2).Checksum() == 4873 - - -def test_gdalalg_raster_mosaic_band(): - - alg = get_mosaic_alg() - alg.GetArg("input").Set(["../gcore/data/rgbsmall.tif"]) - alg.GetArg("output").Set("") - alg.GetArg("band").Set([3, 2]) + alg["input"] = ["../gcore/data/rgbsmall.tif"] + alg["output"] = "" + alg["band"] = [3, 2] assert alg.Run() - ds = alg.GetArg("output").Get().GetDataset() + ds = alg["output"].GetDataset() assert ds.RasterCount == 2 assert ds.GetRasterBand(1).Checksum() == 21349 assert ds.GetRasterBand(2).Checksum() == 21053 @@ -301,10 +283,10 @@ def test_gdalalg_raster_mosaic_band(): def test_gdalalg_raster_mosaic_glob(): alg = get_mosaic_alg() - alg.GetArg("input").Set(["../gcore/data/rgbsm?ll.tif"]) - alg.GetArg("output").Set("") + alg["input"] = ["../gcore/data/rgbsm?ll.tif"] + alg["output"] = "" assert alg.Run() - ds = alg.GetArg("output").Get().GetDataset() + ds = alg["output"].GetDataset() assert ds.RasterCount == 3 @@ -314,18 +296,18 @@ def test_gdalalg_raster_mosaic_at_filename(tmp_vsimem): gdal.FileFromMemBuffer(input_file_list, "../gcore/data/byte.tif") alg = get_mosaic_alg() - alg.GetArg("input").Set([f"@{input_file_list}"]) - alg.GetArg("output").Set("") + alg["input"] = [f"@{input_file_list}"] + alg["output"] = "" assert alg.Run() - ds = alg.GetArg("output").Get().GetDataset() + ds = alg["output"].GetDataset() assert ds.GetRasterBand(1).Checksum() == 4672 def test_gdalalg_raster_mosaic_at_filename_error(): alg = get_mosaic_alg() - alg.GetArg("input").Set(["@i_do_not_exist"]) - alg.GetArg("output").Set("") + alg["input"] = ["@i_do_not_exist"] + alg["output"] = "" with pytest.raises(Exception, match="mosaic: Cannot open i_do_not_exist"): alg.Run() @@ -335,8 +317,8 @@ def test_gdalalg_raster_mosaic_output_ds_alread_set(): out_ds = gdal.GetDriverByName("MEM").Create("", 1, 1) alg = get_mosaic_alg() - alg.GetArg("input").Set(["../gcore/data/byte.tif"]) - alg.GetArg("output").Set(out_ds) + alg["input"] = ["../gcore/data/byte.tif"] + alg["output"] = out_ds with pytest.raises( Exception, match="mosaic: gdal raster mosaic does not support outputting to an already opened output dataset", @@ -347,11 +329,11 @@ def test_gdalalg_raster_mosaic_output_ds_alread_set(): def test_gdalalg_raster_mosaic_co(): alg = get_mosaic_alg() - alg.GetArg("input").Set(["../gcore/data/byte.tif"]) - alg.GetArg("output").Set("") - alg.GetArg("creation-option").Set(["BLOCKXSIZE=10", "BLOCKYSIZE=15"]) + alg["input"] = ["../gcore/data/byte.tif"] + alg["output"] = "" + alg["creation-option"] = ["BLOCKXSIZE=10", "BLOCKYSIZE=15"] assert alg.Run() - ds = alg.GetArg("output").Get().GetDataset() + ds = alg["output"].GetDataset() assert ds.GetRasterBand(1).GetBlockSize() == [10, 15] @@ -405,8 +387,8 @@ def test_gdalalg_raster_mosaic_inconsistent_characteristics(): src2_ds.SetSpatialRef(srs) alg = get_mosaic_alg() - alg.GetArg("input").Set([src1_ds, src2_ds]) - alg.GetArg("output").Set("") + alg["input"] = [src1_ds, src2_ds] + alg["output"] = "" with pytest.raises( Exception, match="gdal raster mosaic does not support heterogeneous projection" ): diff --git a/autotest/utilities/test_gdalalg_raster_overview.py b/autotest/utilities/test_gdalalg_raster_overview.py index e193469d5154..d911b480e661 100755 --- a/autotest/utilities/test_gdalalg_raster_overview.py +++ b/autotest/utilities/test_gdalalg_raster_overview.py @@ -17,28 +17,26 @@ def get_overview_alg(): - reg = gdal.GetGlobalAlgorithmRegistry() - raster = reg.InstantiateAlg("raster") - return raster.InstantiateSubAlgorithm("overview") + return gdal.GetGlobalAlgorithmRegistry()["raster"]["overview"] def get_overview_add_alg(): - return get_overview_alg().InstantiateSubAlgorithm("add") + return get_overview_alg()["add"] def get_overview_delete_alg(): - return get_overview_alg().InstantiateSubAlgorithm("delete") + return get_overview_alg()["delete"] def test_gdalalg_overview_invalid_arguments(): add = get_overview_add_alg() with pytest.raises(Exception): - add.GetArg("levels").Set([1]) + add["levels"] = [1] add = get_overview_add_alg() with pytest.raises(Exception): - add.GetArg("min-size").Set(0) + add["min-size"] = 0 def test_gdalalg_overview_explicit_level(): @@ -46,8 +44,8 @@ def test_gdalalg_overview_explicit_level(): ds = gdal.Translate("", "../gcore/data/byte.tif", format="MEM") add = get_overview_add_alg() - add.GetArg("dataset").Get().SetDataset(ds) - add.GetArg("levels").Set([2]) + add["dataset"] = ds + add["levels"] = [2] assert add.Run() assert ds.GetRasterBand(1).Checksum() == 4672 @@ -60,9 +58,9 @@ def test_gdalalg_overview_minsize_and_resampling(): ds = gdal.Translate("", "../gcore/data/byte.tif", format="MEM") add = get_overview_add_alg() - add.GetArg("dataset").Get().SetDataset(ds) - add.GetArg("resampling").Set("average") - add.GetArg("min-size").Set(10) + add["dataset"] = ds + add["resampling"] = "average" + add["min-size"] = 10 assert add.Run() assert ds.GetRasterBand(1).Checksum() == 4672 @@ -76,9 +74,9 @@ def test_gdalalg_overview_reuse_resampling_and_levels(tmp_vsimem): ds = gdal.Translate(tmp_filename, "../gcore/data/byte.tif") add = get_overview_add_alg() - add.GetArg("dataset").Get().SetDataset(ds) - add.GetArg("resampling").Set("average") - add.GetArg("min-size").Set(10) + add["dataset"] = ds + add["resampling"] = "average" + add["min-size"] = 10 assert add.Run() assert ds.GetRasterBand(1).Checksum() == 4672 @@ -89,7 +87,7 @@ def test_gdalalg_overview_reuse_resampling_and_levels(tmp_vsimem): ds.GetRasterBand(1).GetOverview(0).Fill(0) add = get_overview_add_alg() - add.GetArg("dataset").Get().SetDataset(ds) + add["dataset"] = ds assert add.Run() assert ds.GetRasterBand(1).Checksum() == 4672 @@ -136,15 +134,15 @@ def test_gdalalg_overview_delete(): ds = gdal.Translate("", "../gcore/data/byte.tif", format="MEM") add = get_overview_add_alg() - add.GetArg("dataset").Get().SetDataset(ds) - add.GetArg("resampling").Set("average") - add.GetArg("min-size").Set(10) + add["dataset"] = ds + add["resampling"] = "average" + add["min-size"] = 10 assert add.Run() assert ds.GetRasterBand(1).GetOverviewCount() == 1 delete = get_overview_delete_alg() - delete.GetArg("dataset").Get().SetDataset(ds) + delete["dataset"] = ds assert delete.Run() assert ds.GetRasterBand(1).GetOverviewCount() == 0 diff --git a/autotest/utilities/test_gdalalg_raster_pipeline.py b/autotest/utilities/test_gdalalg_raster_pipeline.py index 55ede9b0fa23..570ac85b8fd5 100755 --- a/autotest/utilities/test_gdalalg_raster_pipeline.py +++ b/autotest/utilities/test_gdalalg_raster_pipeline.py @@ -19,9 +19,7 @@ def get_pipeline_alg(): - reg = gdal.GetGlobalAlgorithmRegistry() - raster = reg.InstantiateAlg("raster") - return raster.InstantiateSubAlgorithm("pipeline") + return gdal.GetGlobalAlgorithmRegistry()["raster"]["pipeline"] def test_gdalalg_raster_pipeline_read_and_write(tmp_vsimem): @@ -68,11 +66,9 @@ def test_gdalalg_raster_pipeline_as_api(tmp_vsimem): out_filename = str(tmp_vsimem / "out.tif") pipeline = get_pipeline_alg() - pipeline.GetArg("pipeline").Set( - f"read ../gcore/data/byte.tif ! write {out_filename}" - ) + pipeline["pipeline"] = f"read ../gcore/data/byte.tif ! write {out_filename}" assert pipeline.Run() - ds = pipeline.GetArg("output").Get().GetDataset() + ds = pipeline["output"].GetDataset() assert ds.GetRasterBand(1).Checksum() == 4672 assert pipeline.Finalize() ds = None @@ -86,8 +82,8 @@ def test_gdalalg_raster_pipeline_input_through_api(tmp_vsimem): out_filename = str(tmp_vsimem / "out.tif") pipeline = get_pipeline_alg() - pipeline.GetArg("input").Get().SetDataset(gdal.OpenEx("../gcore/data/byte.tif")) - pipeline.GetArg("pipeline").Set(f"read ! write {out_filename}") + pipeline["input"] = "../gcore/data/byte.tif" + pipeline["pipeline"] = f"read ! write {out_filename}" assert pipeline.Run() assert pipeline.Finalize() @@ -100,8 +96,8 @@ def test_gdalalg_raster_pipeline_input_through_api_run_twice(tmp_vsimem): out_filename = str(tmp_vsimem / "out.tif") pipeline = get_pipeline_alg() - pipeline.GetArg("input").Get().SetDataset(gdal.OpenEx("../gcore/data/byte.tif")) - pipeline.GetArg("pipeline").Set(f"read ! write {out_filename}") + pipeline["input"] = "../gcore/data/byte.tif" + pipeline["pipeline"] = f"read ! write {out_filename}" assert pipeline.Run() with pytest.raises( Exception, match=r"pipeline: Step nr 0 \(read\) has already an output dataset" @@ -114,8 +110,8 @@ def test_gdalalg_raster_pipeline_output_through_api(tmp_vsimem): out_filename = str(tmp_vsimem / "out.tif") pipeline = get_pipeline_alg() - pipeline.GetArg("output").Get().SetName(out_filename) - pipeline.GetArg("pipeline").Set("read ../gcore/data/byte.tif ! write") + pipeline["output"] = out_filename + pipeline["pipeline"] = "read ../gcore/data/byte.tif ! write" assert pipeline.Run() assert pipeline.Finalize() @@ -126,7 +122,7 @@ def test_gdalalg_raster_pipeline_output_through_api(tmp_vsimem): def test_gdalalg_raster_pipeline_as_api_error(): pipeline = get_pipeline_alg() - pipeline.GetArg("pipeline").Set("read") + pipeline["pipeline"] = "read" with pytest.raises(Exception, match="pipeline: At least 2 steps must be provided"): pipeline.Run() diff --git a/autotest/utilities/test_gdalalg_vector_clip.py b/autotest/utilities/test_gdalalg_vector_clip.py index 9c5afbe0379a..0ec6a9fea798 100755 --- a/autotest/utilities/test_gdalalg_vector_clip.py +++ b/autotest/utilities/test_gdalalg_vector_clip.py @@ -20,9 +20,7 @@ def get_clip_alg(): - reg = gdal.GetGlobalAlgorithmRegistry() - vector = reg.InstantiateAlg("vector") - return vector.InstantiateSubAlgorithm("clip") + return gdal.GetGlobalAlgorithmRegistry()["vector"]["clip"] @pytest.mark.require_driver("GPKG") @@ -136,14 +134,14 @@ def test_gdalalg_vector_clip_bbox(): src_lyr.CreateFeature(f) clip = get_clip_alg() - clip.GetArg("input").Get().SetDataset(src_ds) + clip["input"] = src_ds assert clip.ParseCommandLineArguments( ["--bbox", "0.2,0.3,0.7,0.8", "--of", "Memory", "--output", "memory_ds"] ) assert clip.Run() - out_ds = clip.GetArg("output").Get().GetDataset() + out_ds = clip["output"].GetDataset() out_lyr = out_ds.GetLayer(0) out_f = out_lyr.GetNextFeature() assert out_f["foo"] == "bar" @@ -169,7 +167,7 @@ def test_gdalalg_vector_clip_bbox_srs(): src_lyr.CreateFeature(f) clip = get_clip_alg() - clip.GetArg("input").Get().SetDataset(src_ds) + clip["input"] = src_ds assert clip.ParseCommandLineArguments( [ @@ -185,7 +183,7 @@ def test_gdalalg_vector_clip_bbox_srs(): ) assert clip.Run() - out_ds = clip.GetArg("output").Get().GetDataset() + out_ds = clip["output"].GetDataset() out_lyr = out_ds.GetLayer(0) assert out_lyr.GetSpatialRef().GetAuthorityCode(None) == "4326" out_f = out_lyr.GetNextFeature() @@ -220,14 +218,14 @@ def test_gdalalg_vector_clip_split_multipart(): src_lyr.CreateFeature(f) clip = get_clip_alg() - clip.GetArg("input").Get().SetDataset(src_ds) + clip["input"] = src_ds assert clip.ParseCommandLineArguments( ["--bbox", "-0.1,0.3,1.1,0.7", "--of", "Memory", "--output", "memory_ds"] ) assert clip.Run() - out_ds = clip.GetArg("output").Get().GetDataset() + out_ds = clip["output"].GetDataset() out_lyr = out_ds.GetLayer(0) ref1_geom = ogr.CreateGeometryFromWkt( @@ -284,14 +282,14 @@ def test_gdalalg_vector_clip_geom(clip_geom): src_lyr.CreateFeature(f) clip = get_clip_alg() - clip.GetArg("input").Get().SetDataset(src_ds) + clip["input"] = src_ds assert clip.ParseCommandLineArguments( ["--geometry", clip_geom, "--of", "Memory", "--output", "memory_ds"] ) assert clip.Run() - out_ds = clip.GetArg("output").Get().GetDataset() + out_ds = clip["output"].GetDataset() out_lyr = out_ds.GetLayer(0) out_f = out_lyr.GetNextFeature() assert out_f["foo"] == "bar" @@ -324,7 +322,7 @@ def test_gdalalg_vector_clip_geom_srs(clip_geom): src_lyr.CreateFeature(f) clip = get_clip_alg() - clip.GetArg("input").Get().SetDataset(src_ds) + clip["input"] = src_ds clip_geom = clip_geom.replace("0.2", "22263.8981586547") clip_geom = clip_geom.replace("0.3", "33395.9998333802") clip_geom = clip_geom.replace("0.7", "77923.6435552915") @@ -343,7 +341,7 @@ def test_gdalalg_vector_clip_geom_srs(clip_geom): ) assert clip.Run() - out_ds = clip.GetArg("output").Get().GetDataset() + out_ds = clip["output"].GetDataset() out_lyr = out_ds.GetLayer(0) out_f = out_lyr.GetNextFeature() assert out_f["foo"] == "bar" @@ -376,7 +374,7 @@ def test_gdalalg_vector_clip_geom_not_rectangle(): src_lyr.CreateFeature(f) clip = get_clip_alg() - clip.GetArg("input").Get().SetDataset(src_ds) + clip["input"] = src_ds assert clip.ParseCommandLineArguments( [ @@ -390,7 +388,7 @@ def test_gdalalg_vector_clip_geom_not_rectangle(): ) assert clip.Run() - out_ds = clip.GetArg("output").Get().GetDataset() + out_ds = clip["output"].GetDataset() out_lyr = out_ds.GetLayer(0) out_f = out_lyr.GetNextFeature() assert out_f["foo"] == "bar" @@ -411,7 +409,7 @@ def test_gdalalg_vector_clip_intersection_incompatible_geometry_type(): src_lyr.CreateFeature(f) clip = get_clip_alg() - clip.GetArg("input").Get().SetDataset(src_ds) + clip["input"] = src_ds assert clip.ParseCommandLineArguments( [ @@ -425,7 +423,7 @@ def test_gdalalg_vector_clip_intersection_incompatible_geometry_type(): ) assert clip.Run() - out_ds = clip.GetArg("output").Get().GetDataset() + out_ds = clip["output"].GetDataset() out_lyr = out_ds.GetLayer(0) assert out_lyr.GetNextFeature() is None @@ -444,7 +442,7 @@ def test_gdalalg_vector_clip_intersection_promote_simple_type_to_multi(): src_lyr.CreateFeature(f) clip = get_clip_alg() - clip.GetArg("input").Get().SetDataset(src_ds) + clip["input"] = src_ds assert clip.ParseCommandLineArguments( [ @@ -458,7 +456,7 @@ def test_gdalalg_vector_clip_intersection_promote_simple_type_to_multi(): ) assert clip.Run() - out_ds = clip.GetArg("output").Get().GetDataset() + out_ds = clip["output"].GetDataset() out_lyr = out_ds.GetLayer(0) out_f = out_lyr.GetNextFeature() ogrtest.check_feature_geometry( @@ -487,13 +485,13 @@ def test_gdalalg_vector_clip_like_vector(): like_lyr.CreateFeature(f) clip = get_clip_alg() - clip.GetArg("input").Get().SetDataset(src_ds) - clip.GetArg("like").Get().SetDataset(like_ds) + clip["input"] = src_ds + clip["like"] = like_ds assert clip.ParseCommandLineArguments(["--of", "Memory", "--output", "memory_ds"]) assert clip.Run() - out_ds = clip.GetArg("output").Get().GetDataset() + out_ds = clip["output"].GetDataset() out_lyr = out_ds.GetLayer(0) out_f = out_lyr.GetNextFeature() assert out_f["foo"] == "bar" @@ -525,8 +523,8 @@ def test_gdalalg_vector_clip_like_vector_invalid_geom(): like_lyr.CreateFeature(f) clip = get_clip_alg() - clip.GetArg("input").Get().SetDataset(src_ds) - clip.GetArg("like").Get().SetDataset(like_ds) + clip["input"] = src_ds + clip["like"] = like_ds assert clip.ParseCommandLineArguments(["--of", "Memory", "--output", "memory_ds"]) with gdal.quiet_errors(), pytest.raises( @@ -556,15 +554,15 @@ def test_gdalalg_vector_clip_like_vector_like_layer(): like_lyr.CreateFeature(f) clip = get_clip_alg() - clip.GetArg("input").Get().SetDataset(src_ds) - clip.GetArg("like").Get().SetDataset(like_ds) + clip["input"] = src_ds + clip["like"] = like_ds assert clip.ParseCommandLineArguments( ["--like-layer", "my_layer", "--of", "Memory", "--output", "memory_ds"] ) assert clip.Run() - out_ds = clip.GetArg("output").Get().GetDataset() + out_ds = clip["output"].GetDataset() out_lyr = out_ds.GetLayer(0) out_f = out_lyr.GetNextFeature() assert out_f["foo"] == "bar" @@ -585,8 +583,8 @@ def test_gdalalg_vector_clip_like_vector_like_layer_invalid(): like_ds.CreateLayer("test") clip = get_clip_alg() - clip.GetArg("input").Get().SetDataset(src_ds) - clip.GetArg("like").Get().SetDataset(like_ds) + clip["input"] = src_ds + clip["like"] = like_ds assert clip.ParseCommandLineArguments( ["--like-layer", "invalid", "--of", "Memory", "--output", "memory_ds"] @@ -618,8 +616,8 @@ def test_gdalalg_vector_clip_like_vector_like_sql(): like_lyr.CreateFeature(f) clip = get_clip_alg() - clip.GetArg("input").Get().SetDataset(src_ds) - clip.GetArg("like").Get().SetDataset(like_ds) + clip["input"] = src_ds + clip["like"] = like_ds assert clip.ParseCommandLineArguments( [ @@ -633,7 +631,7 @@ def test_gdalalg_vector_clip_like_vector_like_sql(): ) assert clip.Run() - out_ds = clip.GetArg("output").Get().GetDataset() + out_ds = clip["output"].GetDataset() out_lyr = out_ds.GetLayer(0) out_f = out_lyr.GetNextFeature() assert out_f["foo"] == "bar" @@ -672,15 +670,15 @@ def test_gdalalg_vector_clip_like_vector_like_where(): like_lyr.CreateFeature(f) clip = get_clip_alg() - clip.GetArg("input").Get().SetDataset(src_ds) - clip.GetArg("like").Get().SetDataset(like_ds) + clip["input"] = src_ds + clip["like"] = like_ds assert clip.ParseCommandLineArguments( ["--like-where", "id=1", "--of", "Memory", "--output", "memory_ds"] ) assert clip.Run() - out_ds = clip.GetArg("output").Get().GetDataset() + out_ds = clip["output"].GetDataset() out_lyr = out_ds.GetLayer(0) out_f = out_lyr.GetNextFeature() assert out_f["foo"] == "bar" @@ -708,8 +706,8 @@ def test_gdalalg_vector_clip_like_vector_like_where_empty(): like_lyr.CreateField(ogr.FieldDefn("id", ogr.OFTInteger)) clip = get_clip_alg() - clip.GetArg("input").Get().SetDataset(src_ds) - clip.GetArg("like").Get().SetDataset(like_ds) + clip["input"] = src_ds + clip["like"] = like_ds assert clip.ParseCommandLineArguments( ["--like-where", "id=1", "--of", "Memory", "--output", "memory_ds"] @@ -745,13 +743,13 @@ def test_gdalalg_vector_clip_like_vector_srs(): like_lyr.CreateFeature(f) clip = get_clip_alg() - clip.GetArg("input").Get().SetDataset(src_ds) - clip.GetArg("like").Get().SetDataset(like_ds) + clip["input"] = src_ds + clip["like"] = like_ds assert clip.ParseCommandLineArguments(["--of", "Memory", "--output", "memory_ds"]) assert clip.Run() - out_ds = clip.GetArg("output").Get().GetDataset() + out_ds = clip["output"].GetDataset() out_lyr = out_ds.GetLayer(0) out_f = out_lyr.GetNextFeature() ogrtest.check_feature_geometry( @@ -777,13 +775,13 @@ def test_gdalalg_vector_clip_like_raster(): like_ds.SetGeoTransform([0.2, 0.5, 0, 0.3, 0, 0.5]) clip = get_clip_alg() - clip.GetArg("input").Get().SetDataset(src_ds) - clip.GetArg("like").Get().SetDataset(like_ds) + clip["input"] = src_ds + clip["like"] = like_ds assert clip.ParseCommandLineArguments(["--of", "Memory", "--output", "memory_ds"]) assert clip.Run() - out_ds = clip.GetArg("output").Get().GetDataset() + out_ds = clip["output"].GetDataset() out_lyr = out_ds.GetLayer(0) out_f = out_lyr.GetNextFeature() assert out_f["foo"] == "bar" @@ -818,13 +816,13 @@ def test_gdalalg_vector_clip_like_raster_srs(): like_ds.SetSpatialRef(srs_3857) clip = get_clip_alg() - clip.GetArg("input").Get().SetDataset(src_ds) - clip.GetArg("like").Get().SetDataset(like_ds) + clip["input"] = src_ds + clip["like"] = like_ds assert clip.ParseCommandLineArguments(["--of", "Memory", "--output", "memory_ds"]) assert clip.Run() - out_ds = clip.GetArg("output").Get().GetDataset() + out_ds = clip["output"].GetDataset() out_lyr = out_ds.GetLayer(0) out_f = out_lyr.GetNextFeature() assert out_f["foo"] == "bar" @@ -852,7 +850,7 @@ def test_gdalalg_vector_clip_geometry_invalid(): src_ds.CreateLayer("test") clip = get_clip_alg() - clip.GetArg("input").Get().SetDataset(src_ds) + clip["input"] = src_ds assert clip.ParseCommandLineArguments( ["--geometry", "invalid", "--of", "Memory", "--output", "memory_ds"] @@ -873,8 +871,8 @@ def test_gdalalg_vector_clip_like_vector_too_many_layers(): like_ds = gdal.GetDriverByName("MEM").Create("", 1, 1, 1) clip = get_clip_alg() - clip.GetArg("input").Get().SetDataset(src_ds) - clip.GetArg("like").Get().SetDataset(like_ds) + clip["input"] = src_ds + clip["like"] = like_ds assert clip.ParseCommandLineArguments(["--of", "Memory", "--output", "memory_ds"]) with pytest.raises( @@ -895,8 +893,8 @@ def test_gdalalg_vector_clip_like_raster_no_geotransform(): like_ds.CreateLayer("test2") clip = get_clip_alg() - clip.GetArg("input").Get().SetDataset(src_ds) - clip.GetArg("like").Get().SetDataset(like_ds) + clip["input"] = src_ds + clip["like"] = like_ds assert clip.ParseCommandLineArguments(["--of", "Memory", "--output", "memory_ds"]) with pytest.raises( @@ -913,8 +911,8 @@ def test_gdalalg_vector_clip_like_neither_raster_no_vector(): like_ds = gdal.GetDriverByName("Memory").Create("clip", 0, 0, 0, gdal.GDT_Unknown) clip = get_clip_alg() - clip.GetArg("input").Get().SetDataset(src_ds) - clip.GetArg("like").Get().SetDataset(like_ds) + clip["input"] = src_ds + clip["like"] = like_ds assert clip.ParseCommandLineArguments(["--of", "Memory", "--output", "memory_ds"]) with pytest.raises( diff --git a/autotest/utilities/test_gdalalg_vector_convert.py b/autotest/utilities/test_gdalalg_vector_convert.py index b38ffc7a1241..b42be87d39b7 100755 --- a/autotest/utilities/test_gdalalg_vector_convert.py +++ b/autotest/utilities/test_gdalalg_vector_convert.py @@ -17,9 +17,7 @@ def get_convert_alg(): - reg = gdal.GetGlobalAlgorithmRegistry() - vector = reg.InstantiateAlg("vector") - return vector.InstantiateSubAlgorithm("convert") + return gdal.GetGlobalAlgorithmRegistry()["vector"]["convert"] @pytest.mark.require_driver("GPKG") @@ -156,8 +154,11 @@ def test_gdalalg_vector_wrong_layer_name(tmp_vsimem): def test_gdalalg_vector_convert_error_output_not_set(): convert = get_convert_alg() - convert.GetArg("input").Get().SetName("../ogr/data/poly.shp") - convert.GetArg("output").Set(convert.GetArg("output").Get()) + convert["input"] = "../ogr/data/poly.shp" + + # Make it such that the "output" argment is set, but to a unset GDALArgDatasetValue + convert["output"] = convert["output"] + with pytest.raises( Exception, match="convert: Argument 'output' has no dataset object or dataset name", diff --git a/autotest/utilities/test_gdalalg_vector_filter.py b/autotest/utilities/test_gdalalg_vector_filter.py index f0a62a97c65c..33b11a4239a3 100755 --- a/autotest/utilities/test_gdalalg_vector_filter.py +++ b/autotest/utilities/test_gdalalg_vector_filter.py @@ -15,9 +15,7 @@ def get_filter_alg(): - reg = gdal.GetGlobalAlgorithmRegistry() - vector = reg.InstantiateAlg("vector") - return vector.InstantiateSubAlgorithm("filter") + return gdal.GetGlobalAlgorithmRegistry()["vector"]["filter"] def test_gdalalg_vector_filter_no_filter(tmp_vsimem): @@ -27,7 +25,7 @@ def test_gdalalg_vector_filter_no_filter(tmp_vsimem): filter_alg = get_filter_alg() assert filter_alg.ParseCommandLineArguments(["../ogr/data/poly.shp", out_filename]) assert filter_alg.Run() - ds = filter_alg.GetArg("output").Get().GetDataset() + ds = filter_alg["output"].GetDataset() assert ds.GetLayer(0).GetFeatureCount() == 10 assert filter_alg.Finalize() ds = None diff --git a/autotest/utilities/test_gdalalg_vector_info.py b/autotest/utilities/test_gdalalg_vector_info.py index 9fbaace7eee7..524dcf104a14 100755 --- a/autotest/utilities/test_gdalalg_vector_info.py +++ b/autotest/utilities/test_gdalalg_vector_info.py @@ -19,9 +19,7 @@ def get_info_alg(): - reg = gdal.GetGlobalAlgorithmRegistry() - vector = reg.InstantiateAlg("vector") - return vector.InstantiateSubAlgorithm("info") + return gdal.GetGlobalAlgorithmRegistry()["vector"]["info"] def test_gdalalg_vector_info_stdout(): @@ -42,14 +40,14 @@ def test_gdalalg_vector_info_stdout(): def test_gdalalg_vector_info_text(): info = get_info_alg() assert info.ParseRunAndFinalize(["--format=text", "data/path.shp"]) - output_string = info.GetArg("output-string").Get() + output_string = info["output-string"] assert output_string.startswith("INFO: Open of") def test_gdalalg_vector_info_json(): info = get_info_alg() assert info.ParseRunAndFinalize(["data/path.shp"]) - output_string = info.GetArg("output-string").Get() + output_string = info["output-string"] j = json.loads(output_string) assert j["layers"][0]["name"] == "path" assert "features" not in j["layers"][0] @@ -58,7 +56,7 @@ def test_gdalalg_vector_info_json(): def test_gdalalg_vector_info_features(): info = get_info_alg() assert info.ParseRunAndFinalize(["--features", "data/path.shp"]) - output_string = info.GetArg("output-string").Get() + output_string = info["output-string"] j = json.loads(output_string) assert "features" in j["layers"][0] @@ -68,7 +66,7 @@ def test_gdalalg_vector_info_sql(): assert info.ParseRunAndFinalize( ["--sql", "SELECT 1 AS foo FROM path", "data/path.shp"] ) - output_string = info.GetArg("output-string").Get() + output_string = info["output-string"] j = json.loads(output_string) assert len(j["layers"][0]["fields"]) == 1 assert j["layers"][0]["fields"][0]["name"] == "foo" @@ -77,7 +75,7 @@ def test_gdalalg_vector_info_sql(): def test_gdalalg_vector_info_layer(): info = get_info_alg() assert info.ParseRunAndFinalize(["-l", "path", "data/path.shp"]) - output_string = info.GetArg("output-string").Get() + output_string = info["output-string"] j = json.loads(output_string) assert j["layers"][0]["name"] == "path" @@ -92,7 +90,7 @@ def test_gdalalg_vector_info_wrong_layer(): def test_gdalalg_vector_info_where(cond, featureCount): info = get_info_alg() assert info.ParseRunAndFinalize(["--where", cond, "data/path.shp"]) - output_string = info.GetArg("output-string").Get() + output_string = info["output-string"] j = json.loads(output_string) assert j["layers"][0]["featureCount"] == featureCount @@ -110,6 +108,6 @@ def test_gdalalg_vector_info_dialect(): "data/path.shp", ] ) - output_string = info.GetArg("output-string").Get() + output_string = info["output-string"] j = json.loads(output_string) assert j["layers"][0]["features"][0]["properties"]["version"].startswith("3.") diff --git a/autotest/utilities/test_gdalalg_vector_pipeline.py b/autotest/utilities/test_gdalalg_vector_pipeline.py index d16968344b56..db93707735f7 100755 --- a/autotest/utilities/test_gdalalg_vector_pipeline.py +++ b/autotest/utilities/test_gdalalg_vector_pipeline.py @@ -19,9 +19,7 @@ def get_pipeline_alg(): - reg = gdal.GetGlobalAlgorithmRegistry() - vector = reg.InstantiateAlg("vector") - return vector.InstantiateSubAlgorithm("pipeline") + return gdal.GetGlobalAlgorithmRegistry()["vector"]["pipeline"] def test_gdalalg_vector_pipeline_read_and_write(tmp_vsimem): @@ -68,9 +66,9 @@ def test_gdalalg_vector_pipeline_as_api(tmp_vsimem): out_filename = str(tmp_vsimem / "out.shp") pipeline = get_pipeline_alg() - pipeline.GetArg("pipeline").Set(f"read ../ogr/data/poly.shp ! write {out_filename}") + pipeline["pipeline"] = f"read ../ogr/data/poly.shp ! write {out_filename}" assert pipeline.Run() - ds = pipeline.GetArg("output").Get().GetDataset() + ds = pipeline["output"].GetDataset() assert ds.GetLayer(0).GetFeatureCount() == 10 assert pipeline.Finalize() ds = None @@ -84,8 +82,8 @@ def test_gdalalg_vector_pipeline_input_through_api(tmp_vsimem): out_filename = str(tmp_vsimem / "out.shp") pipeline = get_pipeline_alg() - pipeline.GetArg("input").Get().SetDataset(gdal.OpenEx("../ogr/data/poly.shp")) - pipeline.GetArg("pipeline").Set(f"read ! write {out_filename}") + pipeline["input"] = gdal.OpenEx("../ogr/data/poly.shp") + pipeline["pipeline"] = f"read ! write {out_filename}" assert pipeline.Run() assert pipeline.Finalize() @@ -98,8 +96,8 @@ def test_gdalalg_vector_pipeline_input_through_api_run_twice(tmp_vsimem): out_filename = str(tmp_vsimem / "out.shp") pipeline = get_pipeline_alg() - pipeline.GetArg("input").Get().SetDataset(gdal.OpenEx("../ogr/data/poly.shp")) - pipeline.GetArg("pipeline").Set(f"read ! write {out_filename}") + pipeline["input"] = gdal.OpenEx("../ogr/data/poly.shp") + pipeline["pipeline"] = f"read ! write {out_filename}" assert pipeline.Run() with pytest.raises( Exception, match=r"pipeline: Step nr 0 \(read\) has already an output dataset" @@ -112,8 +110,8 @@ def test_gdalalg_vector_pipeline_output_through_api(tmp_vsimem): out_filename = str(tmp_vsimem / "out.shp") pipeline = get_pipeline_alg() - pipeline.GetArg("output").Get().SetName(out_filename) - pipeline.GetArg("pipeline").Set("read ../ogr/data/poly.shp ! write") + pipeline["output"] = out_filename + pipeline["pipeline"] = "read ../ogr/data/poly.shp ! write" assert pipeline.Run() assert pipeline.Finalize() @@ -124,7 +122,7 @@ def test_gdalalg_vector_pipeline_output_through_api(tmp_vsimem): def test_gdalalg_vector_pipeline_as_api_error(): pipeline = get_pipeline_alg() - pipeline.GetArg("pipeline").Set("read") + pipeline["pipeline"] = "read" with pytest.raises(Exception, match="pipeline: At least 2 steps must be provided"): pipeline.Run() @@ -624,10 +622,10 @@ def test_gdalalg_vector_pipeline_reproject_missing_layer_crs(tmp_vsimem): pipeline = get_pipeline_alg() mem_ds = gdal.GetDriverByName("Memory").Create("", 0, 0, 0, gdal.GDT_Unknown) mem_ds.CreateLayer("layer") - pipeline.GetArg("input").Get().SetDataset(mem_ds) - pipeline.GetArg("pipeline").Set( - f"read ! reproject --dst-crs=EPSG:4326 ! write {out_filename}" - ) + pipeline["input"] = mem_ds + pipeline[ + "pipeline" + ] = f"read ! reproject --dst-crs=EPSG:4326 ! write {out_filename}" with pytest.raises( Exception, match="reproject: Layer 'layer' has no spatial reference system" ): diff --git a/swig/include/python/gdal_python.i b/swig/include/python/gdal_python.i index 3421377200fb..db9a0ac37dcb 100644 --- a/swig/include/python/gdal_python.i +++ b/swig/include/python/gdal_python.i @@ -5251,6 +5251,92 @@ class VSIFile(BytesIO): return VSIFTellL(self._fp) %} + +/* -------------------------------------------------------------------- */ +/* GDALAlgorithmRegistryHS */ +/* -------------------------------------------------------------------- */ + +%extend GDALAlgorithmRegistryHS { +%pythoncode %{ + + def __getitem__(self, key): + """Instantiate an algorithm + + Shortcut for self.InstantiateAlg(key) + + Example + ------- + >>> gdal.GetGlobalAlgorithmRegistry()["raster"] + """ + + return self.InstantiateAlg(key) +%} +} + +/* -------------------------------------------------------------------- */ +/* GDALAlgorithmHS */ +/* -------------------------------------------------------------------- */ + +%extend GDALAlgorithmHS { +%pythoncode %{ + + def __getitem__(self, key): + """Get the value of an argument. + + Shortcut for self.GetActualAlgorithm().GetArg(key).Get() + or self.InstantiateSubAlgorithm(key) for a non-leaf algorithm + + Parameters + ----------- + key: str + Name of a known argument of the algorithm + value: + Value of the argument + + Example + ------- + >>> alg["output-string"] + >>> alg["output"].GetName() + >>> alg["output"].GetDataset() + >>> gdal.GetGlobalAlgorithmRegistry()["raster"]["info"] + """ + + if self.HasSubAlgorithms(): + return self.InstantiateSubAlgorithm(key) + else: + return self.GetActualAlgorithm().GetArg(key).Get() + + def __setitem__(self, key, value): + """Set the value of an argment. + + Shortcut for self.GetArg(key).Set(value) + + Parameters + ----------- + key: str + Name of a known argument of the algorithm + value: + Value of the argument + + Examples + -------- + >>> alg["bbox"] = [2, 49, 3, 50] + >>> alg["where"] = "country = 'France'" + >>> alg["input"] = "byte.tif" + >>> alg["input"] = gdal.Open("byte.tif") + >>> alg["target-aligned-pixels"] = True + + >>> # Multiple input datasets + >>> alg["input"] = ["one.tif", "two.tif"] + >>> alg["input"] = [one_ds, two_ds] + """ + + if not self.GetArg(key).Set(value): + raise Exception(f"Cannot set argument {key} to {value}") +%} +} + + /* -------------------------------------------------------------------- */ /* GDALAlgorithmArgHS */ /* -------------------------------------------------------------------- */ @@ -5290,9 +5376,11 @@ class VSIFile(BytesIO): return self.SetAsDouble(value) if type == GAAT_DATASET: if isinstance(value, str): - return self.GetAsDatasetValue().SetName(value) + self.GetAsDatasetValue().SetName(value) + return True elif isinstance(value, Dataset): - return self.GetAsDatasetValue().SetDataset(value) + self.GetAsDatasetValue().SetDataset(value) + return True else: return self.SetAsDatasetValue(value) if type == GAAT_STRING_LIST: From 8edd60120b9d350fdcf3166dfabbf0893b76ec26 Mon Sep 17 00:00:00 2001 From: Even Rouault Date: Fri, 7 Feb 2025 18:35:03 +0100 Subject: [PATCH 4/6] Doc: add a few examples of how to use GDAL algorithms from Python --- doc/source/programs/gdal_cli_from_python.rst | 128 +++++++++++++++++++ doc/source/programs/index.rst | 2 + 2 files changed, 130 insertions(+) create mode 100644 doc/source/programs/gdal_cli_from_python.rst diff --git a/doc/source/programs/gdal_cli_from_python.rst b/doc/source/programs/gdal_cli_from_python.rst new file mode 100644 index 000000000000..93ad2fa49813 --- /dev/null +++ b/doc/source/programs/gdal_cli_from_python.rst @@ -0,0 +1,128 @@ +.. _gdal_cli_from_python: + +================================================================================ +How to use "gdal" CLI algorithms from Python +================================================================================ + +Raster commands +--------------- + +* Getting information on a raster dataset as JSON + +.. code-block:: python + + from osgeo import gdal + import json + + gdal.UseExceptions() + alg = gdal.GetGlobalAlgorithmRegistry()["raster"]["info"] + alg["input"] = "byte.tif" + alg.Run() + info = json.loads(alg["output-string"]) + print(info) + + +* Converting a georeferenced netCDF file to cloud-optimized GeoTIFF + +.. code-block:: python + + from osgeo import gdal + gdal.UseExceptions() + alg = gdal.GetGlobalAlgorithmRegistry()["raster"]["convert"] + alg["input"] = "in.nc" + alg["output"] = "out.tif" + alg["output-format"] = "COG" + alg["overwrite"] = True # if the output file may exist + alg.Run() + alg.Finalize() # ensure output dataset is closed + +or + +.. code-block:: python + + from osgeo import gdal + gdal.UseExceptions() + alg = gdal.GetGlobalAlgorithmRegistry()["raster"]["convert"] + alg.ParseRunAndFinalize(["--input=in.nc", "--output-format=COG", "--output=out.tif", "--overwrite"]) + + +* Reprojecting a GeoTIFF file to a Deflate compressed tiled GeoTIFF file + +.. code-block:: python + + from osgeo import gdal + gdal.UseExceptions() + alg = gdal.GetGlobalAlgorithmRegistry()["raster"]["reproject"] + alg["input"] = "in.tif" + alg["output"] = "out.tif" + alg["dst-crs"] = "EPSG:4326" + alg["creation-options"] = ["TILED=YES", "COMPRESS=DEFLATE"] + alg["overwrite"] = True # if the output file may exist + alg.Run() + alg.Finalize() # ensure output dataset is closed + +or + +.. code-block:: python + + from osgeo import gdal + gdal.UseExceptions() + alg = gdal.GetGlobalAlgorithmRegistry()["raster"]["reproject"] + alg.ParseRunAndFinalize(["--input=in.tif", "--output=out.tif", "--dst-crs=EPSG:4326", "--co=TILED=YES,COMPRESS=DEFLATE", "--overwrite"]) + + +* Reprojecting a (possibly in-memory) dataset to a in-memory dataset + +.. code-block:: python + + from osgeo import gdal + gdal.UseExceptions() + alg = gdal.GetGlobalAlgorithmRegistry()["raster"]["reproject"] + alg["input"] = input_ds + alg["output"] = "dummy_name" + alg["output-format"] = "MEM" + alg["dst-crs"] = "EPSG:4326" + alg.Run() + output_ds = alg["output"].GetDataset() + + + +Vector commands +--------------- + +* Getting information on a vector dataset as JSON + +.. code-block:: python + + from osgeo import gdal + import json + + gdal.UseExceptions() + alg = gdal.GetGlobalAlgorithmRegistry()["vector"]["info"] + alg["input"] = "poly.gpkg" + alg.Run() + info = json.loads(alg["output-string"]) + print(info) + + +* Converting a shapefile to a GeoPackage + +.. code-block:: python + + from osgeo import gdal + gdal.UseExceptions() + alg = gdal.GetGlobalAlgorithmRegistry()["vector"]["convert"] + alg["input"] = "in.shp" + alg["output"] = "out.gpkg" + alg["overwrite"] = True # if the output file may exist + alg.Run() + alg.Finalize() # ensure output dataset is closed + +or + +.. code-block:: python + + from osgeo import gdal + gdal.UseExceptions() + alg = gdal.GetGlobalAlgorithmRegistry()["vector"]["convert"] + alg.ParseRunAndFinalize(["--input=in.shp", "--output=out.gpkg", "--overwrite"]) diff --git a/doc/source/programs/index.rst b/doc/source/programs/index.rst index 95477fda0777..f98a1bccd476 100644 --- a/doc/source/programs/index.rst +++ b/doc/source/programs/index.rst @@ -26,6 +26,7 @@ single :program:`gdal` program that accepts commands and subcommands. :hidden: migration_guide_to_gdal_cli + gdal_cli_from_python gdal gdal_info gdal_convert @@ -50,6 +51,7 @@ single :program:`gdal` program that accepts commands and subcommands. .. only:: html - :ref:`migration_guide_to_gdal_cli`: Migration guide to "gdal" command line interface + - :ref:`gdal_cli_from_python`: How to use "gdal" CLI algorithms from Python - :ref:`gdal_program`: Main "gdal" entry point - :ref:`gdal_info_command`: Get information on a dataset - :ref:`gdal_convert_command`: Convert a dataset From f80739ac623fe682914b35c79a481d40e9c74520 Mon Sep 17 00:00:00 2001 From: Even Rouault Date: Fri, 7 Feb 2025 15:19:44 +0100 Subject: [PATCH 5/6] Make all Transform methods of OGRCoordinateTransformation and GDALTransformerFunc return FALSE as soon as one point fails to transform Fixes #11817 Kind of a breaking change, but the current behavior was highly inconsistent and hard to reason about. New paragraph in MIGRATION_GUIDE.TXT: - The following methods OGRCoordinateTransformation::Transform(size_t nCount, double *x, double *y, double *z, double *t, int *pabSuccess) and OGRCoordinateTransformation::TransformWithErrorCodes(size_t nCount, double *x, double *y, double *z, double *t, int *panErrorCodes) are modified to return FALSE as soon as at least one point fails to transform (to be consistent with the other form of Transform() that doesn't take a "t" argument), whereas previously they would return FALSE only if no transformation was found. When FALSE is returned the pabSuccess[] or panErrorCodes[] arrays indicate which point succeeded or failed to transform. The GDALTransformerFunc callback and its implementations (GenImgProjTransformer, RPCTransformer, etc.) are also modified to return FALSE as soon as at least one point fails to transform. --- MIGRATION_GUIDE.TXT | 15 +++ alg/gdal_alg.h | 16 +++ alg/gdal_crs.cpp | 6 +- alg/gdal_rpc.cpp | 25 +++- alg/gdal_tps.cpp | 2 +- alg/gdalgeoloc.cpp | 9 +- alg/gdaltransformer.cpp | 186 +++++++++++++----------------- alg/gdalwarpoperation.cpp | 28 +---- apps/gdalwarp_lib.cpp | 51 ++++---- autotest/gcore/geoloc.py | 6 +- doc/source/spelling_wordlist.txt | 2 + ogr/ogr_spatialref.h | 24 +++- ogr/ogrct.cpp | 192 ++++++++++++++++--------------- 13 files changed, 300 insertions(+), 262 deletions(-) diff --git a/MIGRATION_GUIDE.TXT b/MIGRATION_GUIDE.TXT index c4ad53a63d5e..883819628f47 100644 --- a/MIGRATION_GUIDE.TXT +++ b/MIGRATION_GUIDE.TXT @@ -10,6 +10,21 @@ MIGRATION GUIDE FROM GDAL 3.10 to GDAL 3.11 - If only a specific GDAL Minor version is to be supported, this must now be specified in the find_package call in CMake via a version range specification. +- The following methods + OGRCoordinateTransformation::Transform(size_t nCount, double *x, double *y, + double *z, double *t, int *pabSuccess) and + OGRCoordinateTransformation::TransformWithErrorCodes(size_t nCount, double *x, + double *y, double *z, double *t, int *panErrorCodes) are modified to return + FALSE as soon as at least one point fails to transform (to be consistent with + the other form of Transform() that doesn't take a "t" argument), whereas + previously they would return FALSE only if no transformation was found. When + FALSE is returned the pabSuccess[] or panErrorCodes[] arrays indicate which + point succeeded or failed to transform. + + The GDALTransformerFunc callback and its implementations (GenImgProjTransformer, + RPCTransformer, etc.) are also modified to return FALSE as soon as at least + one point fails to transform. + MIGRATION GUIDE FROM GDAL 3.9 to GDAL 3.10 ------------------------------------------ diff --git a/alg/gdal_alg.h b/alg/gdal_alg.h index e92cd6625313..8bccbb4429d4 100644 --- a/alg/gdal_alg.h +++ b/alg/gdal_alg.h @@ -76,6 +76,22 @@ CPLErr CPL_DLL CPL_STDCALL GDALSieveFilter( * Warp Related. */ +/** + * Callback to transforms points. + * + * @param pTransformerArg return value from a GDALCreateXXXXTransformer() function + * @param bDstToSrc TRUE if transformation is from the destination + * (georeferenced) coordinates to pixel/line or FALSE when transforming + * from pixel/line to georeferenced coordinates. + * @param nPointCount the number of values in the x, y and z arrays. + * @param[in,out] x array containing the X values to be transformed. Must not be NULL. + * @param[in,out] y array containing the Y values to be transformed. Must not be NULL. + * @param[in,out] z array containing the Z values to be transformed. Must not be NULL. + * @param[out] panSuccess array in which a flag indicating success (TRUE) or + * failure (FALSE) of the transformation are placed. Must not be NULL. + * + * @return TRUE if all points have been successfully transformed. + */ typedef int (*GDALTransformerFunc)(void *pTransformerArg, int bDstToSrc, int nPointCount, double *x, double *y, double *z, int *panSuccess); diff --git a/alg/gdal_crs.cpp b/alg/gdal_crs.cpp index 9c134f0b69f5..d58e508f4b4c 100644 --- a/alg/gdal_crs.cpp +++ b/alg/gdal_crs.cpp @@ -422,7 +422,7 @@ void GDALDestroyGCPTransformer(void *pTransformArg) * @param panSuccess array in which a flag indicating success (TRUE) or * failure (FALSE) of the transformation are placed. * - * @return TRUE. + * @return TRUE if all points have been successfully transformed. */ int GDALGCPTransform(void *pTransformArg, int bDstToSrc, int nPointCount, @@ -436,10 +436,12 @@ int GDALGCPTransform(void *pTransformArg, int bDstToSrc, int nPointCount, if (psInfo->bReversed) bDstToSrc = !bDstToSrc; + int bRet = TRUE; for (i = 0; i < nPointCount; i++) { if (x[i] == HUGE_VAL || y[i] == HUGE_VAL) { + bRet = FALSE; panSuccess[i] = FALSE; continue; } @@ -459,7 +461,7 @@ int GDALGCPTransform(void *pTransformArg, int bDstToSrc, int nPointCount, panSuccess[i] = TRUE; } - return TRUE; + return bRet; } /************************************************************************/ diff --git a/alg/gdal_rpc.cpp b/alg/gdal_rpc.cpp index cbff487fc53c..fa342ae613cf 100644 --- a/alg/gdal_rpc.cpp +++ b/alg/gdal_rpc.cpp @@ -1449,10 +1449,15 @@ GDALRPCTransformWholeLineWithDEM(const GDALRPCTransformInfo *psTransform, const int nY = static_cast(dfY); const double dfDeltaY = dfY - nY; + int bRet = TRUE; for (int i = 0; i < nPointCount; i++) { if (padfX[i] == HUGE_VAL) + { + bRet = FALSE; + panSuccess[i] = FALSE; continue; + } double dfDEMH = 0.0; const double dfZ_i = padfZ ? padfZ[i] : 0.0; @@ -1502,6 +1507,7 @@ GDALRPCTransformWholeLineWithDEM(const GDALRPCTransformInfo *psTransform, dfDEMH = psTransform->dfDEMMissingValue; else { + bRet = FALSE; panSuccess[i] = FALSE; continue; } @@ -1546,6 +1552,7 @@ GDALRPCTransformWholeLineWithDEM(const GDALRPCTransformInfo *psTransform, { if (!RPCIsValidLongLat(psTransform, padfX[i], padfY[i])) { + bRet = FALSE; panSuccess[i] = FALSE; padfX[i] = HUGE_VAL; padfY[i] = HUGE_VAL; @@ -1565,6 +1572,7 @@ GDALRPCTransformWholeLineWithDEM(const GDALRPCTransformInfo *psTransform, { if (!RPCIsValidLongLat(psTransform, padfX[i], padfY[i])) { + bRet = FALSE; panSuccess[i] = FALSE; padfX[i] = HUGE_VAL; padfY[i] = HUGE_VAL; @@ -1582,6 +1590,7 @@ GDALRPCTransformWholeLineWithDEM(const GDALRPCTransformInfo *psTransform, } else { + bRet = FALSE; panSuccess[i] = FALSE; padfX[i] = HUGE_VAL; padfY[i] = HUGE_VAL; @@ -1613,6 +1622,7 @@ GDALRPCTransformWholeLineWithDEM(const GDALRPCTransformInfo *psTransform, dfDEMH = psTransform->dfDEMMissingValue; else { + bRet = FALSE; panSuccess[i] = FALSE; padfX[i] = HUGE_VAL; padfY[i] = HUGE_VAL; @@ -1623,6 +1633,7 @@ GDALRPCTransformWholeLineWithDEM(const GDALRPCTransformInfo *psTransform, if (!RPCIsValidLongLat(psTransform, padfX[i], padfY[i])) { + bRet = FALSE; panSuccess[i] = FALSE; padfX[i] = HUGE_VAL; padfY[i] = HUGE_VAL; @@ -1638,7 +1649,7 @@ GDALRPCTransformWholeLineWithDEM(const GDALRPCTransformInfo *psTransform, VSIFree(padfDEMBuffer); - return TRUE; + return bRet; } /************************************************************************/ @@ -1900,10 +1911,12 @@ int GDALRPCTransform(void *pTransformArg, int bDstToSrc, int nPointCount, } } + int bRet = TRUE; for (int i = 0; i < nPointCount; i++) { if (!RPCIsValidLongLat(psTransform, padfX[i], padfY[i])) { + bRet = FALSE; panSuccess[i] = FALSE; padfX[i] = HUGE_VAL; padfY[i] = HUGE_VAL; @@ -1913,6 +1926,7 @@ int GDALRPCTransform(void *pTransformArg, int bDstToSrc, int nPointCount, if (!GDALRPCGetHeightAtLongLat(psTransform, padfX[i], padfY[i], &dfHeight)) { + bRet = FALSE; panSuccess[i] = FALSE; padfX[i] = HUGE_VAL; padfY[i] = HUGE_VAL; @@ -1925,13 +1939,15 @@ int GDALRPCTransform(void *pTransformArg, int bDstToSrc, int nPointCount, panSuccess[i] = TRUE; } - return TRUE; + return bRet; } if (padfZ == nullptr) { CPLError(CE_Failure, CPLE_NotSupported, "Z array should be provided for reverse RPC computation"); + for (int i = 0; i < nPointCount; i++) + panSuccess[i] = FALSE; return FALSE; } @@ -1940,6 +1956,7 @@ int GDALRPCTransform(void *pTransformArg, int bDstToSrc, int nPointCount, /* function uses an iterative method from an initial linear */ /* approximation. */ /* -------------------------------------------------------------------- */ + int bRet = TRUE; for (int i = 0; i < nPointCount; i++) { double dfResultX = 0.0; @@ -1948,6 +1965,7 @@ int GDALRPCTransform(void *pTransformArg, int bDstToSrc, int nPointCount, if (!RPCInverseTransformPoint(psTransform, padfX[i], padfY[i], padfZ[i], &dfResultX, &dfResultY)) { + bRet = FALSE; panSuccess[i] = FALSE; padfX[i] = HUGE_VAL; padfY[i] = HUGE_VAL; @@ -1955,6 +1973,7 @@ int GDALRPCTransform(void *pTransformArg, int bDstToSrc, int nPointCount, } if (!RPCIsValidLongLat(psTransform, padfX[i], padfY[i])) { + bRet = FALSE; panSuccess[i] = FALSE; padfX[i] = HUGE_VAL; padfY[i] = HUGE_VAL; @@ -1967,7 +1986,7 @@ int GDALRPCTransform(void *pTransformArg, int bDstToSrc, int nPointCount, panSuccess[i] = TRUE; } - return TRUE; + return bRet; } /************************************************************************/ diff --git a/alg/gdal_tps.cpp b/alg/gdal_tps.cpp index cb259c95e38f..81d9b240c945 100644 --- a/alg/gdal_tps.cpp +++ b/alg/gdal_tps.cpp @@ -324,7 +324,7 @@ void GDALDestroyTPSTransformer(void *pTransformArg) * @param panSuccess array in which a flag indicating success (TRUE) or * failure (FALSE) of the transformation are placed. * - * @return TRUE. + * @return TRUE if all points have been successfully transformed. */ int GDALTPSTransform(void *pTransformArg, int bDstToSrc, int nPointCount, diff --git a/alg/gdalgeoloc.cpp b/alg/gdalgeoloc.cpp index 32f28648cc1f..1cff39151721 100644 --- a/alg/gdalgeoloc.cpp +++ b/alg/gdalgeoloc.cpp @@ -589,6 +589,7 @@ int GDALGeoLoc::Transform(void *pTransformArg, int bDstToSrc, double *padfY, double * /* padfZ */, int *panSuccess) { + int bSuccess = TRUE; GDALGeoLocTransformInfo *psTransform = static_cast(pTransformArg); @@ -607,6 +608,7 @@ int GDALGeoLoc::Transform(void *pTransformArg, int bDstToSrc, { if (padfX[i] == HUGE_VAL || padfY[i] == HUGE_VAL) { + bSuccess = FALSE; panSuccess[i] = FALSE; continue; } @@ -623,6 +625,7 @@ int GDALGeoLoc::Transform(void *pTransformArg, int bDstToSrc, if (!PixelLineToXY(psTransform, dfGeoLocPixel, dfGeoLocLine, padfX[i], padfY[i])) { + bSuccess = FALSE; panSuccess[i] = FALSE; padfX[i] = HUGE_VAL; padfY[i] = HUGE_VAL; @@ -665,6 +668,7 @@ int GDALGeoLoc::Transform(void *pTransformArg, int bDstToSrc, { if (padfX[i] == HUGE_VAL || padfY[i] == HUGE_VAL) { + bSuccess = FALSE; panSuccess[i] = FALSE; continue; } @@ -688,6 +692,7 @@ int GDALGeoLoc::Transform(void *pTransformArg, int bDstToSrc, dfBMX + 1 < psTransform->nBackMapWidth && dfBMY + 1 < psTransform->nBackMapHeight)) { + bSuccess = FALSE; panSuccess[i] = FALSE; padfX[i] = HUGE_VAL; padfY[i] = HUGE_VAL; @@ -701,6 +706,7 @@ int GDALGeoLoc::Transform(void *pTransformArg, int bDstToSrc, const auto fBMY_0_0 = pAccessors->backMapYAccessor.Get(iBMX, iBMY); if (fBMX_0_0 == INVALID_BMXY) { + bSuccess = FALSE; panSuccess[i] = FALSE; padfX[i] = HUGE_VAL; padfY[i] = HUGE_VAL; @@ -914,6 +920,7 @@ int GDALGeoLoc::Transform(void *pTransformArg, int bDstToSrc, } if (!bDone) { + bSuccess = FALSE; panSuccess[i] = FALSE; padfX[i] = HUGE_VAL; padfY[i] = HUGE_VAL; @@ -924,7 +931,7 @@ int GDALGeoLoc::Transform(void *pTransformArg, int bDstToSrc, } } - return TRUE; + return bSuccess; } /*! @endcond */ diff --git a/alg/gdaltransformer.cpp b/alg/gdaltransformer.cpp index 8e5e5e2b02a0..08c059d9149f 100644 --- a/alg/gdaltransformer.cpp +++ b/alg/gdaltransformer.cpp @@ -185,10 +185,9 @@ CPLErr CPL_STDCALL GDALSuggestedWarpOutput(GDALDatasetH hSrcDS, adfExtent, 0); } -static int GDALSuggestedWarpOutput2_MustAdjustForRightBorder( +static bool GDALSuggestedWarpOutput2_MustAdjustForRightBorder( GDALTransformerFunc pfnTransformer, void *pTransformArg, double *padfExtent, - CPL_UNUSED int nPixels, int nLines, double dfPixelSizeX, - double dfPixelSizeY) + int /* nPixels*/, int nLines, double dfPixelSizeX, double dfPixelSizeY) { double adfX[21] = {}; double adfY[21] = {}; @@ -213,38 +212,35 @@ static int GDALSuggestedWarpOutput2_MustAdjustForRightBorder( int abSuccess[21] = {}; - bool bErr = false; - if (!pfnTransformer(pTransformArg, TRUE, nSamplePoints, adfX, adfY, adfZ, - abSuccess)) - { - bErr = true; - } + pfnTransformer(pTransformArg, TRUE, nSamplePoints, adfX, adfY, adfZ, + abSuccess); - if (!bErr && !pfnTransformer(pTransformArg, FALSE, nSamplePoints, adfX, - adfY, adfZ, abSuccess)) - { - bErr = true; - } + int abSuccess2[21] = {}; + + pfnTransformer(pTransformArg, FALSE, nSamplePoints, adfX, adfY, adfZ, + abSuccess2); nSamplePoints = 0; int nBadCount = 0; - for (double dfRatio = 0.0; !bErr && dfRatio <= 1.01; dfRatio += 0.05) + for (double dfRatio = 0.0; dfRatio <= 1.01; dfRatio += 0.05) { const double expected_x = dfMaxXOut; const double expected_y = dfMaxYOut - dfPixelSizeY * dfRatio * nLines; - if (fabs(adfX[nSamplePoints] - expected_x) > dfPixelSizeX || + if (!abSuccess[nSamplePoints] || !abSuccess2[nSamplePoints] || + fabs(adfX[nSamplePoints] - expected_x) > dfPixelSizeX || fabs(adfY[nSamplePoints] - expected_y) > dfPixelSizeY) + { nBadCount++; + } nSamplePoints++; } return nBadCount == nSamplePoints; } -static int GDALSuggestedWarpOutput2_MustAdjustForBottomBorder( +static bool GDALSuggestedWarpOutput2_MustAdjustForBottomBorder( GDALTransformerFunc pfnTransformer, void *pTransformArg, double *padfExtent, - int nPixels, CPL_UNUSED int nLines, double dfPixelSizeX, - double dfPixelSizeY) + int nPixels, int /* nLines */, double dfPixelSizeX, double dfPixelSizeY) { double adfX[21] = {}; double adfY[21] = {}; @@ -269,28 +265,26 @@ static int GDALSuggestedWarpOutput2_MustAdjustForBottomBorder( int abSuccess[21] = {}; - bool bErr = false; - if (!pfnTransformer(pTransformArg, TRUE, nSamplePoints, adfX, adfY, adfZ, - abSuccess)) - { - bErr = true; - } + pfnTransformer(pTransformArg, TRUE, nSamplePoints, adfX, adfY, adfZ, + abSuccess); - if (!bErr && !pfnTransformer(pTransformArg, FALSE, nSamplePoints, adfX, - adfY, adfZ, abSuccess)) - { - bErr = true; - } + int abSuccess2[21] = {}; + + pfnTransformer(pTransformArg, FALSE, nSamplePoints, adfX, adfY, adfZ, + abSuccess2); nSamplePoints = 0; int nBadCount = 0; - for (double dfRatio = 0.0; !bErr && dfRatio <= 1.01; dfRatio += 0.05) + for (double dfRatio = 0.0; dfRatio <= 1.01; dfRatio += 0.05) { const double expected_x = dfMinXOut + dfPixelSizeX * dfRatio * nPixels; const double expected_y = dfMinYOut; - if (fabs(adfX[nSamplePoints] - expected_x) > dfPixelSizeX || + if (!abSuccess[nSamplePoints] || !abSuccess2[nSamplePoints] || + fabs(adfX[nSamplePoints] - expected_x) > dfPixelSizeX || fabs(adfY[nSamplePoints] - expected_y) > dfPixelSizeY) + { nBadCount++; + } nSamplePoints++; } @@ -530,17 +524,10 @@ CPLErr CPL_STDCALL GDALSuggestedWarpOutput2(GDALDatasetH hSrcDS, /* -------------------------------------------------------------------- */ /* Transform them to the output coordinate system. */ /* -------------------------------------------------------------------- */ - if (!pfnTransformer(pTransformArg, FALSE, nSamplePoints, padfX, padfY, - padfZ, pabSuccess)) - { - CPLError(CE_Failure, CPLE_AppDefined, - "GDALSuggestedWarpOutput() failed because the passed " - "transformer failed."); - CPLFree(padfX); - CPLFree(padfXRevert); - CPLFree(pabSuccess); - return CE_Failure; - } + CPLTurnFailureIntoWarning(true); + pfnTransformer(pTransformArg, FALSE, nSamplePoints, padfX, padfY, padfZ, + pabSuccess); + CPLTurnFailureIntoWarning(false); constexpr int SIGN_FINAL_UNINIT = -2; constexpr int SIGN_FINAL_INVALID = 0; @@ -601,6 +588,7 @@ CPLErr CPL_STDCALL GDALSuggestedWarpOutput2(GDALDatasetH hSrcDS, double ayTemp[2] = {padfY[i], padfY[i]}; double azTemp[2] = {padfZ[i], padfZ[i]}; int abSuccess[2] = {FALSE, FALSE}; + CPLTurnFailureIntoWarning(true); if (pfnTransformer(pTransformArg, TRUE, 2, axTemp, ayTemp, azTemp, abSuccess) && fabs(axTemp[0] - axTemp[1]) < 1e-8 && @@ -608,6 +596,7 @@ CPLErr CPL_STDCALL GDALSuggestedWarpOutput2(GDALDatasetH hSrcDS, { padfX[i] = iSignDiscontinuity * 180.0; } + CPLTurnFailureIntoWarning(false); } } } @@ -626,56 +615,53 @@ CPLErr CPL_STDCALL GDALSuggestedWarpOutput2(GDALDatasetH hSrcDS, memcpy(padfXRevert, padfX, nSamplePoints * sizeof(double)); memcpy(padfYRevert, padfY, nSamplePoints * sizeof(double)); memcpy(padfZRevert, padfZ, nSamplePoints * sizeof(double)); - if (pfnTransformer(pTransformArg, TRUE, nSamplePoints, padfXRevert, - padfYRevert, padfZRevert, pabSuccess)) + CPLTurnFailureIntoWarning(true); + pfnTransformer(pTransformArg, TRUE, nSamplePoints, padfXRevert, + padfYRevert, padfZRevert, pabSuccess); + CPLTurnFailureIntoWarning(false); + + for (int i = 0; nFailedCount == 0 && i < nSamplePoints; i++) { - for (int i = 0; nFailedCount == 0 && i < nSamplePoints; i++) + if (!pabSuccess[i]) { - if (!pabSuccess[i]) - { - nFailedCount++; - break; - } + nFailedCount++; + break; + } - double dfRatio = (i % nStepsPlusOne) * dfStep; - if (dfRatio > 0.99) - dfRatio = 1.0; + double dfRatio = (i % nStepsPlusOne) * dfStep; + if (dfRatio > 0.99) + dfRatio = 1.0; - double dfExpectedX = 0.0; - double dfExpectedY = 0.0; - if (i < nStepsPlusOne) - { - dfExpectedX = dfRatio * nInXSize; - } - else if (i < 2 * nStepsPlusOne) - { - dfExpectedX = dfRatio * nInXSize; - dfExpectedY = nInYSize; - } - else if (i < 3 * nStepsPlusOne) - { - dfExpectedY = dfRatio * nInYSize; - } - else - { - dfExpectedX = nInXSize; - dfExpectedY = dfRatio * nInYSize; - } - - if (fabs(padfXRevert[i] - dfExpectedX) > - nInXSize / static_cast(nSteps) || - fabs(padfYRevert[i] - dfExpectedY) > - nInYSize / static_cast(nSteps)) - nFailedCount++; + double dfExpectedX = 0.0; + double dfExpectedY = 0.0; + if (i < nStepsPlusOne) + { + dfExpectedX = dfRatio * nInXSize; } - if (nFailedCount != 0) - CPLDebug("WARP", - "At least one point failed after revert transform"); - } - else - { - nFailedCount = 1; + else if (i < 2 * nStepsPlusOne) + { + dfExpectedX = dfRatio * nInXSize; + dfExpectedY = nInYSize; + } + else if (i < 3 * nStepsPlusOne) + { + dfExpectedY = dfRatio * nInYSize; + } + else + { + dfExpectedX = nInXSize; + dfExpectedY = dfRatio * nInYSize; + } + + if (fabs(padfXRevert[i] - dfExpectedX) > + nInXSize / static_cast(nSteps) || + fabs(padfYRevert[i] - dfExpectedY) > + nInYSize / static_cast(nSteps)) + nFailedCount++; } + if (nFailedCount != 0) + CPLDebug("WARP", + "At least one point failed after revert transform"); } /* -------------------------------------------------------------------- */ @@ -707,19 +693,10 @@ CPLErr CPL_STDCALL GDALSuggestedWarpOutput2(GDALDatasetH hSrcDS, CPLAssert(nSamplePoints == nSampleMax); - if (!pfnTransformer(pTransformArg, FALSE, nSamplePoints, padfX, padfY, - padfZ, pabSuccess)) - { - CPLError(CE_Failure, CPLE_AppDefined, - "GDALSuggestedWarpOutput() failed because the passed" - "transformer failed."); - - CPLFree(padfX); - CPLFree(padfXRevert); - CPLFree(pabSuccess); - - return CE_Failure; - } + CPLTurnFailureIntoWarning(true); + pfnTransformer(pTransformArg, FALSE, nSamplePoints, padfX, padfY, padfZ, + pabSuccess); + CPLTurnFailureIntoWarning(false); } /* -------------------------------------------------------------------- */ @@ -2910,11 +2887,12 @@ int GDALGenImgProjTransform(void *pTransformArgIn, int bDstToSrc, pTransformer = psInfo->pSrcTransformer; } + int ret = TRUE; if (pTransformArg != nullptr) { if (!pTransformer(pTransformArg, FALSE, nPointCount, padfX, padfY, padfZ, panSuccess)) - return FALSE; + ret = FALSE; } else { @@ -2942,7 +2920,7 @@ int GDALGenImgProjTransform(void *pTransformArgIn, int bDstToSrc, { if (!psInfo->pReproject(psInfo->pReprojectArg, bDstToSrc, nPointCount, padfX, padfY, padfZ, panSuccess)) - return FALSE; + ret = FALSE; } /* -------------------------------------------------------------------- */ @@ -2965,7 +2943,7 @@ int GDALGenImgProjTransform(void *pTransformArgIn, int bDstToSrc, { if (!pTransformer(pTransformArg, TRUE, nPointCount, padfX, padfY, padfZ, panSuccess)) - return FALSE; + ret = FALSE; } else { @@ -2986,7 +2964,7 @@ int GDALGenImgProjTransform(void *pTransformArgIn, int bDstToSrc, } } - return TRUE; + return ret; } /************************************************************************/ diff --git a/alg/gdalwarpoperation.cpp b/alg/gdalwarpoperation.cpp index f4997c01b16a..fc0bbe9f2956 100644 --- a/alg/gdalwarpoperation.cpp +++ b/alg/gdalwarpoperation.cpp @@ -2579,14 +2579,10 @@ void GDALWarpOperation::ComputeSourceWindowStartingFromSource( /* Transform them to the output pixel coordinate space */ /* -------------------------------------------------------------------- */ - if (!psOptions->pfnTransformer( - psOptions->pTransformerArg, FALSE, nSampleMax, - privateData->adfDstX.data(), privateData->adfDstY.data(), - adfDstZ.data(), privateData->abSuccess.data())) - { - return; - } - + psOptions->pfnTransformer(psOptions->pTransformerArg, FALSE, nSampleMax, + privateData->adfDstX.data(), + privateData->adfDstY.data(), adfDstZ.data(), + privateData->abSuccess.data()); privateData->nStepCount = nStepCount; } @@ -2834,26 +2830,14 @@ bool GDALWarpOperation::ComputeSourceWindowTransformPoints( CPLSetThreadLocalConfigOption("CHECK_WITH_INVERT_PROJ", "YES"); RefreshTransformer(); } - int ret = psOptions->pfnTransformer(psOptions->pTransformerArg, TRUE, - nSamplePoints, padfX, padfY, padfZ, - pabSuccess); + psOptions->pfnTransformer(psOptions->pTransformerArg, TRUE, nSamplePoints, + padfX, padfY, padfZ, pabSuccess); if (bTryWithCheckWithInvertProj) { CPLSetThreadLocalConfigOption("CHECK_WITH_INVERT_PROJ", nullptr); RefreshTransformer(); } - if (!ret) - { - CPLFree(padfX); - CPLFree(pabSuccess); - - CPLError(CE_Failure, CPLE_AppDefined, - "GDALWarperOperation::ComputeSourceWindow() failed because " - "the pfnTransformer failed."); - return false; - } - /* -------------------------------------------------------------------- */ /* Collect the bounds, ignoring any failed points. */ /* -------------------------------------------------------------------- */ diff --git a/apps/gdalwarp_lib.cpp b/apps/gdalwarp_lib.cpp index e0dd6c9b7e19..95bedf8ef0b7 100644 --- a/apps/gdalwarp_lib.cpp +++ b/apps/gdalwarp_lib.cpp @@ -2762,36 +2762,35 @@ static GDALDatasetH GDALWarpDirect(const char *pszDest, GDALDatasetH hDstDS, } } std::vector abSuccess(nPoints); - if (pfnTransformer(hTransformArg, TRUE, nPoints, &adfX[0], - &adfY[0], &adfZ[0], &abSuccess[0])) + pfnTransformer(hTransformArg, TRUE, nPoints, &adfX[0], &adfY[0], + &adfZ[0], &abSuccess[0]); + + double dfMinSrcX = std::numeric_limits::infinity(); + double dfMaxSrcX = -std::numeric_limits::infinity(); + double dfMinSrcY = std::numeric_limits::infinity(); + double dfMaxSrcY = -std::numeric_limits::infinity(); + for (int i = 0; i < nPoints; i++) { - double dfMinSrcX = std::numeric_limits::infinity(); - double dfMaxSrcX = -std::numeric_limits::infinity(); - double dfMinSrcY = std::numeric_limits::infinity(); - double dfMaxSrcY = -std::numeric_limits::infinity(); - for (int i = 0; i < nPoints; i++) + if (abSuccess[i]) { - if (abSuccess[i]) - { - dfMinSrcX = std::min(dfMinSrcX, adfX[i]); - dfMaxSrcX = std::max(dfMaxSrcX, adfX[i]); - dfMinSrcY = std::min(dfMinSrcY, adfY[i]); - dfMaxSrcY = std::max(dfMaxSrcY, adfY[i]); - } - } - if (dfMaxSrcX > dfMinSrcX) - { - dfTargetRatioX = (dfMaxSrcX - dfMinSrcX) / - GDALGetRasterXSize(hDstDS); - } - if (dfMaxSrcY > dfMinSrcY) - { - dfTargetRatioY = (dfMaxSrcY - dfMinSrcY) / - GDALGetRasterYSize(hDstDS); + dfMinSrcX = std::min(dfMinSrcX, adfX[i]); + dfMaxSrcX = std::max(dfMaxSrcX, adfX[i]); + dfMinSrcY = std::min(dfMinSrcY, adfY[i]); + dfMaxSrcY = std::max(dfMaxSrcY, adfY[i]); } - // take the minimum of these ratios #7019 - dfTargetRatio = std::min(dfTargetRatioX, dfTargetRatioY); } + if (dfMaxSrcX > dfMinSrcX) + { + dfTargetRatioX = + (dfMaxSrcX - dfMinSrcX) / GDALGetRasterXSize(hDstDS); + } + if (dfMaxSrcY > dfMinSrcY) + { + dfTargetRatioY = + (dfMaxSrcY - dfMinSrcY) / GDALGetRasterYSize(hDstDS); + } + // take the minimum of these ratios #7019 + dfTargetRatio = std::min(dfTargetRatioX, dfTargetRatioY); } else { diff --git a/autotest/gcore/geoloc.py b/autotest/gcore/geoloc.py index eb5460b2587b..b5058d2ad60e 100755 --- a/autotest/gcore/geoloc.py +++ b/autotest/gcore/geoloc.py @@ -118,10 +118,8 @@ def test_geoloc_fill_line(use_temp_datasets): with gdaltest.config_option("GDAL_GEOLOC_USE_TEMP_DATASETS", use_temp_datasets): warped_ds = gdal.Warp("", ds, format="MEM") assert warped_ds - assert warped_ds.GetRasterBand(1).Checksum() in ( - 22339, - 22336, - ) # 22336 with Intel(R) oneAPI DPC++/C++ Compiler 2022.1.0 + # 20174 with Intel compiler + assert warped_ds.GetRasterBand(1).Checksum() in [20177, 20174] ############################################################################### diff --git a/doc/source/spelling_wordlist.txt b/doc/source/spelling_wordlist.txt index 68ca7fe1ae66..61944360fb33 100644 --- a/doc/source/spelling_wordlist.txt +++ b/doc/source/spelling_wordlist.txt @@ -2244,6 +2244,7 @@ OverviewCount ovf ovr ozi +pabSuccess pabyData pabyDstData pabyHeader @@ -2280,6 +2281,7 @@ panBandList panBandMap pand panDstBands +panErrorCodes panHistogram panMap panOffsets diff --git a/ogr/ogr_spatialref.h b/ogr/ogr_spatialref.h index 23b236006fe2..9d3c8a61855e 100644 --- a/ogr/ogr_spatialref.h +++ b/ogr/ogr_spatialref.h @@ -813,8 +813,10 @@ class CPL_DLL OGRCoordinateTransformation * @param pabSuccess array of per-point flags set to TRUE if that point * transforms, or FALSE if it does not. Might be NULL. * - * @return TRUE if a transformation could be found (but not all points may - * have necessarily succeed to transform), otherwise FALSE. + * @return TRUE on success, or FALSE if some or all points fail to + * transform. When FALSE is returned the pabSuccess[] array indicates which + * points succeeded or failed to transform. When TRUE is returned, all + * values in pabSuccess[] are set to true. */ int Transform(size_t nCount, double *x, double *y, double *z = nullptr, int *pabSuccess = nullptr); @@ -835,8 +837,13 @@ class CPL_DLL OGRCoordinateTransformation * @param pabSuccess array of per-point flags set to TRUE if that point * transforms, or FALSE if it does not. Might be NULL. * - * @return TRUE if a transformation could be found (but not all points may - * have necessarily succeed to transform), otherwise FALSE. + * @return TRUE on success, or FALSE if some or all points fail to + * transform. When FALSE is returned the pabSuccess[] array indicates which + * points succeeded or failed to transform. When TRUE is returned, all + * values in pabSuccess[] are set to true. + * Caution: prior to GDAL 3.11, TRUE could be returned if a + * transformation could be found but not all points may + * have necessarily succeed to transform. */ virtual int Transform(size_t nCount, double *x, double *y, double *z, double *t, int *pabSuccess) = 0; @@ -857,8 +864,13 @@ class CPL_DLL OGRCoordinateTransformation * @param panErrorCodes Output array of nCount value that will be set to 0 * for success, or a non-zero value for failure. Refer to PROJ 8 public * error codes. Might be NULL - * @return TRUE if a transformation could be found (but not all points may - * have necessarily succeed to transform), otherwise FALSE. + * @return TRUE on success, or FALSE if some or all points fail to + * transform. When FALSE is returned the panErrorCodes[] array indicates + * which points succeeded or failed to transform. When TRUE is returned, all + * values in panErrorCodes[] are set to zero. + * Caution: prior to GDAL 3.11, TRUE could be returned if a + * transformation could be found but not all points may + * have necessarily succeed to transform. * @since GDAL 3.3, and PROJ 8 to be able to use PROJ public error codes */ virtual int TransformWithErrorCodes(size_t nCount, double *x, double *y, diff --git a/ogr/ogrct.cpp b/ogr/ogrct.cpp index 4da6f38a037f..ae93f7163579 100644 --- a/ogr/ogrct.cpp +++ b/ogr/ogrct.cpp @@ -2188,22 +2188,12 @@ int OGRCoordinateTransformation::Transform(size_t nCount, double *x, double *y, if (!pabSuccess) return FALSE; - bool bOverallSuccess = - CPL_TO_BOOL(Transform(nCount, x, y, z, nullptr, pabSuccess)); - - for (size_t i = 0; i < nCount; i++) - { - if (!pabSuccess[i]) - { - bOverallSuccess = false; - break; - } - } + const int bRet = Transform(nCount, x, y, z, nullptr, pabSuccess); if (pabSuccess != pabSuccessIn) CPLFree(pabSuccess); - return bOverallSuccess; + return bRet; } /************************************************************************/ @@ -2219,13 +2209,12 @@ int OGRCoordinateTransformation::TransformWithErrorCodes(size_t nCount, if (nCount == 1) { int nSuccess = 0; - const bool bOverallSuccess = - CPL_TO_BOOL(Transform(nCount, x, y, z, t, &nSuccess)); + const int bRet = Transform(nCount, x, y, z, t, &nSuccess); if (panErrorCodes) { panErrorCodes[0] = nSuccess ? 0 : -1; } - return bOverallSuccess; + return bRet; } std::vector abSuccess; @@ -2240,8 +2229,7 @@ int OGRCoordinateTransformation::TransformWithErrorCodes(size_t nCount, return FALSE; } - const bool bOverallSuccess = - CPL_TO_BOOL(Transform(nCount, x, y, z, t, abSuccess.data())); + const int bRet = Transform(nCount, x, y, z, t, abSuccess.data()); if (panErrorCodes) { @@ -2251,7 +2239,7 @@ int OGRCoordinateTransformation::TransformWithErrorCodes(size_t nCount, } } - return bOverallSuccess; + return bRet; } /************************************************************************/ @@ -2262,8 +2250,7 @@ int OGRProjCT::Transform(size_t nCount, double *x, double *y, double *z, double *t, int *pabSuccess) { - bool bOverallSuccess = - CPL_TO_BOOL(TransformWithErrorCodes(nCount, x, y, z, t, pabSuccess)); + const int bRet = TransformWithErrorCodes(nCount, x, y, z, t, pabSuccess); if (pabSuccess) { @@ -2273,7 +2260,7 @@ int OGRProjCT::Transform(size_t nCount, double *x, double *y, double *z, } } - return bOverallSuccess; + return bRet; } /************************************************************************/ @@ -2405,6 +2392,7 @@ int OGRProjCT::TransformWithErrorCodes(size_t nCount, double *x, double *y, /* Optimized transform from WebMercator to WGS84 */ /* -------------------------------------------------------------------- */ bool bTransformDone = false; + int bRet = TRUE; if (bWebMercatorToWGS84LongLat) { constexpr double REVERSE_SPHERE_RADIUS = 1.0 / 6378137.0; @@ -2417,7 +2405,11 @@ int OGRProjCT::TransformWithErrorCodes(size_t nCount, double *x, double *y, double y0 = y[0]; for (size_t i = 0; i < nCount; i++) { - if (x[i] != HUGE_VAL) + if (x[i] == HUGE_VAL) + { + bRet = FALSE; + } + else { x[i] = x[i] * REVERSE_SPHERE_RADIUS; if (x[i] > M_PI) @@ -2697,6 +2689,7 @@ int OGRProjCT::TransformWithErrorCodes(size_t nCount, double *x, double *y, const double yIn = y[i]; if (!std::isfinite(xIn)) { + bRet = FALSE; x[i] = HUGE_VAL; y[i] = HUGE_VAL; if (panErrorCodes) @@ -2725,6 +2718,7 @@ int OGRProjCT::TransformWithErrorCodes(size_t nCount, double *x, double *y, int err = 0; if (std::isnan(coord.xyzt.x)) { + bRet = FALSE; // This shouldn't normally happen if PROJ projections behave // correctly, but e.g inverse laea before PROJ 8.1.1 could // do that for points out of domain. @@ -2743,6 +2737,7 @@ int OGRProjCT::TransformWithErrorCodes(size_t nCount, double *x, double *y, } else if (coord.xyzt.x == HUGE_VAL) { + bRet = FALSE; err = proj_errno(pj); // PROJ should normally emit an error, but in case it does not // (e.g PROJ 6.3 with the +ortho projection), synthetize one @@ -2759,6 +2754,7 @@ int OGRProjCT::TransformWithErrorCodes(size_t nCount, double *x, double *y, if (fabs(coord.xyzt.x - xIn) > dfThreshold || fabs(coord.xyzt.y - yIn) > dfThreshold) { + bRet = FALSE; err = PROJ_ERR_COORD_TRANSFM_OUTSIDE_PROJECTION_DOMAIN; x[i] = HUGE_VAL; y[i] = HUGE_VAL; @@ -2936,7 +2932,7 @@ int OGRProjCT::TransformWithErrorCodes(size_t nCount, double *x, double *y, // static_cast(delay * 1000)); #endif - return TRUE; + return bRet; } /************************************************************************/ @@ -2944,39 +2940,42 @@ int OGRProjCT::TransformWithErrorCodes(size_t nCount, double *x, double *y, /************************************************************************/ // --------------------------------------------------------------------------- -static double simple_min(const double *data, const int arr_len) +static double simple_min(const double *data, const int *panErrorCodes, + const int arr_len) { - double min_value = data[0]; - for (int iii = 1; iii < arr_len; iii++) + double min_value = HUGE_VAL; + for (int iii = 0; iii < arr_len; iii++) { - if (data[iii] < min_value) + if ((data[iii] < min_value || min_value == HUGE_VAL) && + panErrorCodes[iii] == 0) min_value = data[iii]; } return min_value; } // --------------------------------------------------------------------------- -static double simple_max(const double *data, const int arr_len) +static double simple_max(const double *data, const int *panErrorCodes, + const int arr_len) { - double max_value = data[0]; - for (int iii = 1; iii < arr_len; iii++) + double max_value = HUGE_VAL; + for (int iii = 0; iii < arr_len; iii++) { if ((data[iii] > max_value || max_value == HUGE_VAL) && - data[iii] != HUGE_VAL) + panErrorCodes[iii] == 0) max_value = data[iii]; } return max_value; } // --------------------------------------------------------------------------- -static int _find_previous_index(const int iii, const double *data, +static int _find_previous_index(const int iii, const int *panErrorCodes, const int arr_len) { // find index of nearest valid previous value if exists int prev_iii = iii - 1; if (prev_iii == -1) // handle wraparound prev_iii = arr_len - 1; - while (data[prev_iii] == HUGE_VAL && prev_iii != iii) + while (panErrorCodes[prev_iii] != 0 && prev_iii != iii) { prev_iii--; if (prev_iii == -1) // handle wraparound @@ -3033,7 +3032,8 @@ but smalller than 240 to account for possible irregularities in distances when re-projecting. Also, 200 ensures latitudes are ignored for axis order handling. ******************************************************************************/ -static double antimeridian_min(const double *data, const int arr_len) +static double antimeridian_min(const double *data, const int *panErrorCodes, + const int arr_len) { double positive_min = HUGE_VAL; double min_value = HUGE_VAL; @@ -3042,9 +3042,9 @@ static double antimeridian_min(const double *data, const int arr_len) for (int iii = 0; iii < arr_len; iii++) { - if (data[iii] == HUGE_VAL) + if (panErrorCodes[iii]) continue; - int prev_iii = _find_previous_index(iii, data, arr_len); + int prev_iii = _find_previous_index(iii, panErrorCodes, arr_len); // check if crossed meridian double delta = data[prev_iii] - data[iii]; // 180 -> -180 @@ -3086,7 +3086,8 @@ static double antimeridian_min(const double *data, const int arr_len) // Note: This requires a densified ring with at least 2 additional // points per edge to correctly handle global extents. // See antimeridian_min docstring for reasoning. -static double antimeridian_max(const double *data, const int arr_len) +static double antimeridian_max(const double *data, const int *panErrorCodes, + const int arr_len) { double negative_max = -HUGE_VAL; double max_value = -HUGE_VAL; @@ -3095,9 +3096,9 @@ static double antimeridian_max(const double *data, const int arr_len) for (int iii = 0; iii < arr_len; iii++) { - if (data[iii] == HUGE_VAL) + if (panErrorCodes[iii]) continue; - int prev_iii = _find_previous_index(iii, data, arr_len); + int prev_iii = _find_previous_index(iii, panErrorCodes, arr_len); // check if crossed meridian double delta = data[prev_iii] - data[iii]; // 180 -> -180 @@ -3119,11 +3120,11 @@ static double antimeridian_max(const double *data, const int arr_len) // negative meridian side max if (negative_meridian && (data[iii] > negative_max || negative_max == HUGE_VAL) && - data[iii] != HUGE_VAL) + panErrorCodes[iii] == 0) negative_max = data[iii]; // track general max value if ((data[iii] > max_value || max_value == HUGE_VAL) && - data[iii] != HUGE_VAL) + panErrorCodes[iii] == 0) max_value = data[iii]; } if (crossed_meridian_count == 2) @@ -3149,17 +3150,14 @@ bool OGRProjCT::ContainsNorthPole(const double xmin, const double ymin, pole_y = 0; pole_x = 90; } - auto inverseCT = GetInverse(); + auto inverseCT = std::unique_ptr(GetInverse()); if (!inverseCT) return false; - bool success = inverseCT->TransformWithErrorCodes( + CPLErrorStateBackuper oBackuper(CPLQuietErrorHandler); + const bool success = inverseCT->TransformWithErrorCodes( 1, &pole_x, &pole_y, nullptr, nullptr, nullptr); - if (success && CPLGetLastErrorType() != CE_None) - CPLErrorReset(); - delete inverseCT; - if (xmin < pole_x && pole_x < xmax && ymax > pole_y && pole_y > ymin) - return true; - return false; + return success && xmin < pole_x && pole_x < xmax && ymax > pole_y && + pole_y > ymin; } // --------------------------------------------------------------------------- @@ -3177,17 +3175,14 @@ bool OGRProjCT::ContainsSouthPole(const double xmin, const double ymin, pole_y = 0; pole_x = -90; } - auto inverseCT = GetInverse(); + auto inverseCT = std::unique_ptr(GetInverse()); if (!inverseCT) return false; - bool success = inverseCT->TransformWithErrorCodes( + CPLErrorStateBackuper oBackuper(CPLQuietErrorHandler); + const bool success = inverseCT->TransformWithErrorCodes( 1, &pole_x, &pole_y, nullptr, nullptr, nullptr); - if (success && CPLGetLastErrorType() != CE_None) - CPLErrorReset(); - delete inverseCT; - if (xmin < pole_x && pole_x < xmax && ymax > pole_y && pole_y > ymin) - return true; - return false; + return success && xmin < pole_x && pole_x < xmax && ymax > pole_y && + pole_y > ymin; } int OGRProjCT::TransformBounds(const double xmin, const double ymin, @@ -3196,8 +3191,6 @@ int OGRProjCT::TransformBounds(const double xmin, const double ymin, double *out_xmax, double *out_ymax, const int densify_pts) { - CPLErrorReset(); - if (bNoTransform) { *out_xmin = xmin; @@ -3268,10 +3261,12 @@ int OGRProjCT::TransformBounds(const double xmin, const double ymin, const int boundary_len = side_pts * 4; std::vector x_boundary_array; std::vector y_boundary_array; + std::vector anErrorCodes; try { x_boundary_array.resize(boundary_len); y_boundary_array.resize(boundary_len); + anErrorCodes.resize(boundary_len); } catch (const std::exception &e) // memory allocation failure { @@ -3284,20 +3279,10 @@ int OGRProjCT::TransformBounds(const double xmin, const double ymin, bool south_pole_in_bounds = false; if (degree_output) { - CPLErrorHandlerPusher oErrorHandlerPusher(CPLQuietErrorHandler); - north_pole_in_bounds = ContainsNorthPole(xmin, ymin, xmax, ymax, output_lon_lat_order); - if (CPLGetLastErrorType() != CE_None) - { - return false; - } south_pole_in_bounds = ContainsSouthPole(xmin, ymin, xmax, ymax, output_lon_lat_order); - if (CPLGetLastErrorType() != CE_None) - { - return false; - } } if (degree_input && xmax < xmin) @@ -3350,26 +3335,35 @@ int OGRProjCT::TransformBounds(const double xmin, const double ymin, } { - CPLErrorHandlerPusher oErrorHandlerPusher(CPLQuietErrorHandler); + CPLErrorStateBackuper oBackuper(CPLQuietErrorHandler); bool success = TransformWithErrorCodes( boundary_len, &x_boundary_array[0], &y_boundary_array[0], nullptr, - nullptr, nullptr); - if (success && CPLGetLastErrorType() != CE_None) - { - CPLErrorReset(); - } - else if (!success) + nullptr, anErrorCodes.data()); + if (!success) { - return false; + for (int i = 0; i < boundary_len; ++i) + { + if (anErrorCodes[i] == 0) + { + success = true; + break; + } + } + if (!success) + return false; } } if (!degree_output) { - *out_xmin = simple_min(&x_boundary_array[0], boundary_len); - *out_xmax = simple_max(&x_boundary_array[0], boundary_len); - *out_ymin = simple_min(&y_boundary_array[0], boundary_len); - *out_ymax = simple_max(&y_boundary_array[0], boundary_len); + *out_xmin = + simple_min(&x_boundary_array[0], anErrorCodes.data(), boundary_len); + *out_xmax = + simple_max(&x_boundary_array[0], anErrorCodes.data(), boundary_len); + *out_ymin = + simple_min(&y_boundary_array[0], anErrorCodes.data(), boundary_len); + *out_ymax = + simple_max(&y_boundary_array[0], anErrorCodes.data(), boundary_len); if (poSRSTarget->IsProjected()) { @@ -3475,13 +3469,15 @@ int OGRProjCT::TransformBounds(const double xmin, const double ymin, else if (north_pole_in_bounds && output_lon_lat_order) { *out_xmin = -180; - *out_ymin = simple_min(&y_boundary_array[0], boundary_len); + *out_ymin = + simple_min(&y_boundary_array[0], anErrorCodes.data(), boundary_len); *out_xmax = 180; *out_ymax = 90; } else if (north_pole_in_bounds) { - *out_xmin = simple_min(&x_boundary_array[0], boundary_len); + *out_xmin = + simple_min(&x_boundary_array[0], anErrorCodes.data(), boundary_len); *out_ymin = -180; *out_xmax = 90; *out_ymax = 180; @@ -3491,28 +3487,38 @@ int OGRProjCT::TransformBounds(const double xmin, const double ymin, *out_xmin = -180; *out_ymin = -90; *out_xmax = 180; - *out_ymax = simple_max(&y_boundary_array[0], boundary_len); + *out_ymax = + simple_max(&y_boundary_array[0], anErrorCodes.data(), boundary_len); } else if (south_pole_in_bounds) { *out_xmin = -90; *out_ymin = -180; - *out_xmax = simple_max(&x_boundary_array[0], boundary_len); + *out_xmax = + simple_max(&x_boundary_array[0], anErrorCodes.data(), boundary_len); *out_ymax = 180; } else if (output_lon_lat_order) { - *out_xmin = antimeridian_min(&x_boundary_array[0], boundary_len); - *out_xmax = antimeridian_max(&x_boundary_array[0], boundary_len); - *out_ymin = simple_min(&y_boundary_array[0], boundary_len); - *out_ymax = simple_max(&y_boundary_array[0], boundary_len); + *out_xmin = antimeridian_min(&x_boundary_array[0], anErrorCodes.data(), + boundary_len); + *out_xmax = antimeridian_max(&x_boundary_array[0], anErrorCodes.data(), + boundary_len); + *out_ymin = + simple_min(&y_boundary_array[0], anErrorCodes.data(), boundary_len); + *out_ymax = + simple_max(&y_boundary_array[0], anErrorCodes.data(), boundary_len); } else { - *out_xmin = simple_min(&x_boundary_array[0], boundary_len); - *out_xmax = simple_max(&x_boundary_array[0], boundary_len); - *out_ymin = antimeridian_min(&y_boundary_array[0], boundary_len); - *out_ymax = antimeridian_max(&y_boundary_array[0], boundary_len); + *out_xmin = + simple_min(&x_boundary_array[0], anErrorCodes.data(), boundary_len); + *out_xmax = + simple_max(&x_boundary_array[0], anErrorCodes.data(), boundary_len); + *out_ymin = antimeridian_min(&y_boundary_array[0], anErrorCodes.data(), + boundary_len); + *out_ymax = antimeridian_max(&y_boundary_array[0], anErrorCodes.data(), + boundary_len); } return *out_xmin != HUGE_VAL && *out_ymin != HUGE_VAL && From 40b4d5ed68e68ab7662fb1fa50f8a8ea0c3ec2c4 Mon Sep 17 00:00:00 2001 From: Even Rouault Date: Sun, 9 Feb 2025 23:06:22 +0100 Subject: [PATCH 6/6] Add 'gdal vector sql', as standalone or part of 'gdal vector pipeline' --- apps/CMakeLists.txt | 1 + apps/gdalalg_abstract_pipeline.h | 15 + apps/gdalalg_vector.cpp | 2 + apps/gdalalg_vector_pipeline.cpp | 22 +- apps/gdalalg_vector_sql.cpp | 328 ++++++++++++++++++ apps/gdalalg_vector_sql.h | 63 ++++ autotest/utilities/test_gdalalg_vector_sql.py | 166 +++++++++ doc/source/conf.py | 7 + doc/source/programs/gdal_vector.rst | 2 + doc/source/programs/gdal_vector_pipeline.rst | 17 + doc/source/programs/gdal_vector_sql.rst | 134 +++++++ doc/source/programs/index.rst | 2 + gcore/gdalalgorithm.cpp | 64 +++- gcore/gdalalgorithm.h | 2 + ogr/ogrsf_frmts/generic/ogrlayerpool.cpp | 34 +- ogr/ogrsf_frmts/generic/ogrlayerpool.h | 5 + 16 files changed, 835 insertions(+), 29 deletions(-) create mode 100644 apps/gdalalg_vector_sql.cpp create mode 100644 apps/gdalalg_vector_sql.h create mode 100644 autotest/utilities/test_gdalalg_vector_sql.py create mode 100644 doc/source/programs/gdal_vector_sql.rst diff --git a/apps/CMakeLists.txt b/apps/CMakeLists.txt index 3b2c309994a4..9f48f3be08f1 100644 --- a/apps/CMakeLists.txt +++ b/apps/CMakeLists.txt @@ -31,6 +31,7 @@ add_library( gdalalg_vector_read.cpp gdalalg_vector_filter.cpp gdalalg_vector_reproject.cpp + gdalalg_vector_sql.cpp gdalalg_vector_write.cpp gdalinfo_lib.cpp gdalbuildvrt_lib.cpp diff --git a/apps/gdalalg_abstract_pipeline.h b/apps/gdalalg_abstract_pipeline.h index f2eb0589a8a4..5e198afbfe27 100644 --- a/apps/gdalalg_abstract_pipeline.h +++ b/apps/gdalalg_abstract_pipeline.h @@ -19,6 +19,8 @@ #include "cpl_json.h" #include "gdalalgorithm.h" +#include + template class GDALAbstractPipelineAlgorithm CPL_NON_FINAL : public StepAlgorithm { @@ -44,6 +46,19 @@ class GDALAbstractPipelineAlgorithm CPL_NON_FINAL : public StepAlgorithm { } + ~GDALAbstractPipelineAlgorithm() override + { + // Destroy steps in the reverse order they have been constructed, + // as a step can create object that depends on the validity of + // objects of previous steps, and while cleaning them it needs those + // prior objects to be still alive. + // Typically for "gdal vector pipeline read ... ! sql ..." + for (auto it = std::rbegin(m_steps); it != std::rend(m_steps); it++) + { + it->reset(); + } + } + virtual GDALArgDatasetValue &GetOutputDataset() = 0; std::string m_pipeline{}; diff --git a/apps/gdalalg_vector.cpp b/apps/gdalalg_vector.cpp index 75191f2c01ba..6445cd808577 100644 --- a/apps/gdalalg_vector.cpp +++ b/apps/gdalalg_vector.cpp @@ -18,6 +18,7 @@ #include "gdalalg_vector_pipeline.h" #include "gdalalg_vector_filter.h" #include "gdalalg_vector_reproject.h" +#include "gdalalg_vector_sql.h" /************************************************************************/ /* GDALVectorAlgorithm */ @@ -43,6 +44,7 @@ class GDALVectorAlgorithm final : public GDALAlgorithm RegisterSubAlgorithm(); RegisterSubAlgorithm(); RegisterSubAlgorithm(); + RegisterSubAlgorithm(); } private: diff --git a/apps/gdalalg_vector_pipeline.cpp b/apps/gdalalg_vector_pipeline.cpp index eba4c0ae10ab..82764aa8f934 100644 --- a/apps/gdalalg_vector_pipeline.cpp +++ b/apps/gdalalg_vector_pipeline.cpp @@ -15,6 +15,7 @@ #include "gdalalg_vector_clip.h" #include "gdalalg_vector_filter.h" #include "gdalalg_vector_reproject.h" +#include "gdalalg_vector_sql.h" #include "gdalalg_vector_write.h" #include "cpl_conv.h" @@ -60,9 +61,12 @@ void GDALVectorPipelineStepAlgorithm::AddInputArgs(bool hiddenForCLI) AddInputDatasetArg(&m_inputDataset, GDAL_OF_VECTOR, /* positionalAndRequired = */ !hiddenForCLI) .SetHiddenForCLI(hiddenForCLI); - AddArg("input-layer", 'l', _("Input layer name(s)"), &m_inputLayerNames) - .AddAlias("layer") - .SetHiddenForCLI(hiddenForCLI); + if (GetName() != "sql") + { + AddArg("input-layer", 'l', _("Input layer name(s)"), &m_inputLayerNames) + .AddAlias("layer") + .SetHiddenForCLI(hiddenForCLI); + } } /************************************************************************/ @@ -94,10 +98,13 @@ void GDALVectorPipelineStepAlgorithm::AddOutputArgs( &m_appendLayer) .SetDefault(false) .SetHiddenForCLI(hiddenForCLI); - AddArg("output-layer", shortNameOutputLayerAllowed ? 'l' : 0, - _("Output layer name"), &m_outputLayerName) - .AddHiddenAlias("nln") // For ogr2ogr nostalgic people - .SetHiddenForCLI(hiddenForCLI); + if (GetName() != "sql") + { + AddArg("output-layer", shortNameOutputLayerAllowed ? 'l' : 0, + _("Output layer name"), &m_outputLayerName) + .AddHiddenAlias("nln") // For ogr2ogr nostalgic people + .SetHiddenForCLI(hiddenForCLI); + } } /************************************************************************/ @@ -178,6 +185,7 @@ GDALVectorPipelineAlgorithm::GDALVectorPipelineAlgorithm() m_stepRegistry.Register(); m_stepRegistry.Register(); m_stepRegistry.Register(); + m_stepRegistry.Register(); } /************************************************************************/ diff --git a/apps/gdalalg_vector_sql.cpp b/apps/gdalalg_vector_sql.cpp new file mode 100644 index 000000000000..04cd5c1a9e25 --- /dev/null +++ b/apps/gdalalg_vector_sql.cpp @@ -0,0 +1,328 @@ +/****************************************************************************** + * + * Project: GDAL + * Purpose: "sql" step of "vector pipeline" + * Author: Even Rouault + * + ****************************************************************************** + * Copyright (c) 2025, Even Rouault + * + * SPDX-License-Identifier: MIT + ****************************************************************************/ + +#include "gdalalg_vector_sql.h" + +#include "gdal_priv.h" +#include "ogrsf_frmts.h" +#include "ogrlayerpool.h" + +#include + +//! @cond Doxygen_Suppress + +#ifndef _ +#define _(x) (x) +#endif + +/************************************************************************/ +/* GDALVectorSQLAlgorithm::GDALVectorSQLAlgorithm() */ +/************************************************************************/ + +GDALVectorSQLAlgorithm::GDALVectorSQLAlgorithm(bool standaloneStep) + : GDALVectorPipelineStepAlgorithm(NAME, DESCRIPTION, HELP_URL, + standaloneStep) +{ + AddArg("sql", 0, _("SQL statement(s)"), &m_sql) + .SetPositional() + .SetRequired() + .SetPackedValuesAllowed(false) + .SetReadFromFileAtSyntaxAllowed() + .SetMetaVar("|@") + .SetRemoveSQLCommentsEnabled(); + AddArg("output-layer", standaloneStep ? 0 : 'l', _("Output layer name(s)"), + &m_outputLayer); + AddArg("dialect", 0, _("SQL dialect (e.g. OGRSQL, SQLITE)"), &m_dialect); +} + +/************************************************************************/ +/* GDALVectorSQLAlgorithmDataset */ +/************************************************************************/ + +namespace +{ +class GDALVectorSQLAlgorithmDataset final : public GDALDataset +{ + GDALDataset &m_oSrcDS; + std::vector m_layers{}; + + CPL_DISALLOW_COPY_ASSIGN(GDALVectorSQLAlgorithmDataset) + + public: + explicit GDALVectorSQLAlgorithmDataset(GDALDataset &oSrcDS) + : m_oSrcDS(oSrcDS) + { + } + + ~GDALVectorSQLAlgorithmDataset() override + { + for (OGRLayer *poLayer : m_layers) + m_oSrcDS.ReleaseResultSet(poLayer); + } + + void AddLayer(OGRLayer *poLayer) + { + m_layers.push_back(poLayer); + } + + int GetLayerCount() override + { + return static_cast(m_layers.size()); + } + + OGRLayer *GetLayer(int idx) override + { + return idx >= 0 && idx < GetLayerCount() ? m_layers[idx] : nullptr; + } +}; +} // namespace + +/************************************************************************/ +/* GDALVectorSQLAlgorithmDatasetMultiLayer */ +/************************************************************************/ + +namespace +{ + +class ProxiedSQLLayer final : public OGRProxiedLayer +{ + OGRFeatureDefn *m_poLayerDefn = nullptr; + + CPL_DISALLOW_COPY_ASSIGN(ProxiedSQLLayer) + + public: + ProxiedSQLLayer(const std::string &osName, OGRLayerPool *poPoolIn, + OpenLayerFunc pfnOpenLayerIn, + ReleaseLayerFunc pfnReleaseLayerIn, + FreeUserDataFunc pfnFreeUserDataIn, void *pUserDataIn) + : OGRProxiedLayer(poPoolIn, pfnOpenLayerIn, pfnReleaseLayerIn, + pfnFreeUserDataIn, pUserDataIn) + { + SetDescription(osName.c_str()); + } + + ~ProxiedSQLLayer() + { + if (m_poLayerDefn) + m_poLayerDefn->Release(); + } + + const char *GetName() override + { + return GetDescription(); + } + + OGRFeatureDefn *GetLayerDefn() override + { + if (!m_poLayerDefn) + { + m_poLayerDefn = OGRProxiedLayer::GetLayerDefn()->Clone(); + m_poLayerDefn->SetName(GetDescription()); + } + return m_poLayerDefn; + } +}; + +class GDALVectorSQLAlgorithmDatasetMultiLayer final : public GDALDataset +{ + // We can't safely have 2 SQL layers active simultaneously on the same + // source dataset. So each time we access one, we must close the last + // active one. + OGRLayerPool m_oPool{1}; + GDALDataset &m_oSrcDS; + std::vector> m_layers{}; + + struct UserData + { + GDALDataset &oSrcDS; + std::string osSQL{}; + std::string osDialect{}; + std::string osLayerName{}; + + UserData(GDALDataset &oSrcDSIn, const std::string &osSQLIn, + const std::string &osDialectIn, + const std::string &osLayerNameIn) + : oSrcDS(oSrcDSIn), osSQL(osSQLIn), osDialect(osDialectIn), + osLayerName(osLayerNameIn) + { + } + CPL_DISALLOW_COPY_ASSIGN(UserData) + }; + + CPL_DISALLOW_COPY_ASSIGN(GDALVectorSQLAlgorithmDatasetMultiLayer) + + public: + explicit GDALVectorSQLAlgorithmDatasetMultiLayer(GDALDataset &oSrcDS) + : m_oSrcDS(oSrcDS) + { + } + + void AddLayer(const std::string &osSQL, const std::string &osDialect, + const std::string &osLayerName) + { + const auto OpenLayer = [](void *pUserDataIn) + { + UserData *pUserData = static_cast(pUserDataIn); + return pUserData->oSrcDS.ExecuteSQL( + pUserData->osSQL.c_str(), nullptr, + pUserData->osDialect.empty() ? nullptr + : pUserData->osDialect.c_str()); + }; + + const auto CloseLayer = [](OGRLayer *poLayer, void *pUserDataIn) + { + UserData *pUserData = static_cast(pUserDataIn); + pUserData->oSrcDS.ReleaseResultSet(poLayer); + }; + + const auto DeleteUserData = [](void *pUserDataIn) + { delete static_cast(pUserDataIn); }; + + auto pUserData = new UserData(m_oSrcDS, osSQL, osDialect, osLayerName); + auto poLayer = std::make_unique( + osLayerName, &m_oPool, OpenLayer, CloseLayer, DeleteUserData, + pUserData); + m_layers.push_back(std::move(poLayer)); + } + + int GetLayerCount() override + { + return static_cast(m_layers.size()); + } + + OGRLayer *GetLayer(int idx) override + { + return idx >= 0 && idx < GetLayerCount() ? m_layers[idx].get() + : nullptr; + } +}; +} // namespace + +/************************************************************************/ +/* GDALVectorSQLAlgorithm::RunStep() */ +/************************************************************************/ + +bool GDALVectorSQLAlgorithm::RunStep(GDALProgressFunc, void *) +{ + CPLAssert(m_inputDataset.GetDatasetRef()); + CPLAssert(m_outputDataset.GetName().empty()); + CPLAssert(!m_outputDataset.GetDatasetRef()); + + if (!m_outputLayer.empty() && m_outputLayer.size() != m_sql.size()) + { + ReportError(CE_Failure, CPLE_AppDefined, + "There should be as many layer names in --output-layer as " + "in --statement"); + return false; + } + + auto poSrcDS = m_inputDataset.GetDatasetRef(); + + if (m_sql.size() == 1) + { + auto outDS = std::make_unique(*poSrcDS); + outDS->SetDescription(poSrcDS->GetDescription()); + + const auto nErrorCounter = CPLGetErrorCounter(); + OGRLayer *poLayer = poSrcDS->ExecuteSQL( + m_sql[0].c_str(), nullptr, + m_dialect.empty() ? nullptr : m_dialect.c_str()); + if (!poLayer) + { + if (nErrorCounter == CPLGetErrorCounter()) + { + ReportError(CE_Failure, CPLE_AppDefined, + "Execution of the SQL statement '%s' did not " + "result in a result layer.", + m_sql[0].c_str()); + } + return false; + } + + if (!m_outputLayer.empty()) + { + const std::string &osLayerName = m_outputLayer[0]; + poLayer->GetLayerDefn()->SetName(osLayerName.c_str()); + poLayer->SetDescription(osLayerName.c_str()); + } + outDS->AddLayer(poLayer); + m_outputDataset.Set(std::move(outDS)); + } + else + { + // First pass to check all statements are valid and figure out layer + // names + std::set setOutputLayerNames; + std::vector aosLayerNames; + for (const std::string &sql : m_sql) + { + const auto nErrorCounter = CPLGetErrorCounter(); + auto poLayer = poSrcDS->ExecuteSQL( + sql.c_str(), nullptr, + m_dialect.empty() ? nullptr : m_dialect.c_str()); + if (!poLayer) + { + if (nErrorCounter == CPLGetErrorCounter()) + { + ReportError(CE_Failure, CPLE_AppDefined, + "Execution of the SQL statement '%s' did not " + "result in a result layer.", + sql.c_str()); + } + return false; + } + + std::string osLayerName; + + if (!m_outputLayer.empty()) + { + osLayerName = m_outputLayer[aosLayerNames.size()]; + } + else if (cpl::contains(setOutputLayerNames, + poLayer->GetDescription())) + { + int num = 1; + do + { + osLayerName = poLayer->GetDescription(); + ++num; + osLayerName += std::to_string(num); + } while (cpl::contains(setOutputLayerNames, osLayerName)); + } + + if (!osLayerName.empty()) + { + poLayer->GetLayerDefn()->SetName(osLayerName.c_str()); + poLayer->SetDescription(osLayerName.c_str()); + } + setOutputLayerNames.insert(poLayer->GetDescription()); + aosLayerNames.push_back(poLayer->GetDescription()); + + poSrcDS->ReleaseResultSet(poLayer); + } + + auto outDS = + std::make_unique(*poSrcDS); + outDS->SetDescription(poSrcDS->GetDescription()); + + for (size_t i = 0; i < aosLayerNames.size(); ++i) + { + outDS->AddLayer(m_sql[i], m_dialect, aosLayerNames[i]); + } + + m_outputDataset.Set(std::move(outDS)); + } + + return true; +} + +//! @endcond diff --git a/apps/gdalalg_vector_sql.h b/apps/gdalalg_vector_sql.h new file mode 100644 index 000000000000..2192a850ee96 --- /dev/null +++ b/apps/gdalalg_vector_sql.h @@ -0,0 +1,63 @@ +/****************************************************************************** + * + * Project: GDAL + * Purpose: "sql" step of "vector pipeline" + * Author: Even Rouault + * + ****************************************************************************** + * Copyright (c) 2025, Even Rouault + * + * SPDX-License-Identifier: MIT + ****************************************************************************/ + +#ifndef GDALALG_VECTOR_SQL_INCLUDED +#define GDALALG_VECTOR_SQL_INCLUDED + +#include "gdalalg_vector_pipeline.h" + +//! @cond Doxygen_Suppress + +/************************************************************************/ +/* GDALVectorSQLAlgorithm */ +/************************************************************************/ + +class GDALVectorSQLAlgorithm /* non final */ + : public GDALVectorPipelineStepAlgorithm +{ + public: + static constexpr const char *NAME = "sql"; + static constexpr const char *DESCRIPTION = + "Apply SQL statement(s) to a dataset."; + static constexpr const char *HELP_URL = "/programs/gdal_vector_sql.html"; + + static std::vector GetAliases() + { + return {}; + } + + explicit GDALVectorSQLAlgorithm(bool standaloneStep = false); + + private: + bool RunStep(GDALProgressFunc pfnProgress, void *pProgressData) override; + + std::vector m_sql{}; + std::vector m_outputLayer{}; + std::string m_dialect{}; +}; + +/************************************************************************/ +/* GDALVectorSQLAlgorithmStandalone */ +/************************************************************************/ + +class GDALVectorSQLAlgorithmStandalone final : public GDALVectorSQLAlgorithm +{ + public: + GDALVectorSQLAlgorithmStandalone() + : GDALVectorSQLAlgorithm(/* standaloneStep = */ true) + { + } +}; + +//! @endcond + +#endif /* GDALALG_VECTOR_SQL_INCLUDED */ diff --git a/autotest/utilities/test_gdalalg_vector_sql.py b/autotest/utilities/test_gdalalg_vector_sql.py new file mode 100644 index 000000000000..e9ca4db73a2a --- /dev/null +++ b/autotest/utilities/test_gdalalg_vector_sql.py @@ -0,0 +1,166 @@ +#!/usr/bin/env pytest +# -*- coding: utf-8 -*- +############################################################################### +# Project: GDAL/OGR Test Suite +# Purpose: 'gdal vector sql' testing +# Author: Even Rouault +# +############################################################################### +# Copyright (c) 2025, Even Rouault +# +# SPDX-License-Identifier: MIT +############################################################################### + +import pytest + +from osgeo import gdal + + +def get_sql_alg(): + reg = gdal.GetGlobalAlgorithmRegistry() + vector = reg.InstantiateAlg("vector") + return vector.InstantiateSubAlgorithm("sql") + + +def test_gdalalg_vector_sql_base(tmp_vsimem): + + out_filename = str(tmp_vsimem / "out.shp") + + sql_alg = get_sql_alg() + assert sql_alg.ParseRunAndFinalize( + ["../ogr/data/poly.shp", out_filename, "select * from poly limit 1"] + ) + + with gdal.OpenEx(out_filename) as ds: + assert ds.GetLayerCount() == 1 + assert ds.GetLayer(0).GetFeatureCount() == 1 + assert ds.GetLayer(-1) is None + assert ds.GetLayer(1) is None + + +def test_gdalalg_vector_sql_layer_name(tmp_vsimem): + + out_filename = str(tmp_vsimem / "out") + + sql_alg = get_sql_alg() + assert sql_alg.ParseRunAndFinalize( + [ + "--output-layer=foo", + "../ogr/data/poly.shp", + out_filename, + "select * from poly limit 1", + ] + ) + + with gdal.OpenEx(out_filename) as ds: + assert ds.GetLayer(0).GetFeatureCount() == 1 + assert ds.GetLayer(0).GetName() == "foo" + + +def test_gdalalg_vector_sql_error(tmp_vsimem): + + out_filename = str(tmp_vsimem / "out.shp") + + sql_alg = get_sql_alg() + with pytest.raises(Exception): + sql_alg.ParseRunAndFinalize(["../ogr/data/poly.shp", out_filename, "error"]) + + +def test_gdalalg_vector_sql_error_2_layers(tmp_vsimem): + + out_filename = str(tmp_vsimem / "out.shp") + + sql_alg = get_sql_alg() + with pytest.raises(Exception): + sql_alg.ParseRunAndFinalize( + ["../ogr/data/poly.shp", out_filename, "select * from poly", "error"] + ) + + +def test_gdalalg_vector_sql_layer_name_inconsistent_number(tmp_vsimem): + + out_filename = str(tmp_vsimem / "out") + + sql_alg = get_sql_alg() + with pytest.raises( + Exception, + match="sql: There should be as many layer names in --output-layer as in --statement", + ): + sql_alg.ParseRunAndFinalize( + [ + "--output-layer=foo,bar", + "../ogr/data/poly.shp", + out_filename, + "select * from poly limit 1", + ] + ) + + +def test_gdalalg_vector_sql_several(tmp_vsimem): + + out_filename = str(tmp_vsimem / "out") + + sql_alg = get_sql_alg() + assert sql_alg.ParseRunAndFinalize( + [ + "../ogr/data/poly.shp", + out_filename, + "select * from poly limit 1", + "select * from poly limit 2", + ] + ) + + with gdal.OpenEx(out_filename) as ds: + assert ds.GetLayerCount() == 2 + assert ds.GetLayer(-1) is None + assert ds.GetLayer(2) is None + assert ds.GetLayer(0).GetFeatureCount() == 1 + assert ds.GetLayer(0).GetDescription() == "poly" + assert ds.GetLayer(1).GetFeatureCount() == 2 + assert ds.GetLayer(1).GetDescription() == "poly2" + assert ds.GetLayer(0).GetFeatureCount() == 1 + assert ds.GetLayer(0).GetDescription() == "poly" + + +@pytest.mark.require_driver("SQLite") +def test_gdalalg_vector_sql_dialect(tmp_vsimem): + + out_filename = str(tmp_vsimem / "out.shp") + + sql_alg = get_sql_alg() + assert sql_alg.ParseRunAndFinalize( + [ + "--dialect", + "SQLite", + "../ogr/data/poly.shp", + out_filename, + "select *, sqlite_version() from poly limit 1", + ] + ) + + with gdal.OpenEx(out_filename) as ds: + assert ds.GetLayer(0).GetFeatureCount() == 1 + + +def test_gdalalg_vector_sql_layer_names(tmp_vsimem): + + out_filename = str(tmp_vsimem / "out") + + sql_alg = get_sql_alg() + assert sql_alg.ParseRunAndFinalize( + [ + "--output-layer", + "lyr1,lyr2", + "../ogr/data/poly.shp", + out_filename, + "select * from poly limit 1", + "select * from poly limit 2", + ] + ) + + with gdal.OpenEx(out_filename) as ds: + assert ds.GetLayerCount() == 2 + assert ds.GetLayer(0).GetFeatureCount() == 1 + assert ds.GetLayer(0).GetDescription() == "lyr1" + assert ds.GetLayer(1).GetFeatureCount() == 2 + assert ds.GetLayer(1).GetDescription() == "lyr2" diff --git a/doc/source/conf.py b/doc/source/conf.py index 952b1c723073..01247cffc559 100644 --- a/doc/source/conf.py +++ b/doc/source/conf.py @@ -320,6 +320,13 @@ [author_evenr], 1, ), + ( + "programs/gdal_vector_sql", + "gdal-vector-sql", + "Apply SQL statement(s) to a dataset", + [author_evenr], + 1, + ), # Traditional utilities ( "programs/gdalinfo", diff --git a/doc/source/programs/gdal_vector.rst b/doc/source/programs/gdal_vector.rst index 9b503db16b7a..1ba9017f8620 100644 --- a/doc/source/programs/gdal_vector.rst +++ b/doc/source/programs/gdal_vector.rst @@ -25,6 +25,7 @@ Synopsis - info: Return information on a vector dataset. - pipeline: Process a vector dataset. - reproject: Reproject a vector dataset. + - sql: Apply SQL statement(s) to a dataset. Available sub-commands ---------------------- @@ -33,6 +34,7 @@ Available sub-commands - :ref:`gdal_vector_convert_subcommand` - :ref:`gdal_vector_info_subcommand` - :ref:`gdal_vector_pipeline_subcommand` +- :ref:`gdal_vector_sql_subcommand` Examples -------- diff --git a/doc/source/programs/gdal_vector_pipeline.rst b/doc/source/programs/gdal_vector_pipeline.rst index 20d6b756fccd..a0eba8db1950 100644 --- a/doc/source/programs/gdal_vector_pipeline.rst +++ b/doc/source/programs/gdal_vector_pipeline.rst @@ -93,6 +93,23 @@ Details for options can be found in :ref:`gdal_vector_clip_subcommand`. -s, --src-crs Source CRS -d, --dst-crs Destination CRS [required] +* sql [OPTIONS] + +.. code-block:: + + Apply SQL statement(s) to a dataset. + + Positional arguments: + --sql |@ SQL statement(s) [may be repeated] [required] + + Options: + -l, --output-layer Output layer name(s) [may be repeated] + --dialect SQL dialect (e.g. OGRSQL, SQLITE) + + +Details for options can be found in :ref:`gdal_vector_sql_subcommand`. + + * write [OPTIONS] .. code-block:: diff --git a/doc/source/programs/gdal_vector_sql.rst b/doc/source/programs/gdal_vector_sql.rst new file mode 100644 index 000000000000..dbd357422dde --- /dev/null +++ b/doc/source/programs/gdal_vector_sql.rst @@ -0,0 +1,134 @@ +.. _gdal_vector_sql_subcommand: + +================================================================================ +"gdal vector sql" sub-command +================================================================================ + +.. versionadded:: 3.11 + +.. only:: html + + Apply SQL statement(s) to a dataset. + +.. Index:: gdal vector sql + +Synopsis +-------- + +.. code-block:: + + Usage: gdal vector sql [OPTIONS] |@ + + Apply SQL statement(s) to a dataset. + + Positional arguments: + -i, --input Input vector dataset [required] + -o, --output Output vector dataset [required] + --sql |@ SQL statement(s) [may be repeated] [required] + + Common Options: + -h, --help Display help message and exit + --version Display GDAL version and exit + --json-usage Display usage as JSON document and exit + --drivers Display driver list as JSON document and exit + --config = Configuration option [may be repeated] + --progress Display progress bar + + Options: + -f, --of, --format, --output-format Output format + --co, --creation-option = Creation option [may be repeated] + --lco, --layer-creation-option = Layer creation option [may be repeated] + --overwrite Whether overwriting existing output is allowed + --update Whether to open existing dataset in update mode + --overwrite-layer Whether overwriting existing layer is allowed + --append Whether appending to existing layer is allowed + --output-layer Output layer name(s) [may be repeated] + --dialect SQL dialect (e.g. OGRSQL, SQLITE) + + Advanced Options: + --if, --input-format Input formats [may be repeated] + --oo, --open-option Open options [may be repeated] + + +Description +----------- + +:program:`gdal vector sql` returns one or several layers evaluated from +SQL statements. + +Standard options +++++++++++++++++ + +.. include:: gdal_options/of_vector.rst + +.. include:: gdal_options/co_vector.rst + +.. include:: gdal_options/overwrite.rst + +.. option:: --sql |@ + + SQL statement to execute that returns a table/layer (typically a SELECT + statement). + + Can be repeated to generated multiple output layers (repeating --sql + for each output layer) + +.. option:: --dialect + + SQL dialect. + + By default the native SQL of an RDBMS is used when using + ``gdal vector sql``. If using ``sql`` as a step of ``gdal vector pipeline``, + this is only true if the step preceding ``sql`` is ``read``, otherwise the + :ref:`OGRSQL ` dialect is used. + + If a datasource does not support SQL natively, the default is to use the + ``OGRSQL`` dialect, which can also be specified with any data source. + + The :ref:`sql_sqlite_dialect` dialect can be chosen with the ``SQLITE`` + and ``INDIRECT_SQLITE`` dialect values, and this can be used with any data source. + Overriding the default dialect may be beneficial because the capabilities of + the SQL dialects vary. What SQL dialects a driver supports can be checked + with "gdal vector info". + + .. code-block:: + + $ gdal vector info --format "PostgreSQL" + Supported SQL dialects: NATIVE OGRSQL SQLITE + + $ gdal vector info --format "ESRI Shapefile" + Supported SQL dialects: OGRSQL SQLITE + + +.. option:: --output-layer + + Output SQL layer name(s). If not specified, a generic layer name such as + "SELECT" may be generated. + + Must be specified as many times as there are SQL statements, either as + several --output-layer arguments, or a single one with the layer names + combined with comma. + +Advanced options +++++++++++++++++ + +.. include:: gdal_options/oo.rst + +.. include:: gdal_options/if.rst + +Examples +-------- + +.. example:: + :title: Generate a GeoPackage file with a layer sorted by descending population + + .. code-block:: bash + + $ gdal vector sql in.gpkg out.gpkg --output-layer country_sorted_by_pop --sql="SELECT * FROM country ORDER BY pop DESC" + +.. example:: + :title: Generate a GeoPackage file with 2 SQL result layers + + .. code-block:: bash + + $ gdal vector sql in.gpkg out.gpkg --output-layer=beginning,end --sql="SELECT * FROM my_layer LIMIT 100" --sql="SELECT * FROM my_layer OFFSET 100000 LIMIT 100" diff --git a/doc/source/programs/index.rst b/doc/source/programs/index.rst index 95477fda0777..a9e8c930ff1d 100644 --- a/doc/source/programs/index.rst +++ b/doc/source/programs/index.rst @@ -46,6 +46,7 @@ single :program:`gdal` program that accepts commands and subcommands. gdal_vector_clip gdal_vector_convert gdal_vector_pipeline + gdal_vector_sql .. only:: html @@ -70,6 +71,7 @@ single :program:`gdal` program that accepts commands and subcommands. - :ref:`gdal_vector_clip_subcommand`: Clip a vector dataset - :ref:`gdal_vector_convert_subcommand`: Convert a vector dataset - :ref:`gdal_vector_pipeline_subcommand`: Process a vector dataset + - :ref:`gdal_vector_sql_subcommand`: Apply SQL statement(s) to a dataset "Traditional" applications diff --git a/gcore/gdalalgorithm.cpp b/gcore/gdalalgorithm.cpp index 26fd554e733c..4695cd7031f4 100644 --- a/gcore/gdalalgorithm.cpp +++ b/gcore/gdalalgorithm.cpp @@ -231,18 +231,8 @@ bool GDALAlgorithmArg::Set(bool value) return SetInternal(value); } -bool GDALAlgorithmArg::Set(const std::string &value) +bool GDALAlgorithmArg::ProcessString(std::string &value) const { - if (m_decl.GetType() != GAAT_STRING) - { - CPLError(CE_Failure, CPLE_AppDefined, - "Calling Set(std::string) on argument '%s' of type %s is not " - "supported", - GetName().c_str(), GDALAlgorithmArgTypeName(m_decl.GetType())); - return false; - } - - std::string newValue(value); if (m_decl.IsReadFromFileAtSyntaxAllowed() && !value.empty() && value.front() == '@') { @@ -257,7 +247,7 @@ bool GDALAlgorithmArg::Set(const std::string &value) { offset = 3; } - newValue = reinterpret_cast(pabyData + offset); + value = reinterpret_cast(pabyData + offset); VSIFree(pabyData); } else @@ -267,9 +257,24 @@ bool GDALAlgorithmArg::Set(const std::string &value) } if (m_decl.IsRemoveSQLCommentsEnabled()) - newValue = CPLRemoveSQLComments(newValue); + value = CPLRemoveSQLComments(value); + + return true; +} + +bool GDALAlgorithmArg::Set(const std::string &value) +{ + if (m_decl.GetType() != GAAT_STRING) + { + CPLError(CE_Failure, CPLE_AppDefined, + "Calling Set(std::string) on argument '%s' of type %s is not " + "supported", + GetName().c_str(), GDALAlgorithmArgTypeName(m_decl.GetType())); + return false; + } - return SetInternal(newValue); + std::string newValue(value); + return ProcessString(newValue) && SetInternal(newValue); } bool GDALAlgorithmArg::Set(int value) @@ -392,7 +397,22 @@ bool GDALAlgorithmArg::Set(const std::vector &value) GetName().c_str(), GDALAlgorithmArgTypeName(m_decl.GetType())); return false; } - return SetInternal(value); + + if (m_decl.IsReadFromFileAtSyntaxAllowed() || + m_decl.IsRemoveSQLCommentsEnabled()) + { + std::vector newValue(value); + for (auto &s : newValue) + { + if (!ProcessString(s)) + return false; + } + return SetInternal(newValue); + } + else + { + return SetInternal(value); + } } bool GDALAlgorithmArg::Set(const std::vector &value) @@ -2972,9 +2992,17 @@ GDALAlgorithm::GetUsageForCLI(bool shortUsage, osRet += " [OPTIONS]"; for (const auto *arg : m_positionalArgs) { - osRet += " <"; - osRet += arg->GetMetaVar(); - osRet += '>'; + const std::string &metavar = arg->GetMetaVar(); + if (!metavar.empty() && metavar[0] == '<') + { + osRet += metavar; + } + else + { + osRet += " <"; + osRet += metavar; + osRet += '>'; + } } } diff --git a/gcore/gdalalgorithm.h b/gcore/gdalalgorithm.h index 3823c8cced4c..3c4dbf8255c7 100644 --- a/gcore/gdalalgorithm.h +++ b/gcore/gdalalgorithm.h @@ -1447,6 +1447,8 @@ class CPL_DLL GDALAlgorithmArg /* non-final */ return RunAllActions(); } + bool ProcessString(std::string &value) const; + bool RunAllActions(); void RunActions(); bool RunValidationActions(); diff --git a/ogr/ogrsf_frmts/generic/ogrlayerpool.cpp b/ogr/ogrsf_frmts/generic/ogrlayerpool.cpp index d3166667e685..58a6ef90fa47 100644 --- a/ogr/ogrsf_frmts/generic/ogrlayerpool.cpp +++ b/ogr/ogrsf_frmts/generic/ogrlayerpool.cpp @@ -131,13 +131,36 @@ void OGRLayerPool::UnchainLayer(OGRAbstractProxiedLayer *poLayer) /* OGRProxiedLayer() */ /************************************************************************/ +static void ReleaseDelete(OGRLayer *poLayer, void *) +{ + delete poLayer; +} + +OGRProxiedLayer::OGRProxiedLayer(OGRLayerPool *poPoolIn, + OpenLayerFunc pfnOpenLayerIn, + FreeUserDataFunc pfnFreeUserDataIn, + void *pUserDataIn) + : OGRAbstractProxiedLayer(poPoolIn), pfnOpenLayer(pfnOpenLayerIn), + pfnReleaseLayer(ReleaseDelete), pfnFreeUserData(pfnFreeUserDataIn), + pUserData(pUserDataIn), poUnderlyingLayer(nullptr), + poFeatureDefn(nullptr), poSRS(nullptr) +{ + CPLAssert(pfnOpenLayerIn != nullptr); +} + +/************************************************************************/ +/* OGRProxiedLayer() */ +/************************************************************************/ + OGRProxiedLayer::OGRProxiedLayer(OGRLayerPool *poPoolIn, OpenLayerFunc pfnOpenLayerIn, + ReleaseLayerFunc pfnReleaseLayerIn, FreeUserDataFunc pfnFreeUserDataIn, void *pUserDataIn) : OGRAbstractProxiedLayer(poPoolIn), pfnOpenLayer(pfnOpenLayerIn), - pfnFreeUserData(pfnFreeUserDataIn), pUserData(pUserDataIn), - poUnderlyingLayer(nullptr), poFeatureDefn(nullptr), poSRS(nullptr) + pfnReleaseLayer(pfnReleaseLayerIn), pfnFreeUserData(pfnFreeUserDataIn), + pUserData(pUserDataIn), poUnderlyingLayer(nullptr), + poFeatureDefn(nullptr), poSRS(nullptr) { CPLAssert(pfnOpenLayerIn != nullptr); } @@ -148,7 +171,7 @@ OGRProxiedLayer::OGRProxiedLayer(OGRLayerPool *poPoolIn, OGRProxiedLayer::~OGRProxiedLayer() { - delete poUnderlyingLayer; + OGRProxiedLayer::CloseUnderlyingLayer(); if (poSRS) poSRS->Release(); @@ -184,7 +207,10 @@ int OGRProxiedLayer::OpenUnderlyingLayer() void OGRProxiedLayer::CloseUnderlyingLayer() { CPLDebug("OGR", "CloseUnderlyingLayer(%p)", this); - delete poUnderlyingLayer; + if (poUnderlyingLayer) + { + pfnReleaseLayer(poUnderlyingLayer, pUserData); + } poUnderlyingLayer = nullptr; } diff --git a/ogr/ogrsf_frmts/generic/ogrlayerpool.h b/ogr/ogrsf_frmts/generic/ogrlayerpool.h index e37ce8ca1112..a5aef18d366d 100644 --- a/ogr/ogrsf_frmts/generic/ogrlayerpool.h +++ b/ogr/ogrsf_frmts/generic/ogrlayerpool.h @@ -18,6 +18,7 @@ #include "ogrsf_frmts.h" typedef OGRLayer *(*OpenLayerFunc)(void *user_data); +typedef void (*ReleaseLayerFunc)(OGRLayer *, void *user_data); typedef void (*FreeUserDataFunc)(void *user_data); class OGRLayerPool; @@ -89,6 +90,7 @@ class CPL_DLL OGRProxiedLayer : public OGRAbstractProxiedLayer CPL_DISALLOW_COPY_ASSIGN(OGRProxiedLayer) OpenLayerFunc pfnOpenLayer; + ReleaseLayerFunc pfnReleaseLayer; FreeUserDataFunc pfnFreeUserData; void *pUserData; OGRLayer *poUnderlyingLayer; @@ -103,6 +105,9 @@ class CPL_DLL OGRProxiedLayer : public OGRAbstractProxiedLayer public: OGRProxiedLayer(OGRLayerPool *poPool, OpenLayerFunc pfnOpenLayer, FreeUserDataFunc pfnFreeUserData, void *pUserData); + OGRProxiedLayer(OGRLayerPool *poPool, OpenLayerFunc pfnOpenLayer, + ReleaseLayerFunc pfnReleaseLayer, + FreeUserDataFunc pfnFreeUserData, void *pUserData); virtual ~OGRProxiedLayer(); OGRLayer *GetUnderlyingLayer();