Skip to content

Commit

Permalink
OCI: add a TIMESTAMP_WITH_TIME_ZONE layer creation option, and ogr2og…
Browse files Browse the repository at this point in the history
…r tweaks

Fixes #11057 (comment)
  • Loading branch information
rouault committed Nov 23, 2024
1 parent 17f6835 commit 7fec8c7
Show file tree
Hide file tree
Showing 8 changed files with 166 additions and 6 deletions.
26 changes: 23 additions & 3 deletions apps/ogr2ogr_lib.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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) &&
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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)
Expand Down
50 changes: 49 additions & 1 deletion autotest/ogr/ogr_oci.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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))
Expand All @@ -736,18 +737,65 @@ 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(
"SELECT MYDATE, MYDATETIME FROM testdate"
) 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...


###############################################################################
Expand Down
11 changes: 11 additions & 0 deletions doc/source/drivers/vector/oci.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
~~~~~~~~~~~~~~~~~~

Expand Down
3 changes: 3 additions & 0 deletions ogr/ogrsf_frmts/oci/ogr_oci.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include "cpl_error.h"

#include <map>
#include <set>

/* -------------------------------------------------------------------- */
/* Low level Oracle spatial declarations. */
Expand Down Expand Up @@ -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,
Expand Down
3 changes: 3 additions & 0 deletions ogr/ogrsf_frmts/oci/ogrocidrivercore.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
7 changes: 6 additions & 1 deletion ogr/ogrsf_frmts/oci/ogrocisession.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
57 changes: 57 additions & 0 deletions ogr/ogrsf_frmts/oci/ogrocitablelayer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
#include "cpl_conv.h"
#include "cpl_string.h"

#include <cmath>

static int nDiscarded = 0;
static int nHits = 0;

Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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);
}
}
}

Expand Down
15 changes: 14 additions & 1 deletion ogr/ogrsf_frmts/oci/ogrociwritablelayer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down

0 comments on commit 7fec8c7

Please sign in to comment.