G2

config/c-compiler.m4

@@ -108,29 +108,61 @@ AC_DEFUN([PGAC_TYPE_128BIT_INT],
 [AC_CACHE_CHECK([for __int128], [pgac_cv__128bit_int],
 [AC_LINK_IFELSE([AC_LANG_PROGRAM([
 /*
+ * We don't actually run this test, just link it to verify that any support
+ * functions needed for __int128 are present.
+ *
  * These are globals to discourage the compiler from folding all the
  * arithmetic tests down to compile-time constants.  We do not have
- * convenient support for 64bit literals at this point...
+ * convenient support for 128bit literals at this point...
  */
 __int128 a = 48828125;
-__int128 b = 97656255;
+__int128 b = 97656250;
 ],[
 __int128 c,d;
 a = (a << 12) + 1; /* 200000000001 */
 b = (b << 12) + 5; /* 400000000005 */
-/* use the most relevant arithmetic ops */
+/* try the most relevant arithmetic ops */
 c = a * b;
 d = (c + b) / b;
-/* return different values, to prevent optimizations */
+/* must use the results, else compiler may optimize arithmetic away */
 if (d != a+1)
-  return 0;
-return 1;
+  return 1;
 ])],
 [pgac_cv__128bit_int=yes],
 [pgac_cv__128bit_int=no])])
 if test x"$pgac_cv__128bit_int" = xyes ; then
-  AC_DEFINE(PG_INT128_TYPE, __int128, [Define to the name of a signed 128-bit integer type.])
-  AC_CHECK_ALIGNOF(PG_INT128_TYPE)
+  # Use of non-default alignment with __int128 tickles bugs in some compilers.
+  # If not cross-compiling, we can test for bugs and disable use of __int128
+  # with buggy compilers.  If cross-compiling, hope for the best.
+  # https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83925
+  AC_CACHE_CHECK([for __int128 alignment bug], [pgac_cv__128bit_int_bug],
+  [AC_RUN_IFELSE([AC_LANG_PROGRAM([
+/* This must match the corresponding code in c.h: */
+#if defined(__GNUC__) || defined(__SUNPRO_C) || defined(__IBMC__)
+#define pg_attribute_aligned(a) __attribute__((aligned(a)))
+#endif
+typedef __int128 int128a
+#if defined(pg_attribute_aligned)
+pg_attribute_aligned(8)
+#endif
+;
+int128a holder;
+void pass_by_val(void *buffer, int128a par) { holder = par; }
+],[
+long int i64 = 97656225L << 12;
+int128a q;
+pass_by_val(main, (int128a) i64);
+q = (int128a) i64;
+if (q != holder)
+  return 1;
+])],
+  [pgac_cv__128bit_int_bug=ok],
+  [pgac_cv__128bit_int_bug=broken],
+  [pgac_cv__128bit_int_bug="assuming ok"])])
+  if test x"$pgac_cv__128bit_int_bug" != xbroken ; then
+    AC_DEFINE(PG_INT128_TYPE, __int128, [Define to the name of a signed 128-bit integer type.])
+    AC_CHECK_ALIGNOF(PG_INT128_TYPE)
+  fi
 fi])# PGAC_TYPE_128BIT_INT
 
 

configure

@@ -14996,12 +14996,15 @@ else
 /* end confdefs.h.  */
 
 /*
+ * We don't actually run this test, just link it to verify that any support
+ * functions needed for __int128 are present.
+ *
  * These are globals to discourage the compiler from folding all the
  * arithmetic tests down to compile-time constants.  We do not have
- * convenient support for 64bit literals at this point...
+ * convenient support for 128bit literals at this point...
  */
 __int128 a = 48828125;
-__int128 b = 97656255;
+__int128 b = 97656250;
 
 int
 main ()
@@ -15010,13 +15013,12 @@ main ()
 __int128 c,d;
 a = (a << 12) + 1; /* 200000000001 */
 b = (b << 12) + 5; /* 400000000005 */
-/* use the most relevant arithmetic ops */
+/* try the most relevant arithmetic ops */
 c = a * b;
 d = (c + b) / b;
-/* return different values, to prevent optimizations */
+/* must use the results, else compiler may optimize arithmetic away */
 if (d != a+1)
-  return 0;
-return 1;
+  return 1;
 
   ;
   return 0;
@@ -15033,10 +15035,65 @@ fi
 { $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv__128bit_int" >&5
 $as_echo "$pgac_cv__128bit_int" >&6; }
 if test x"$pgac_cv__128bit_int" = xyes ; then
