From 7fec8c7aa4d5e6e6556b3dd4ee17f2b918eedb3a Mon Sep 17 00:00:00 2001
From: Even Rouault <even.rouault@spatialys.com>
Date: Sat, 23 Nov 2024 16:30:45 +0100
Subject: [PATCH] OCI: add a TIMESTAMP_WITH_TIME_ZONE layer creation option,
 and ogr2ogr tweaks

Fixes https://github.com/OSGeo/gdal/issues/11057#issuecomment-2495479779
---
 apps/ogr2ogr_lib.cpp                        | 26 ++++++++--
 autotest/ogr/ogr_oci.py                     | 50 +++++++++++++++++-
 doc/source/drivers/vector/oci.rst           | 11 ++++
 ogr/ogrsf_frmts/oci/ogr_oci.h               |  3 ++
 ogr/ogrsf_frmts/oci/ogrocidrivercore.cpp    |  3 ++
 ogr/ogrsf_frmts/oci/ogrocisession.cpp       |  7 ++-
 ogr/ogrsf_frmts/oci/ogrocitablelayer.cpp    | 57 +++++++++++++++++++++
 ogr/ogrsf_frmts/oci/ogrociwritablelayer.cpp | 15 +++++-
 8 files changed, 166 insertions(+), 6 deletions(-)

diff --git a/apps/ogr2ogr_lib.cpp b/apps/ogr2ogr_lib.cpp
index 19734944335d..6b8543d18590 100644
--- a/apps/ogr2ogr_lib.cpp
+++ b/apps/ogr2ogr_lib.cpp
@@ -4625,6 +4625,8 @@ SetupTargetLayer::Setup(OGRLayer *poSrcLayer, const char *pszNewLayerName,
             oCoordPrec.dfMResolution = psOptions->dfMRes;
         }
 
+        auto poSrcDriver = m_poSrcDS->GetDriver();
+
         // Force FID column as 64 bit if the source feature has a 64 bit FID,
         // the target driver supports 64 bit FID and the user didn't set it
         // manually.
@@ -4667,9 +4669,8 @@ SetupTargetLayer::Setup(OGRLayer *poSrcLayer, const char *pszNewLayerName,
         }
         // Detect scenario of converting from GPX to a format like GPKG
         // Cf https://github.com/OSGeo/gdal/issues/9225
