Merge pull request #10815 from rouault/fix_qgis_58086

Parquet: fix crash when using SetIgnoredFields() + SetSpatialFilter()…
OSGeo · Sep 18, 2024 · e2a3b9f · e2a3b9f
2 parents 8ffe534 + 37777a3
commit e2a3b9f
Show file tree

Hide file tree

Showing 3 changed files with 61 additions and 57 deletions.
diff --git a/autotest/ogr/ogr_parquet.py b/autotest/ogr/ogr_parquet.py
@@ -3870,6 +3870,8 @@ def check(lyr):
     lyr = ds.GetLayer(0)
     lyr.SetIgnoredFields(["foo"])
     check(lyr)
+    lyr.SetSpatialFilter(geom)
+    assert lyr.GetFeatureCount() == (3 if geom.GetGeometryCount() > 1 else 2)
 
     ds = ogr.Open(filename_to_open)
     lyr = ds.GetLayer(0)

diff --git a/ogr/ogrsf_frmts/arrow_common/ograrrowlayer.hpp b/ogr/ogrsf_frmts/arrow_common/ograrrowlayer.hpp
@@ -4013,62 +4013,65 @@ OGRArrowLayer::SetBatch(const std::shared_ptr<arrow::RecordBatch> &poBatch)
             {
                 const int idx = m_bIgnoredFields ? oIter->second.iArrayIdx
                                                  : oIter->second.iArrowCol;
-                CPLAssert(idx >= 0);
-                CPLAssert(static_cast<size_t>(idx) < m_poBatchColumns.size());
-                m_poArrayBBOX = m_poBatchColumns[idx].get();
-                CPLAssert(m_poArrayBBOX->type_id() == arrow::Type::STRUCT);
-                const auto castArray =
-                    static_cast<const arrow::StructArray *>(m_poArrayBBOX);
-                const auto &subArrays = castArray->fields();
-                CPLAssert(
-                    static_cast<size_t>(oIter->second.iArrowSubfieldXMin) <
-                    subArrays.size());
-                const auto xminArray =
-                    subArrays[oIter->second.iArrowSubfieldXMin].get();
-                CPLAssert(
-                    static_cast<size_t>(oIter->second.iArrowSubfieldYMin) <
-                    subArrays.size());
-                const auto yminArray =
-                    subArrays[oIter->second.iArrowSubfieldYMin].get();
-                CPLAssert(
-                    static_cast<size_t>(oIter->second.iArrowSubfieldXMax) <
-                    subArrays.size());
-                const auto xmaxArray =
-                    subArrays[oIter->second.iArrowSubfieldXMax].get();
-                CPLAssert(
-                    static_cast<size_t>(oIter->second.iArrowSubfieldYMax) <
-                    subArrays.size());
-                const auto ymaxArray =
-                    subArrays[oIter->second.iArrowSubfieldYMax].get();
-                if (oIter->second.bIsFloat)
+                if (idx >= 0)
                 {
-                    CPLAssert(xminArray->type_id() == arrow::Type::FLOAT);
-                    m_poArrayXMinFloat =
-                        static_cast<const arrow::FloatArray *>(xminArray);
-                    CPLAssert(yminArray->type_id() == arrow::Type::FLOAT);
-                    m_poArrayYMinFloat =
-                        static_cast<const arrow::FloatArray *>(yminArray);
-                    CPLAssert(xmaxArray->type_id() == arrow::Type::FLOAT);
-                    m_poArrayXMaxFloat =
-                        static_cast<const arrow::FloatArray *>(xmaxArray);
-                    CPLAssert(ymaxArray->type_id() == arrow::Type::FLOAT);
-                    m_poArrayYMaxFloat =
-                        static_cast<const arrow::FloatArray *>(ymaxArray);
-                }
-                else
-                {
-                    CPLAssert(xminArray->type_id() == arrow::Type::DOUBLE);
-                    m_poArrayXMinDouble =
-                        static_cast<const arrow::DoubleArray *>(xminArray);
-                    CPLAssert(yminArray->type_id() == arrow::Type::DOUBLE);
-                    m_poArrayYMinDouble =
-                        static_cast<const arrow::DoubleArray *>(yminArray);
-                    CPLAssert(xmaxArray->type_id() == arrow::Type::DOUBLE);
-                    m_poArrayXMaxDouble =
-                        static_cast<const arrow::DoubleArray *>(xmaxArray);
-                    CPLAssert(ymaxArray->type_id() == arrow::Type::DOUBLE);
-                    m_poArrayYMaxDouble =
-                        static_cast<const arrow::DoubleArray *>(ymaxArray);
+                    CPLAssert(static_cast<size_t>(idx) <
+                              m_poBatchColumns.size());
+                    m_poArrayBBOX = m_poBatchColumns[idx].get();
+                    CPLAssert(m_poArrayBBOX->type_id() == arrow::Type::STRUCT);
+                    const auto castArray =
+                        static_cast<const arrow::StructArray *>(m_poArrayBBOX);
+                    const auto &subArrays = castArray->fields();
+                    CPLAssert(
+                        static_cast<size_t>(oIter->second.iArrowSubfieldXMin) <
+                        subArrays.size());
+                    const auto xminArray =
+                        subArrays[oIter->second.iArrowSubfieldXMin].get();
+                    CPLAssert(
+                        static_cast<size_t>(oIter->second.iArrowSubfieldYMin) <
+                        subArrays.size());
+                    const auto yminArray =
+                        subArrays[oIter->second.iArrowSubfieldYMin].get();
+                    CPLAssert(
+                        static_cast<size_t>(oIter->second.iArrowSubfieldXMax) <
+                        subArrays.size());
+                    const auto xmaxArray =
+                        subArrays[oIter->second.iArrowSubfieldXMax].get();
+                    CPLAssert(
+                        static_cast<size_t>(oIter->second.iArrowSubfieldYMax) <
+                        subArrays.size());
+                    const auto ymaxArray =
+                        subArrays[oIter->second.iArrowSubfieldYMax].get();
+                    if (oIter->second.bIsFloat)
+                    {
+                        CPLAssert(xminArray->type_id() == arrow::Type::FLOAT);
+                        m_poArrayXMinFloat =
+                            static_cast<const arrow::FloatArray *>(xminArray);
+                        CPLAssert(yminArray->type_id() == arrow::Type::FLOAT);
+                        m_poArrayYMinFloat =
+                            static_cast<const arrow::FloatArray *>(yminArray);
+                        CPLAssert(xmaxArray->type_id() == arrow::Type::FLOAT);
+                        m_poArrayXMaxFloat =
+                            static_cast<const arrow::FloatArray *>(xmaxArray);
+                        CPLAssert(ymaxArray->type_id() == arrow::Type::FLOAT);
+                        m_poArrayYMaxFloat =
+                            static_cast<const arrow::FloatArray *>(ymaxArray);
+                    }
+                    else
+                    {
+                        CPLAssert(xminArray->type_id() == arrow::Type::DOUBLE);
+                        m_poArrayXMinDouble =
+                            static_cast<const arrow::DoubleArray *>(xminArray);
+                        CPLAssert(yminArray->type_id() == arrow::Type::DOUBLE);
+                        m_poArrayYMinDouble =
+                            static_cast<const arrow::DoubleArray *>(yminArray);
+                        CPLAssert(xmaxArray->type_id() == arrow::Type::DOUBLE);
+                        m_poArrayXMaxDouble =
+                            static_cast<const arrow::DoubleArray *>(xmaxArray);
+                        CPLAssert(ymaxArray->type_id() == arrow::Type::DOUBLE);
+                        m_poArrayYMaxDouble =
+                            static_cast<const arrow::DoubleArray *>(ymaxArray);
+                    }
                 }
             }
         }

diff --git a/ogr/ogrsf_frmts/parquet/ogrparquetlayer.cpp b/ogr/ogrsf_frmts/parquet/ogrparquetlayer.cpp
@@ -1980,8 +1980,7 @@ OGRErr OGRParquetLayer::SetIgnoredFields(CSLConstList papszFields)
                         m_oMapGeomFieldIndexToGeomColBBOXParquet.find(i);
                     if (oIter != m_oMapGeomFieldIndexToGeomColBBOX.end() &&
                         oIterParquet !=
-                            m_oMapGeomFieldIndexToGeomColBBOXParquet.end() &&
-                        !OGRArrowIsGeoArrowStruct(m_aeGeomEncoding[i]))
+                            m_oMapGeomFieldIndexToGeomColBBOXParquet.end())
                     {
                         oIter->second.iArrayIdx = nBatchColumns++;
                         m_anRequestedParquetColumns.insert(