+  # Use of non-default alignment with __int128 tickles bugs in some compilers.
+  # If not cross-compiling, we can test for bugs and disable use of __int128
+  # with buggy compilers.  If cross-compiling, hope for the best.
+  # https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83925
+  { $as_echo "$as_me:${as_lineno-$LINENO}: checking for __int128 alignment bug" >&5
+$as_echo_n "checking for __int128 alignment bug... " >&6; }
+if ${pgac_cv__128bit_int_bug+:} false; then :
+  $as_echo_n "(cached) " >&6
+else
+  if test "$cross_compiling" = yes; then :
+  pgac_cv__128bit_int_bug="assuming ok"
+else
+  cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h.  */
+
+/* This must match the corresponding code in c.h: */
+#if defined(__GNUC__) || defined(__SUNPRO_C) || defined(__IBMC__)
+#define pg_attribute_aligned(a) __attribute__((aligned(a)))
+#endif
+typedef __int128 int128a
+#if defined(pg_attribute_aligned)
+pg_attribute_aligned(8)
+#endif
+;
+int128a holder;
+void pass_by_val(void *buffer, int128a par) { holder = par; }
+
+int
+main ()
+{
+
+long int i64 = 97656225L << 12;
+int128a q;
+pass_by_val(main, (int128a) i64);
+q = (int128a) i64;
+if (q != holder)
+  return 1;
+
+  ;
+  return 0;
+}
+_ACEOF
+if ac_fn_c_try_run "$LINENO"; then :
+  pgac_cv__128bit_int_bug=ok
+else
+  pgac_cv__128bit_int_bug=broken
+fi
+rm -f core *.core core.conftest.* gmon.out bb.out conftest$ac_exeext \
+  conftest.$ac_objext conftest.beam conftest.$ac_ext
+fi
+
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv__128bit_int_bug" >&5
+$as_echo "$pgac_cv__128bit_int_bug" >&6; }
+  if test x"$pgac_cv__128bit_int_bug" != xbroken ; then
 
 $as_echo "#define PG_INT128_TYPE __int128" >>confdefs.h
 
-  # The cast to long int works around a bug in the HP C Compiler,
+    # The cast to long int works around a bug in the HP C Compiler,
 # see AC_CHECK_SIZEOF for more information.
 { $as_echo "$as_me:${as_lineno-$LINENO}: checking alignment of PG_INT128_TYPE" >&5
 $as_echo_n "checking alignment of PG_INT128_TYPE... " >&6; }
@@ -15071,6 +15128,7 @@ cat >>confdefs.h <<_ACEOF
 _ACEOF
 
 
+  fi
 fi
 
 # Check for various atomic operations now that we have checked how to declare

contrib/cube/cube.c

@@ -1337,15 +1337,72 @@ g_cube_distance(PG_FUNCTION_ARGS)
 
 	if (strategy == CubeKNNDistanceCoord)
 	{
+		/*
+		 * Handle ordering by ~> operator.  See comments of cube_coord_llur()
+		 * for details
+		 */
 		int			coord = PG_GETARG_INT32(1);
+		bool		isLeaf = GistPageIsLeaf(entry->page);
+		bool		inverse = false;
 
-		if (DIM(cube) == 0)
-			retval = 0.0;
-		else if (IS_POINT(cube))
-			retval = cube->x[(coord - 1) % DIM(cube)];
+		/* 0 is the only unsupported coordinate value */
+		if (coord == 0)
+			ereport(ERROR,
+					(errcode(ERRCODE_ARRAY_ELEMENT_ERROR),
+					 errmsg("zero cube index is not defined")));
+
+		/* Return inversed value for negative coordinate */
+		if (coord < 0)
+		{
+			coord = -coord;
+			inverse = true;
+		}
+
+		if (coord <= 2 * DIM(cube))
+		{
+			/* dimension index */
+			int		index = (coord - 1) / 2;
+			/* whether this is upper bound (lower bound otherwise) */
+			bool	upper = ((coord - 1) % 2 == 1);
+
+			if (IS_POINT(cube))
+			{
+				retval = cube->x[index];
+			}
+			else
+			{
+				if (isLeaf)
+				{
+					/* For leaf just return required upper/lower bound */
+					if (upper)
+						retval = Max(cube->x[index], cube->x[index + DIM(cube)]);
+					else
+						retval = Min(cube->x[index], cube->x[index + DIM(cube)]);
+				}
+				else
+				{
+					/*
+					 * For non-leaf we should always return lower bound,
+					 * because even upper bound of a child in the subtree can
+					 * be as small as our lower bound.  For inversed case we
+					 * return upper bound because it becomes lower bound for
+					 * inversed value.
+					 */
+					if (!inverse)
+						retval = Min(cube->x[index], cube->x[index + DIM(cube)]);
+					else
+						retval = Max(cube->x[index], cube->x[index + DIM(cube)]);
+				}
+			}
+		}
 		else
-			retval = Min(cube->x[(coord - 1) % DIM(cube)],
-						 cube->x[(coord - 1) % DIM(cube) + DIM(cube)]);
+		{
+			retval = 0.0;
+		}
+
+		/* Inverse return value if needed */
+		if (inverse)
+			retval = -retval;
 	}
 	else
 	{
@@ -1492,43 +1549,84 @@ cube_coord(PG_FUNCTION_ARGS)
 }
 
 
