[GitHub] incubator-trafodion pull request #1277: [TRAFODION-2783] jdbc_test_cdh fails...
Github user selvaganesang commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/1277#discussion_r147251925
--- Diff: core/sql/common/charinfo.cpp ---
@@ -363,21 +363,22 @@ void CollationInfo::display() const
CollationDB::CollationDB(CollHeap *h)
: CollationDBSupertype(h), heap_(h), refreshNeeded_(TRUE)
{
-if (this == ::builtinCollationDB_) return;
+if (this == CharInfo::builtinCollationDB_) return;
--- End diff --
CharInfo::builtinCollationDB_ is a singleton object. It looks like there
could other CollationDB objects, but it will have all the data in the
builtinCollationDB object too
---
[GitHub] incubator-trafodion pull request #1277: [TRAFODION-2783] jdbc_test_cdh fails...
Github user selvaganesang commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/1277#discussion_r147250690
--- Diff: core/sql/common/charinfo.cpp ---
@@ -529,8 +530,10 @@ static const CollationInfo mapCOArray[] = {
CollationInfo(NULL, CharInfo::UNKNOWN_COLLATION,
SQLCOLLATIONSTRING_UNKNOWN,
STATIC_NEG)
};
-static const size_t SIZEOF_CO = sizeof(mapCOArray)/sizeof(CollationInfo);
-const CollationDB CharInfo::builtinCollationDB_(NULL, mapCOArray,
SIZEOF_CO);
+
+#define SIZEOF_CO (sizeof(mapCOArray)/sizeof(CollationInfo))
+
+const CollationDB *CharInfo::builtinCollationDB_;
--- End diff --
It is required in the rework and it is initialized to NULL
---
[GitHub] incubator-trafodion pull request #1275: TRAFODION-2782 reserve UDRs classloa...
Github user asfgit closed the pull request at: https://github.com/apache/incubator-trafodion/pull/1275 ---
[GitHub] incubator-trafodion pull request #1270: [TRAFODION-2714] odb does not load d...
Github user asfgit closed the pull request at: https://github.com/apache/incubator-trafodion/pull/1270 ---
[GitHub] incubator-trafodion pull request #1272: [TRAFODION-2773] When the timeout is...
Github user asfgit closed the pull request at: https://github.com/apache/incubator-trafodion/pull/1272 ---
[GitHub] incubator-trafodion pull request #1277: [TRAFODION-2783] jdbc_test_cdh fails...
Github user DaveBirdsall commented on a diff in the pull request: https://github.com/apache/incubator-trafodion/pull/1277#discussion_r147227161 --- Diff: core/sql/common/charinfo.h --- @@ -260,11 +260,14 @@ class CharInfo Int32 sourceLenInBytes, CharSet targetCS); + static void initBuiltinCollationDB(); + + private: friend class CollationDB; // needs to access builtinCDB_ static const char* const localeCharSet_; - static const CollationDB builtinCollationDB_; + static const CollationDB *builtinCollationDB_; --- End diff -- Oh, sorry; this is the declaration rather than the definition. Ignore my comment. ---
[GitHub] incubator-trafodion pull request #1277: [TRAFODION-2783] jdbc_test_cdh fails...
Github user DaveBirdsall commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/1277#discussion_r147223503
--- Diff: core/sql/common/charinfo.cpp ---
@@ -363,21 +363,22 @@ void CollationInfo::display() const
CollationDB::CollationDB(CollHeap *h)
: CollationDBSupertype(h), heap_(h), refreshNeeded_(TRUE)
{
-if (this == ::builtinCollationDB_) return;
+if (this == CharInfo::builtinCollationDB_) return;
--- End diff --
I'm confused. Is CollationDB a singleton object? Or is there one global
version, and other parts of the code might create their own version?
---
[GitHub] incubator-trafodion pull request #1277: [TRAFODION-2783] jdbc_test_cdh fails...
Github user DaveBirdsall commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/1277#discussion_r147223670
--- Diff: core/sql/common/charinfo.cpp ---
@@ -529,8 +530,10 @@ static const CollationInfo mapCOArray[] = {
CollationInfo(NULL, CharInfo::UNKNOWN_COLLATION,
SQLCOLLATIONSTRING_UNKNOWN,
STATIC_NEG)
};
-static const size_t SIZEOF_CO = sizeof(mapCOArray)/sizeof(CollationInfo);
-const CollationDB CharInfo::builtinCollationDB_(NULL, mapCOArray,
SIZEOF_CO);
+
+#define SIZEOF_CO (sizeof(mapCOArray)/sizeof(CollationInfo))
+
+const CollationDB *CharInfo::builtinCollationDB_;
--- End diff --
Suggest initializing this global pointer to null, so you get better
behavior in race conditions.
---
[GitHub] incubator-trafodion pull request #1277: [TRAFODION-2783] jdbc_test_cdh fails...
Github user DaveBirdsall commented on a diff in the pull request: https://github.com/apache/incubator-trafodion/pull/1277#discussion_r147222686 --- Diff: core/sql/common/charinfo.h --- @@ -260,11 +260,14 @@ class CharInfo Int32 sourceLenInBytes, CharSet targetCS); + static void initBuiltinCollationDB(); + + private: friend class CollationDB; // needs to access builtinCDB_ static const char* const localeCharSet_; - static const CollationDB builtinCollationDB_; + static const CollationDB *builtinCollationDB_; --- End diff -- Suggest initializing to NULL. So if you get a thread race condition, you'll get better behavior if one thread accessing this pointer beats the other thread that is initializing it. ---
[GitHub] incubator-trafodion pull request #1277: [TRAFODION-2783] jdbc_test_cdh fails...
Github user DaveBirdsall commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/1277#discussion_r147223897
--- Diff: core/sql/common/charinfo.cpp ---
@@ -693,3 +696,9 @@ Int32 CharInfo::getMaxConvertedLenInBytes(CharSet
sourceCS,
return ((sourceLenInBytes/minBytesPerChar(sourceCS)) *
maxBytesPerChar(targetCS));
}
+
+void CharInfo::initBuiltinCollationDB()
+{
+ builtinCollationDB_ = new CollationDB(NULL, mapCOArray, SIZEOF_CO);
--- End diff --
Is mapCOArray another global object? If it is const it is OK. If not, how
do we know that it has been constructed already?
---
[GitHub] incubator-trafodion pull request #1274: Miscellaneous authorization changes:
Github user asfgit closed the pull request at: https://github.com/apache/incubator-trafodion/pull/1274 ---