-        else if (!bPreserveFID && !m_bUnsetFid && !bAppend &&
-                 m_poSrcDS->GetDriver() &&
-                 EQUAL(m_poSrcDS->GetDriver()->GetDescription(), "GPX") &&
+        else if (!bPreserveFID && !m_bUnsetFid && !bAppend && poSrcDriver &&
+                 EQUAL(poSrcDriver->GetDescription(), "GPX") &&
                  pszDestCreationOptions &&
                  (strstr(pszDestCreationOptions, "='FID'") != nullptr ||
                   strstr(pszDestCreationOptions, "=\"FID\"") != nullptr) &&
@@ -4761,6 +4762,23 @@ SetupTargetLayer::Setup(OGRLayer *poSrcLayer, const char *pszNewLayerName,
             }
         }
 
+        // Use case of https://github.com/OSGeo/gdal/issues/11057#issuecomment-2495479779
+        // Conversion from GPKG to OCI.
+        // OCI distinguishes between TIMESTAMP and TIMESTAMP WITH TIME ZONE
+        // GeoPackage is supposed to have DateTime in UTC, so we set
+        // TIMESTAMP_WITH_TIME_ZONE=YES
+        if (poSrcDriver && pszDestCreationOptions &&
+            strstr(pszDestCreationOptions, "TIMESTAMP_WITH_TIME_ZONE") &&
+            CSLFetchNameValue(m_papszLCO, "TIMESTAMP_WITH_TIME_ZONE") ==
+                nullptr &&
+            EQUAL(poSrcDriver->GetDescription(), "GPKG"))
+        {
+            papszLCOTemp = CSLSetNameValue(papszLCOTemp,
+                                           "TIMESTAMP_WITH_TIME_ZONE", "YES");
+            CPLDebug("GDALVectorTranslate",
+                     "Setting TIMESTAMP_WITH_TIME_ZONE=YES");
+        }
+
         OGRGeomFieldDefn oGeomFieldDefn(
             osGeomFieldName.c_str(),
             static_cast<OGRwkbGeometryType>(eGCreateLayerType));
@@ -4956,7 +4974,9 @@ SetupTargetLayer::Setup(OGRLayer *poSrcLayer, const char *pszNewLayerName,
 
     bool bError = false;
     OGRArrowArrayStream streamSrc;
+
     const bool bUseWriteArrowBatch =
+        !EQUAL(m_poDstDS->GetDriver()->GetDescription(), "OCI") &&
         CanUseWriteArrowBatch(poSrcLayer, poDstLayer, bJustCreatedLayer,
                               psOptions, bPreserveFID, bError, streamSrc);
     if (bError)
diff --git a/autotest/ogr/ogr_oci.py b/autotest/ogr/ogr_oci.py
index 02e3683cbea4..b213ae4a7038 100755
--- a/autotest/ogr/ogr_oci.py
+++ b/autotest/ogr/ogr_oci.py
@@ -70,6 +70,7 @@ def setup_tests():
     gdaltest.oci_ds.ExecuteSQL("DELLAYER:test_GEOMETRYCOLLECTION")
     gdaltest.oci_ds.ExecuteSQL("DELLAYER:test_NONE")
     gdaltest.oci_ds.ExecuteSQL("DELLAYER:testdate")
+    gdaltest.oci_ds.ExecuteSQL("DELLAYER:testdate_with_tz")
     gdaltest.oci_ds.ExecuteSQL("DELLAYER:ogr_oci_20")
     gdaltest.oci_ds.ExecuteSQL("DELLAYER:ogr_oci_20bis")
     gdaltest.oci_ds.ExecuteSQL("DELLAYER:ogr_oci_21")
@@ -724,7 +725,7 @@ def test_ogr_oci_18():
 # Test date / datetime
 
 
-def test_ogr_oci_19():
+def test_ogr_oci_datetime_no_tz():
 
     lyr = gdaltest.oci_ds.CreateLayer("testdate", geom_type=ogr.wkbNone)
     lyr.CreateField(ogr.FieldDefn("MYDATE", ogr.OFTDate))
@@ -736,6 +737,9 @@ def test_ogr_oci_19():
     feat = ogr.Feature(lyr.GetLayerDefn())
     feat.SetField("MYDATETIME", "2015/02/03 11:33:44.12345")
     lyr.CreateFeature(feat)
+    feat = ogr.Feature(lyr.GetLayerDefn())
+    feat.SetField("MYDATETIME", "2015/02/03 11:33:44.12345+0530")  # Timezone lost
+    lyr.CreateFeature(feat)
     lyr.SyncToDisk()
 
     with gdaltest.oci_ds.ExecuteSQL(
@@ -743,11 +747,55 @@ def test_ogr_oci_19():
     ) as sql_lyr:
         assert sql_lyr.GetLayerDefn().GetFieldDefn(0).GetType() == ogr.OFTDate
         assert sql_lyr.GetLayerDefn().GetFieldDefn(1).GetType() == ogr.OFTDateTime
+        assert sql_lyr.GetLayerDefn().GetFieldDefn(1).GetTZFlag() == ogr.TZFLAG_UNKNOWN
         f = sql_lyr.GetNextFeature()
         assert f.GetField(0) == "2015/02/03"
         assert f.GetField(1) == "2015/02/03 11:33:44"
         f = sql_lyr.GetNextFeature()
         assert f.GetField(1) == "2015/02/03 11:33:44.123"
+        f = sql_lyr.GetNextFeature()
+        assert f.GetField(1) == "2015/02/03 11:33:44.123"  # Timezone lost
+
+
+###############################################################################
+# Test date / datetime with time zone
+
+
+def test_ogr_oci_datetime_with_tz():
+
+    lyr = gdaltest.oci_ds.CreateLayer(
+        "testdate_with_tz",
+        geom_type=ogr.wkbNone,
+        options=["TIMESTAMP_WITH_TIME_ZONE=YES"],
+    )
+    lyr.CreateField(ogr.FieldDefn("MYDATETIME", ogr.OFTDateTime))
+    feat = ogr.Feature(lyr.GetLayerDefn())
+    feat.SetField("MYDATETIME", "2015/02/03 11:33:44.567+0530")
+    lyr.CreateFeature(feat)
+    feat = ogr.Feature(lyr.GetLayerDefn())
+    feat.SetField("MYDATETIME", "2015/02/03 11:33:44-0530")
+    lyr.CreateFeature(feat)
+    feat = ogr.Feature(lyr.GetLayerDefn())
+    feat.SetField("MYDATETIME", "2015/02/03 11:33:44+00")
+    lyr.CreateFeature(feat)
+    feat = ogr.Feature(lyr.GetLayerDefn())
+    feat.SetField("MYDATETIME", "2015/02/03 11:33:44")  # UTC assumed...
+    lyr.CreateFeature(feat)
+    lyr.SyncToDisk()
+
+    with gdaltest.oci_ds.ExecuteSQL(
+        "SELECT MYDATETIME FROM testdate_with_tz"
+    ) as sql_lyr:
+        assert sql_lyr.GetLayerDefn().GetFieldDefn(0).GetType() == ogr.OFTDateTime
+        assert sql_lyr.GetLayerDefn().GetFieldDefn(0).GetTZFlag() == ogr.TZFLAG_MIXED_TZ
+        f = sql_lyr.GetNextFeature()
+        assert f.GetField(0) == "2015/02/03 11:33:44.567+0530"
+        f = sql_lyr.GetNextFeature()
+        assert f.GetField(0) == "2015/02/03 11:33:44-0530"
+        f = sql_lyr.GetNextFeature()
+        assert f.GetField(0) == "2015/02/03 11:33:44+00"
+        f = sql_lyr.GetNextFeature()
+        assert f.GetField(0) == "2015/02/03 11:33:44+00"  # UTC assumed...
 
 
 ###############################################################################
diff --git a/doc/source/drivers/vector/oci.rst b/doc/source/drivers/vector/oci.rst
index 764767c2c9d5..2c367b394faf 100644
--- a/doc/source/drivers/vector/oci.rst
+++ b/doc/source/drivers/vector/oci.rst
@@ -288,6 +288,17 @@ The following layer creation options are supported:
       name, it can be supplied with the GEOMETRY_NAME layer creation
       option.
 
+-  .. lco:: TIMESTAMP_WITH_TIME_ZONE
+      :choices: YES, NO
+      :default: NO
+      :since: 3.10.1
+
+      Whether DateTime fields should be created with TIMESTAMP WITH TIME ZONE
+      Oracle type (otherwise without timezone).
+      When creating a field, if it advertises a known or mixed time zone,
+      TIMESTAMP_WITH_TIME_ZONE will default to YES, otherwise it will default to
+      NO.
+
 Layer Open Options
 ~~~~~~~~~~~~~~~~~~
 
diff --git a/ogr/ogrsf_frmts/oci/ogr_oci.h b/ogr/ogrsf_frmts/oci/ogr_oci.h
index 616822136fd7..a8d483ed469b 100644
--- a/ogr/ogrsf_frmts/oci/ogr_oci.h
+++ b/ogr/ogrsf_frmts/oci/ogr_oci.h
@@ -19,6 +19,7 @@
 #include "cpl_error.h"
 
 #include <map>
+#include <set>
 
 /* -------------------------------------------------------------------- */
 /*      Low level Oracle spatial declarations.                          */
@@ -246,6 +247,8 @@ class OGROCILayer CPL_NON_FINAL : public OGRLayer
     char *pszFIDName;
     int iFIDColumn;
 
+    std::set<int> setFieldIndexWithTimeStampWithTZ{};
+
     OGRGeometry *TranslateGeometry();
     OGRGeometry *TranslateGeometryElement(int *piElement, int nGType,
                                           int nDimension, int nEType,
diff --git a/ogr/ogrsf_frmts/oci/ogrocidrivercore.cpp b/ogr/ogrsf_frmts/oci/ogrocidrivercore.cpp
index b1432a1150a9..80a445636eed 100644
--- a/ogr/ogrsf_frmts/oci/ogrocidrivercore.cpp
+++ b/ogr/ogrsf_frmts/oci/ogrocidrivercore.cpp
@@ -107,6 +107,9 @@ void OGROCIDriverSetCommonMetadata(GDALDriver *poDriver)
         "  <Option name='FIRST_ID' type='int' description='First id value'/>"
         "  <Option name='NO_LOGGING' type='boolean' description='Create table "
         "with no_logging parameters' default='NO'/>"
+        "  <Option name='TIMESTAMP_WITH_TIME_ZONE' type='boolean' description='"
+        "Whether DateTime fields should be created with TIMESTAMP WITH TIME "
+        "ZONE Oracle type (otherwise without timezone)'/>"
         "</LayerCreationOptionList>");
 
     poDriver->SetMetadataItem(GDAL_DMD_CREATIONFIELDDATATYPES,
diff --git a/ogr/ogrsf_frmts/oci/ogrocisession.cpp b/ogr/ogrsf_frmts/oci/ogrocisession.cpp
index 3e8950bf4592..a3339bd8e90d 100644
--- a/ogr/ogrsf_frmts/oci/ogrocisession.cpp
+++ b/ogr/ogrsf_frmts/oci/ogrocisession.cpp
@@ -496,11 +496,16 @@ CPLErr OGROCISession::GetParamInfo(OCIParam *hParamDesc,
             poOGRDefn->SetType(OFTDate);
             break;
         case SQLT_TIMESTAMP:
+        case SQLT_TIME:
+            poOGRDefn->SetType(OFTDateTime);
+            break;
         case SQLT_TIMESTAMP_TZ:
         case SQLT_TIMESTAMP_LTZ:
-        case SQLT_TIME:
         case SQLT_TIME_TZ:
             poOGRDefn->SetType(OFTDateTime);
+            // Indicates that there's timezones. They might not actually be
+            // mixed !
+            poOGRDefn->SetTZFlag(OGR_TZFLAG_MIXED_TZ);
             break;
 
         case SQLT_RID:
diff --git a/ogr/ogrsf_frmts/oci/ogrocitablelayer.cpp b/ogr/ogrsf_frmts/oci/ogrocitablelayer.cpp
index 46646fdefe87..cd87bf3f16b4 100644
--- a/ogr/ogrsf_frmts/oci/ogrocitablelayer.cpp
+++ b/ogr/ogrsf_frmts/oci/ogrocitablelayer.cpp
@@ -16,6 +16,8 @@
 #include "cpl_conv.h"
 #include "cpl_string.h"
 
+#include <cmath>
+
 static int nDiscarded = 0;
 static int nHits = 0;
 
@@ -283,6 +285,11 @@ OGRFeatureDefn *OGROCITableLayer::ReadTableDefinition(const char *pszTable)
             continue;
         }
 
+        if (oField.GetTZFlag() >= OGR_TZFLAG_MIXED_TZ)
+        {
+            setFieldIndexWithTimeStampWithTZ.insert(poDefn->GetFieldCount());
+        }
+
         poDefn->AddFieldDefn(&oField);
     }
 
@@ -2096,6 +2103,56 @@ OGRErr OGROCITableLayer::BoundCreateFeature(OGRFeature *poFeature)
                 ((char *)papWriteFields[i]) + iCache * nEachBufSize;
             strncpy(pszTarget, pszStrValue, nLen);
             pszTarget[nLen] = '\0';
+
+            if (poFldDefn->GetType() == OFTDateTime &&
+                cpl::contains(setFieldIndexWithTimeStampWithTZ, i))
+            {
+                const auto *psField = poFeature->GetRawFieldRef(i);
+                int nTZHour = 0;
+                int nTZMin = 0;
+                if (psField->Date.TZFlag > OGR_TZFLAG_MIXED_TZ)
+                {
+                    const int nOffset =
+                        (psField->Date.TZFlag - OGR_TZFLAG_UTC) * 15;
+                    nTZHour =
+                        static_cast<int>(nOffset / 60);  // Round towards zero.
+                    nTZMin = std::abs(nOffset - nTZHour * 60);
+                }
+                else
+                {
+                    CPLError(CE_Warning, CPLE_AppDefined,
+                             "DateTime %s has no time zone whereas it should "
+                             "have. Assuming +00:00",
+                             pszTarget);
+                }
+                CPLsnprintf(pszTarget, nEachBufSize,
+                            "%04d-%02d-%02d %02d:%02d:%06.3f %s%02d%02d",
+                            psField->Date.Year, psField->Date.Month,
+                            psField->Date.Day, psField->Date.Hour,
+                            psField->Date.Minute, psField->Date.Second,
+                            (psField->Date.TZFlag <= OGR_TZFLAG_MIXED_TZ ||
+                             psField->Date.TZFlag >= OGR_TZFLAG_UTC)
+                                ? "+"
+                                : "-",
+                            std::abs(nTZHour), nTZMin);
+            }
+            else if (poFldDefn->GetType() == OFTDateTime)
+            {
+                const auto *psField = poFeature->GetRawFieldRef(i);
+                if (psField->Date.TZFlag > OGR_TZFLAG_MIXED_TZ)
+                {
+                    CPLError(CE_Warning, CPLE_AppDefined,
+                             "DateTime %s has a time zone whereas the target "
+                             "field does not support time zone. Time zone will "
+                             "be dropped.",
+                             pszTarget);
+                }
+                CPLsnprintf(pszTarget, nEachBufSize,
+                            "%04d-%02d-%02d %02d:%02d:%06.3f",
+                            psField->Date.Year, psField->Date.Month,
+                            psField->Date.Day, psField->Date.Hour,
+                            psField->Date.Minute, psField->Date.Second);
+            }
         }
     }
 
