From 3982b4635f7748a495039e966680b9306e5b9ba6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Philip=20Dub=C3=A9?= <gunkmute@gmail.com>
Date: Thu, 25 Jul 2019 01:24:40 +0000
Subject: [PATCH] CompareShardIntervals: if intervals are equal, compare id.
 Works around sort being unstable

---
 .../distributed/utils/shardinterval_utils.c   | 45 ++++++++++---------
 src/test/regress/bin/normalize.sed            |  2 +
 2 files changed, 25 insertions(+), 22 deletions(-)

diff --git a/src/backend/distributed/utils/shardinterval_utils.c b/src/backend/distributed/utils/shardinterval_utils.c
index 048b860e9..690f6fa49 100644
--- a/src/backend/distributed/utils/shardinterval_utils.c
+++ b/src/backend/distributed/utils/shardinterval_utils.c
@@ -64,39 +64,40 @@ CompareShardIntervals(const void *leftElement, const void *rightElement,
 {
 	ShardInterval *leftShardInterval = *((ShardInterval **) leftElement);
 	ShardInterval *rightShardInterval = *((ShardInterval **) rightElement);
-	Datum leftDatum = 0;
-	Datum rightDatum = 0;
-	Datum comparisonDatum = 0;
 	int comparisonResult = 0;
+	bool leftHasNull = (!leftShardInterval->minValueExists ||
+						!leftShardInterval->maxValueExists);
+	bool rightHasNull = (!rightShardInterval->minValueExists ||
+						 !rightShardInterval->maxValueExists);
 
 	Assert(typeCompareFunction != NULL);
 
-	/*
-	 * Left element should be treated as the greater element in case it doesn't
-	 * have min or max values.
-	 */
-	if (!leftShardInterval->minValueExists || !leftShardInterval->maxValueExists)
+	if (leftHasNull && rightHasNull)
+	{
+		comparisonResult = 0;
+	}
+	else if (leftHasNull)
 	{
 		comparisonResult = 1;
-		return comparisonResult;
 	}
-
-	/*
-	 * Right element should be treated as the greater element in case it doesn't
-	 * have min or max values.
-	 */
-	if (!rightShardInterval->minValueExists || !rightShardInterval->maxValueExists)
+	else if (rightHasNull)
 	{
 		comparisonResult = -1;
-		return comparisonResult;
+	}
+	else
+	{
+		/* if both shard interval have min/max values, calculate comparison result */
+		Datum leftDatum = leftShardInterval->minValue;
+		Datum rightDatum = rightShardInterval->minValue;
+		Datum comparisonDatum = CompareCall2(typeCompareFunction, leftDatum, rightDatum);
+		comparisonResult = DatumGetInt32(comparisonDatum);
 	}
 
-	/* if both shard interval have min/max values, calculate the comparison result */
-	leftDatum = leftShardInterval->minValue;
-	rightDatum = rightShardInterval->minValue;
-
-	comparisonDatum = CompareCall2(typeCompareFunction, leftDatum, rightDatum);
-	comparisonResult = DatumGetInt32(comparisonDatum);
+	/* Two different shards should never be equal */
+	if (comparisonResult == 0)
+	{
+		return CompareShardIntervalsById(leftElement, rightElement);
+	}
 
 	return comparisonResult;
 }
diff --git a/src/test/regress/bin/normalize.sed b/src/test/regress/bin/normalize.sed
index 7b8c14bb5..9f15a17d8 100644
--- a/src/test/regress/bin/normalize.sed
+++ b/src/test/regress/bin/normalize.sed
@@ -8,8 +8,10 @@
 
 # In all tests, normalize worker ports, placement ids, and shard ids
 s/localhost:[0-9]+/localhost:xxxxx/g
+s/ port=[0-9]+ / port=xxxxx /g
 s/placement [0-9]+/placement xxxxx/g
 s/shard [0-9]+/shard xxxxx/g
+s/assigned task [0-9]+ to node/assigned task to node/
 
 # In foreign_key_to_reference_table, normalize shard table names, etc in
 # the generated plan