Add support for pre-release version checks

Version checks fail on pre-release versions (e.g., `1.0.0-rc1`) since
the version parsing didn't expect a pre-release tag at the end.

This change adds support for pre-release tags, using lexicographical
comparison in case of pre-release tags. Note that having a pre-release
tag vs not having one makes the version strictly "smaller", given
otherwise identical versions.

Author: erimatnor

sql/version.sql

@@ -1,8 +1,13 @@
 CREATE OR REPLACE FUNCTION _timescaledb_internal.get_git_commit() RETURNS TEXT
     AS '@MODULE_PATHNAME@', 'get_git_commit' LANGUAGE C IMMUTABLE STRICT PARALLEL SAFE;
 
-CREATE OR REPLACE FUNCTION _timescaledb_internal.get_os_info() RETURNS TABLE(sysname TEXT, version TEXT, release TEXT)
+CREATE OR REPLACE FUNCTION _timescaledb_internal.get_os_info()
+    RETURNS TABLE(sysname TEXT, version TEXT, release TEXT)
     AS '@MODULE_PATHNAME@', 'ts_get_os_info' LANGUAGE C IMMUTABLE STRICT PARALLEL SAFE;
 
+CREATE OR REPLACE FUNCTION _timescaledb_internal.get_version()
+    RETURNS TABLE(major INTEGER, minor INTEGER, patch INTEGER, modtag TEXT)
+    AS '@MODULE_PATHNAME@', 'ts_version_get_info' LANGUAGE C IMMUTABLE STRICT PARALLEL SAFE;
+
 CREATE OR REPLACE FUNCTION get_telemetry_report() RETURNS TEXT
     AS '@MODULE_PATHNAME@', 'ts_get_telemetry_report' LANGUAGE C IMMUTABLE STRICT PARALLEL SAFE;

src/telemetry/telemetry.c

@@ -43,40 +43,15 @@
 #define POSTGIS			"postgis"
 
 static const char *related_extensions[] = {PG_PROMETHEUS, POSTGIS};
-static const char *version_delimiter[3] = {".", ".", ""};
-
-static bool
-parse_version_string(const char *version, long version_result[3])
-{
-	char	   *parse_version = pstrdup(version);
-	int			i;
-
-	for (i = 0; i < 3; i++)
-	{
-		const char *subversion;
-		char	   *endptr;
-
-		subversion = strtok(i == 0 ? parse_version : NULL, version_delimiter[i]);
-
-		if (subversion == NULL)
-			return i > 0;
-
-		version_result[i] = strtol(subversion, &endptr, 10);
-
-		if (endptr != NULL && *endptr != '.' && *endptr != '\0')
-			return false;
-	}
-
-	return true;
-}
 
 bool
-telemetry_parse_version(const char *json, const long installed_version[3], VersionResult *result)
+telemetry_parse_version(const char *json, VersionInfo *installed_version, VersionResult *result)
 {
 	Datum		version = DirectFunctionCall2(json_object_field_text,
 											  CStringGetTextDatum(json),
 											  PointerGetDatum(cstring_to_text(TS_VERSION_JSON_FIELD)));
-	int			i;
+
+	memset(result, 0, sizeof(VersionResult));
 
 	result->versionstr = text_to_cstring(DatumGetTextPP(version));
 
@@ -89,25 +64,17 @@ telemetry_parse_version(const char *json, const long installed_version[3], Versi
 	}
 
 	/*
-	 * Now parse the version string. We expect format to be XX.XX.XX, and if
-	 * not, we error out
+	 * Now parse the version string. We expect format to be
+	 * XX.XX.XX-<prerelease_tag>, and if not, we error out
 	 */
-	if (!parse_version_string(result->versionstr, result->version))
+	if (!version_parse(result->versionstr, &result->vinfo))
 	{
-		result->errhint = "could not parse version string";
+		result->errhint = psprintf("parsing failed for version string \"%s\"", result->versionstr);
 		return false;
 	}
 
-	for (i = 0; i < 3; i++)
-	{
-		if (installed_version[i] < result->version[i])
-			return true;
-
-		if (installed_version[i] > result->version[i])
-			break;
-	}
-
-	result->is_up_to_date = true;
+	if (version_cmp(installed_version, &result->vinfo) >= 0)
+		result->is_up_to_date = true;
 
 	return true;
 }
