[fdo-internals] RFC 68 for review

[Greg Boone]

In general I have no issues with the goals of the RFC.

Some initial feedback on the patch. I will do a deeper review and get back to you.


(a) What is the difference between the following 2 functions? They seem duplicates of each other. SupportsAnsiQuotes() is never used.

Index: Providers/GenericRdbms/Src/PostGis/SchemaMgr/Ph/Mgr.cpp
===================================================================
--- Providers/GenericRdbms/Src/PostGis/SchemaMgr/Ph/Mgr.cpp	(revision 6964)
+++ Providers/GenericRdbms/Src/PostGis/SchemaMgr/Ph/Mgr.cpp	(working copy) 

bool FdoSmPhPostGisMgr::SupportsAnsiQuotes()
{
  if(mIsCaseSensitive)
  {
    return true;
  }
  else
  {
    return false;
  }
}

bool FdoSmPhPostGisMgr::IsCaseSensitive()
{
  return mIsCaseSensitive;
}



(b) Why did you remove the use of GIST_GEOMETRY_OPS below?

Index: Providers/GenericRdbms/Src/PostGis/SchemaMgr/Ph/SpatialIndex.cpp
===================================================================
--- Providers/GenericRdbms/Src/PostGis/SchemaMgr/Ph/SpatialIndex.cpp	(revision 6964)
+++ Providers/GenericRdbms/Src/PostGis/SchemaMgr/Ph/SpatialIndex.cpp	(working copy)
@@ -158,7 +158,7 @@
                 indexName = indexName.Right(L"."); // no schema
 
             sqlStmt = FdoStringP::Format(
-                L"CREATE INDEX \"%ls\"  ON %ls USING GIST (\"%ls\" GIST_GEOMETRY_OPS)",
+                L"CREATE INDEX \"%ls\"  ON %ls USING GIST (\"%ls\")",
                 (FdoString*) indexName,
                 (FdoString*) tableDbQName, // no dbo. prefix etc.
                 column->GetName()


(c) Can you update your indent/tab to be 4 spaces? That is the expected standard. Also. all tab characters should be replaced by 4 spaces.


(d) I believe you have a memory leak. New and no subsequent delete.

Index: Providers/GenericRdbms/Src/PostGis/Fdo/FdoRdbmsPostGisConnectionInfo.cpp
===================================================================
--- Providers/GenericRdbms/Src/PostGis/Fdo/FdoRdbmsPostGisConnectionInfo.cpp	(revision 6964)
+++ Providers/GenericRdbms/Src/PostGis/Fdo/FdoRdbmsPostGisConnectionInfo.cpp	(working copy)
@@ -92,6 +92,16 @@
                 NlsMsgGet(FDORDBMS_117, "DataStore"), L"",
                 false, false, true, false, false, true, false, 0, NULL); 
         mPropertyDictionary->AddProperty(pProp);
+
+        wchar_t **boolValues = new wchar_t*[2];
+        boolValues[0] = new wchar_t[5];
+        boolValues[1] = new wchar_t[6];
+        wcscpy(boolValues[0], L"TRUE");
+        wcscpy(boolValues[1], L"FALSE");
+        pProp = new ConnectionProperty (FDO_RDBMS_CONNECTION_CASESENSITIVE, L"CaseSensitive", L"TRUE", false, false, true, false, false, false, false, 2, (const wchar_t**)boolValues);
+
+
+        mPropertyDictionary->AddProperty(pProp);
     }



Regards,
Greg

-----Original Message-----
From: fdo-internals-bounces at lists.osgeo.org [mailto:fdo-internals-bounces at lists.osgeo.org] On Behalf Of Bruno Scott
Sent: Wednesday, September 11, 2013 8:54 AM
To: fdo-internals at lists.osgeo.org
Subject: [fdo-internals] RFC 68 for review

Hi all,

RFC 68 (http://trac.osgeo.org/fdo/wiki/FDORfc68) has been posted. It proposes to add case insensitive support to the PostgreSQL provider


Bruno Scott



--
View this message in context: http://osgeo-org.1560.x6.nabble.com/RFC-68-for-review-tp5077290.html
Sent from the FDO Internals mailing list archive at Nabble.com.
_______________________________________________
fdo-internals mailing list
fdo-internals at lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/fdo-internals