diff --git a/ogr/ogrsf_frmts/oci/ogrociwritablelayer.cpp b/ogr/ogrsf_frmts/oci/ogrociwritablelayer.cpp
index de9bb30d5c71..8f83d7599233 100644
--- a/ogr/ogrsf_frmts/oci/ogrociwritablelayer.cpp
+++ b/ogr/ogrsf_frmts/oci/ogrociwritablelayer.cpp
@@ -291,7 +291,20 @@ OGRErr OGROCIWritableLayer::CreateField(const OGRFieldDefn *poFieldIn,
     }
     else if (oField.GetType() == OFTDateTime)
     {
-        snprintf(szFieldType, sizeof(szFieldType), "TIMESTAMP(3)");
+        const char *pszTIMESTAMP_WITH_TIME_ZONE =
+            CSLFetchNameValue(papszOptions, "TIMESTAMP_WITH_TIME_ZONE");
+        if ((!pszTIMESTAMP_WITH_TIME_ZONE &&
+             oField.GetTZFlag() >= OGR_TZFLAG_MIXED_TZ) ||
+            (pszTIMESTAMP_WITH_TIME_ZONE &&
+             CPLTestBool(pszTIMESTAMP_WITH_TIME_ZONE)))
+        {
+            setFieldIndexWithTimeStampWithTZ.insert(
+                poFeatureDefn->GetFieldCount());
+            snprintf(szFieldType, sizeof(szFieldType),
+                     "TIMESTAMP(3) WITH TIME ZONE");
+        }
+        else
+            snprintf(szFieldType, sizeof(szFieldType), "TIMESTAMP(3)");
     }
     else if (bApproxOK)
     {