@@ -117,21 +84,18 @@ telemetry_parse_version(const char *json, const long installed_version[3], Versi
  * called "current_timescaledb_version". Check this against the local
  * version, and notify the user if it is behind.
  */
-static void
+static bool
 process_response(const char *json)
 {
-	const long	installed_version[3] = {
-		strtol(TIMESCALEDB_MAJOR_VERSION, NULL, 10),
-		strtol(TIMESCALEDB_MINOR_VERSION, NULL, 10),
-	strtol(TIMESCALEDB_PATCH_VERSION, NULL, 10)};
-	VersionResult result = {0};
+	VersionInfo installed_version;
+	VersionResult result;
 
-	if (!telemetry_parse_version(json, installed_version, &result))
-	{
-		if (NULL == result.versionstr)
-			elog(ERROR, "could not get TimescaleDB version from server response");
+	version_get_info(&installed_version);
 
-		elog(ERROR, "ill-formatted TimescaleDB version in server response");
+	if (!telemetry_parse_version(json, &installed_version, &result))
+	{
+		elog(WARNING, "could not get TimescaleDB version from server response: %s", result.errhint);
+		return false;
 	}
 
 	if (result.is_up_to_date)
@@ -141,6 +105,8 @@ process_response(const char *json)
 				(errmsg("the \"%s\" extension is not up-to-date", EXTENSION_NAME),
 				 errhint("The most up-to-date version is %s, the installed version is %s",
 						 result.versionstr, TIMESCALEDB_VERSION_MOD)));
+
+	return true;
 }
 
 static char *
@@ -286,7 +252,10 @@ telemetry_connect(void)
 	conn = connection_create(CONNECTION_SSL);
 
 	if (conn == NULL)
-		elog(ERROR, "could not create telemetry connection");
+	{
+		elog(WARNING, "could not create telemetry connection");
+		return NULL;
+	}
 
 	ret = connection_connect(conn, TELEMETRY_HOST, TELEMETRY_SCHEME, 0);
 
@@ -295,8 +264,9 @@ telemetry_connect(void)
 		const char *errstr = connection_get_and_clear_error(conn);
 
 		connection_destroy(conn);
+		conn = NULL;
 
-		ereport(ERROR,
+		ereport(WARNING,
 				(errcode(ERRCODE_INTERNAL_ERROR),
 				 errmsg("could not make a connection to %s", TELEMETRY_ENDPOINT),
 				 errdetail("%s", errstr)));
@@ -335,11 +305,17 @@ telemetry_main()
 	connection_destroy(conn);
 
 	if (err != HTTP_ERROR_NONE)
-		elog(ERROR, "telemetry error: %s", http_strerror(err));
+	{
+		elog(WARNING, "telemetry error: %s", http_strerror(err));
+		return;
+	}
 
 	if (!http_response_state_valid_status(rsp))
-		elog(ERROR, "telemetry got unexpected HTTP response status: %d",
+	{
+		elog(WARNING, "telemetry got unexpected HTTP response status: %d",
 			 http_response_state_status_code(rsp));
+		return;
+	}
 
 	/*
 	 * Do the version-check. Response is the body of a well-formed HTTP

src/telemetry/telemetry.h

@@ -22,15 +22,15 @@
 
 typedef struct VersionResult
 {
+	VersionInfo vinfo;
 	const char *versionstr;
-	long		version[3];
 	bool		is_up_to_date;
 	const char *errhint;
 } VersionResult;
 
 HttpRequest *build_version_request(const char *host, const char *path);
 Connection *telemetry_connect(void);
-bool		telemetry_parse_version(const char *json, const long installed_version[3], VersionResult *result);
+bool		telemetry_parse_version(const char *json, VersionInfo *vinfo, VersionResult *result);
 
 /*
  *	This function is intended as the main function for a BGW.

src/version.c

@@ -9,10 +9,152 @@
 #include "compat.h"
 #include "gitcommit.h"
 #include "version.h"
+#include "config.h"
 
 #define STR_EXPAND(x) #x
 #define STR(x) STR_EXPAND(x)
 
+void
+version_get_info(VersionInfo *vinfo)
+{
+	memset(vinfo, 0, sizeof(VersionInfo));
+	vinfo->version[0] = strtol(TIMESCALEDB_MAJOR_VERSION, NULL, 10);
+	vinfo->version[1] = strtol(TIMESCALEDB_MINOR_VERSION, NULL, 10);
+	vinfo->version[2] = strtol(TIMESCALEDB_PATCH_VERSION, NULL, 10);
+
+	if (strlen(TIMESCALEDB_MOD_VERSION) > 0)
+	{
+		StrNCpy(vinfo->version_mod, TIMESCALEDB_MOD_VERSION, sizeof(vinfo->version_mod));
+		vinfo->has_version_mod = true;
+	}
+}
+
+/*
+ * Compare two versions.
+ *
+ * It returns an integer less than, equal to, or greater than zero if version
+ * v1 is found, respectively, to be less than, to match, or be greater than
+ * version v2.
+ */
+int
+version_cmp(VersionInfo *v1, VersionInfo *v2)
+{
+	int			i;
+
+	for (i = 0; i < 3; i++)
+	{
+		if (v1->version[i] > v2->version[i])
+			return 1;
+
+		if (v1->version[i] < v2->version[i])
+			return -1;
+	}
+
+	/*
+	 * Note that the version mod signifies a pre-release version, so having a
+	 * version mod is "less" than not having one with otherwise identical
+	 * versions
+	 */
+	if (v1->has_version_mod && !v2->has_version_mod)
+		return -1;
+
+	if (!v1->has_version_mod && v2->has_version_mod)
+		return 1;
+
+	/* Compare the version mod lexicographically */
+	if (v1->has_version_mod && v2->has_version_mod)
+		return strncmp(v1->version_mod, v2->version_mod, sizeof(v1->version_mod));
+
+	return 0;
+}
+
+#define NUM_VERSION_DELIMS 4
+
+static const char *version_delimiter[NUM_VERSION_DELIMS] = {".", ".", "-", ""};
+
+bool
+version_parse(const char *version, VersionInfo *result)
+{
+	char	   *parse_version = pstrdup(version);
+	int			i;
+
+	memset(result, 0, sizeof(VersionInfo));
+
+	for (i = 0; i < NUM_VERSION_DELIMS; i++)
+	{
+		const char *subversion;
+
+		subversion = strtok(i == 0 ? parse_version : NULL, version_delimiter[i]);
+
+		if (subversion == NULL)
+			return i > 0;
+
+		/*
+		 * If we are past the '-' delimiter, we've found the mod/pre-release
+		 * tag
+		 */
+		if (i > 0 && version_delimiter[i - 1][0] == '-')
+		{
+			int			len = snprintf(result->version_mod, sizeof(result->version_mod) - 1, "%s", subversion);
+
+			if (len > (sizeof(result->version_mod) - 1))
+				return false;
+
+			if (len > 0)
+				result->has_version_mod = true;
+		}
+		else
+		{
+			char	   *endptr;
+
+			result->version[i] = strtol(subversion, &endptr, 10);
+
+			/*
+			 * We expect the parsing of the version num to end at a '\0' since
+			 * strtok() should have replaced the delimiter with a '\0' if the
+			 * delimiter was found
+			 */
+			if (endptr != NULL && *endptr != '\0')
+				return false;
+		}
+	}
+
+	return true;
+}
+
+TS_FUNCTION_INFO_V1(ts_version_get_info);
+
+Datum
+ts_version_get_info(PG_FUNCTION_ARGS)
+{
+	VersionInfo info;
+	TupleDesc	tupdesc;
+	Datum		values[4];
+	bool		nulls[4] = {false};
+	HeapTuple	tuple;
+
+	version_get_info(&info);
+
+	if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
+		ereport(ERROR,
+				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+				 errmsg("function returning record called in context "
+						"that cannot accept type record")));
+
+	values[0] = Int32GetDatum((int32) info.version[0]);
+	values[1] = Int32GetDatum((int32) info.version[1]);
+	values[2] = Int32GetDatum((int32) info.version[2]);
+
+	if (info.has_version_mod)
+		values[3] = CStringGetTextDatum(info.version_mod);
+	else
+		nulls[3] = true;
+
+	tuple = heap_form_tuple(tupdesc, values, nulls);
+
+	return HeapTupleGetDatum(tuple);
+}
+
 const char *git_commit = STR(EXT_GIT_COMMIT);
 
 TS_FUNCTION_INFO_V1(get_git_commit);
@@ -59,9 +201,9 @@ version_get_os_info(VersionOSInfo *info)
 	if (!VerQueryValueA(buffer, TEXT("\\"), &vinfo, &vinfo_len))
 		goto error;
 
-	snprintf(info->sysname, VERSION_OS_INFO_LEN - 1, "Windows");
-	snprintf(info->version, VERSION_OS_INFO_LEN - 1, "%u", HIWORD(vinfo->dwProductVersionMS));
-	snprintf(info->release, VERSION_OS_INFO_LEN - 1, "%u", LOWORD(vinfo->dwProductVersionMS));
+	snprintf(info->sysname, VERSION_INFO_LEN - 1, "Windows");
+	snprintf(info->version, VERSION_INFO_LEN - 1, "%u", HIWORD(vinfo->dwProductVersionMS));
+	snprintf(info->release, VERSION_INFO_LEN - 1, "%u", LOWORD(vinfo->dwProductVersionMS));
 
 	pfree(buffer);
 
@@ -85,9 +227,9 @@ version_get_os_info(VersionOSInfo *info)
 	uname(&os_info);
 
 	memset(info, 0, sizeof(VersionOSInfo));
-	strncpy(info->sysname, os_info.sysname, VERSION_OS_INFO_LEN - 1);
-	strncpy(info->version, os_info.version, VERSION_OS_INFO_LEN - 1);
-	strncpy(info->release, os_info.release, VERSION_OS_INFO_LEN - 1);
+	strncpy(info->sysname, os_info.sysname, VERSION_INFO_LEN - 1);
+	strncpy(info->version, os_info.version, VERSION_INFO_LEN - 1);
+	strncpy(info->release, os_info.release, VERSION_INFO_LEN - 1);
 
 	return true;
 }