Fix "3rd party error log" message in the log with debug builds (#550)

The new CDebugCounter class caused error messages when we called
gpos_terminate in a debug build, because it didn't release its
memory pool before the CMemoryPoolManager that managed it got
destroyed.

The log messages started with "3rd party error log:".

The fix makes sure that we delete the static CDebugCounter object
and its associated memory pool before deleting the CMemoryPoolManager
in the gpos_terminate call.

Also, the code caused simple queries like "select 1" to fall back in
debug builds, due to an assert in the IDatumInt8::GetByteArrayValue()
function.  This has been fixed as well.

Note that most of the diffs in COptimizer::PrintQueryOrPlan() are due
to indentation, the actual code change is just a dynamic cast and an
added "if (NULL != datum)".

Also fix a double deletion issue, visible in debug builds

We had an issue in gpos_terminate, when we deleted the single CDebugCounter
object. This object was allocated from a memory pool, and we destroyed that
memory pool in the destructor - while the CDebugCounter object was still
allocated in the memory pool. This led to a recursive deletion of the same
object and an error.

In some cases (e.g. gpstart on 5X, possibly others) this error caused the
gpstart operation to fail.

The fix moves the destruction of the memory pool out, to a point where the
memory pool is empty.

CMakeLists.txt

@@ -6,7 +6,7 @@ project(gpopt LANGUAGES CXX C)
 set(CMAKE_CXX_STANDARD 98)
 set(GPORCA_VERSION_MAJOR 3)
 set(GPORCA_VERSION_MINOR 80)
-set(GPORCA_VERSION_PATCH 0)
+set(GPORCA_VERSION_PATCH 1)
 set(GPORCA_VERSION_STRING "${GPORCA_VERSION_MAJOR}.${GPORCA_VERSION_MINOR}.${GPORCA_VERSION_PATCH}")
 
 # Whenever an ABI-breaking change is made to GPORCA, this should be incremented.

libgpopt/src/optimizer/COptimizer.cpp

@@ -14,6 +14,7 @@
 #include "gpos/error/CErrorHandlerStandard.h"
 #include "gpos/io/CFileDescriptor.h"
 
+#include "naucrates/base/CDatumGenericGPDB.h"
 #include "naucrates/dxl/operators/CDXLNode.h"
 #include "naucrates/md/IMDProvider.h"
 
@@ -181,23 +182,28 @@ COptimizer::PrintQueryOrPlan
 			if (COperator::EopScalarConst == pop->Eopid())
 			{
 				CScalarConst *popScalarConst = CScalarConst::PopConvert(pop);
-				const char *select_element_bytes = (const char *) popScalarConst->GetDatum()->GetByteArrayValue();
-				ULONG select_element_len = clib::Strlen(select_element_bytes);
-				const char *query_name_prefix = "query name: ";
-				ULONG query_name_prefix_len = clib::Strlen(query_name_prefix);
-
-				if (0 == clib::Strncmp((const char *) select_element_bytes,
-									   query_name_prefix,
-									   query_name_prefix_len))
+				CDatumGenericGPDB *datum = dynamic_cast<CDatumGenericGPDB *>(popScalarConst->GetDatum());
+
+				if (NULL != datum)
 				{
-					// the constant in the select starts with "query_name: "
-					for (ULONG i=query_name_prefix_len; i<select_element_len; i++)
+					const char *select_element_bytes = (const char *) datum->GetByteArrayValue();
+					ULONG select_element_len = clib::Strlen(select_element_bytes);
+					const char *query_name_prefix = "query name: ";
+					ULONG query_name_prefix_len = clib::Strlen(query_name_prefix);
+
+					if (0 == clib::Strncmp((const char *) select_element_bytes,
+										   query_name_prefix,
+										   query_name_prefix_len))
 					{
-						if (select_element_bytes[i] > 0)
+						// the constant in the select starts with "query_name: "
+						for (ULONG i=query_name_prefix_len; i<select_element_len; i++)
 						{
-							// the query name should contain ASCII characters,
-							// we skip all other characters
-							query_name.append(1, select_element_bytes[i]);
+							if (select_element_bytes[i] > 0)
+							{
+								// the query name should contain ASCII characters,
+								// we skip all other characters
+								query_name.append(1, select_element_bytes[i]);
+							}
 						}
 					}
 				}

libgpos/include/gpos/common/CDebugCounter.h

@@ -185,8 +185,12 @@ namespace gpos
 
 		// methods used by ORCA to provide the infrastructure for event counting
 		CDebugCounter(CMemoryPool *mp);
-		// called once in the lifetime of the process from gpos_init
+
+		~CDebugCounter();
+
+		// The static CDebugCounter lives from gpos_init to gpos_terminate
 		static void Init();
+		static void Shutdown();
 
 		// Prepare for running the next query, providing an optional name for the next
 		// query. Note that if a name is provided, we assume the current query is a

libgpos/src/_api.cpp

@@ -282,6 +282,9 @@ int gpos_exec
 //---------------------------------------------------------------------------
 void gpos_terminate()
 {
+#ifdef GPOS_DEBUG_COUNTERS
+	CDebugCounter::Shutdown();
+#endif
 #ifdef GPOS_DEBUG
 #ifdef GPOS_FPSIMULATOR
 	CFSimulator::FSim()->Shutdown();

libgpos/src/common/CDebugCounter.cpp

@@ -47,6 +47,12 @@ CDebugCounter::CDebugCounter(CMemoryPool *mp) :
 	m_hashmap = GPOS_NEW(mp) CounterKeyToValueMap(mp);
 }
 
+CDebugCounter::~CDebugCounter()
+{
+  CRefCount::SafeRelease(m_hashmap);
+  m_hashmap = NULL;
+}
+
 void CDebugCounter::Init()
 {
 	CAutoMemoryPool amp;
@@ -59,6 +65,18 @@ void CDebugCounter::Init()
 	(void) amp.Detach();
 }
 
+void CDebugCounter::Shutdown()
+{
+	if (NULL != m_instance)
+	{
+		CMemoryPool *mp = m_instance->m_mp;
+
+		GPOS_DELETE(m_instance);
+		m_instance = NULL;
+		CMemoryPoolManager::GetMemoryPoolMgr()->Destroy(mp);
+	}
+}
+
 void CDebugCounter::NextQry(const char *next_qry_name)
 {
 	if (NULL != next_qry_name && '\0' != *next_qry_name)