-/*
- * This function works like cube_coord(),
- * but rearranges coordinates of corners to get cube representation
- * in the form of (lower left, upper right).
- * For historical reasons that extension allows us to create cubes in form
- * ((2,1),(1,2)) and instead of normalizing such cube to ((1,1),(2,2)) it
- * stores cube in original way. But to get cubes ordered by one of dimensions
- * directly from the index without extra sort step we need some
- * representation-independent coordinate getter. This function implements it.
+/*----
+ * This function works like cube_coord(), but rearranges coordinates in the
+ * way suitable to support coordinate ordering using KNN-GiST.  For historical
+ * reasons this extension allows us to create cubes in form ((2,1),(1,2)) and
+ * instead of normalizing such cube to ((1,1),(2,2)) it stores cube in original
+ * way.  But in order to get cubes ordered by one of dimensions from the index
+ * without explicit sort step we need this representation-independent coordinate
+ * getter.  Moreover, indexed dataset may contain cubes of different dimensions
+ * number.  Accordingly, this coordinate getter should be able to return
+ * lower/upper bound for particular dimension independently on number of cube
+ * dimensions.  Also, KNN-GiST supports only ascending sorting.  In order to
+ * support descending sorting, this function returns inverse of value when
+ * negative coordinate is given.
+ *
+ * Long story short, this function uses following meaning of coordinates:
+ * # (2 * N - 1) -- lower bound of Nth dimension,
+ * # (2 * N) -- upper bound of Nth dimension,
+ * # - (2 * N - 1) -- negative of lower bound of Nth dimension,
+ * # - (2 * N) -- negative of upper bound of Nth dimension.
+ *
+ * When given coordinate exceeds number of cube dimensions, then 0 returned
+ * (reproducing logic of GiST indexing of variable-length cubes).
  */
 Datum
 cube_coord_llur(PG_FUNCTION_ARGS)
 {
 	NDBOX	   *cube = PG_GETARG_NDBOX_P(0);
 	int			coord = PG_GETARG_INT32(1);
+	bool		inverse = false;
+	float8		result;
 
-	if (coord <= 0 || coord > 2 * DIM(cube))
+	/* 0 is the only unsupported coordinate value */
+	if (coord == 0)
 		ereport(ERROR,
 				(errcode(ERRCODE_ARRAY_ELEMENT_ERROR),
-				 errmsg("cube index %d is out of bounds", coord)));
+				 errmsg("zero cube index is not defined")));
+
+	/* Return inversed value for negative coordinate */
+	if (coord < 0)
+	{
+		coord = -coord;
+		inverse = true;
+	}
 
-	if (coord <= DIM(cube))
+	if (coord <= 2 * DIM(cube))
 	{
+		/* dimension index */
+		int		index = (coord - 1) / 2;
+		/* whether this is upper bound (lower bound otherwise) */
+		bool	upper = ((coord - 1) % 2 == 1);
+
 		if (IS_POINT(cube))
-			PG_RETURN_FLOAT8(cube->x[coord - 1]);
+		{
+			result = cube->x[index];
+		}
 		else
-			PG_RETURN_FLOAT8(Min(cube->x[coord - 1],
-								 cube->x[coord - 1 + DIM(cube)]));
+		{
+			if (upper)
+				result = Max(cube->x[index], cube->x[index + DIM(cube)]);
+			else
+				result = Min(cube->x[index], cube->x[index + DIM(cube)]);
+		}
 	}
 	else
 	{
-		if (IS_POINT(cube))
-			PG_RETURN_FLOAT8(cube->x[(coord - 1) % DIM(cube)]);
-		else
-			PG_RETURN_FLOAT8(Max(cube->x[coord - 1],
-								 cube->x[coord - 1 - DIM(cube)]));
+		/*
+		 * Return zero if coordinate is out of bound.  That reproduces logic of
+		 * how cubes with low dimension number are expanded during GiST
+		 * indexing.
+		 */
+		result = 0.0;
 	}
+
+	/* Inverse value if needed */
+	if (inverse)
+		result = -result;
+
+	PG_RETURN_FLOAT8(result);
 }
 
 /* Increase or decrease box size by a radius in at least n dimensions. */