agora inbox for [email protected]  
help / color / mirror / Atom feed
[PATCH 1/6] Pass all scan keys to BRIN consistent function at once
8+ messages / 6 participants
[nested] [flat]

* [PATCH 1/6] Pass all scan keys to BRIN consistent function at once
@ 2020-09-12 13:07 Tomas Vondra <[email protected]>
  0 siblings, 0 replies; 8+ messages in thread

From: Tomas Vondra @ 2020-09-12 13:07 UTC (permalink / raw)

Passing all scan keys to the BRIN consistent function at once may allow
elimination of additional ranges, which would be impossible when only
passing individual scan keys.

The code continues to support both the original (one scan key at a time)
and new (all scan keys at once) approaches, depending on whether the
consistent function accepts three or four arguments.

This modifies the existing BRIN opclasses (minmax, inclusion) although
those don't really benefit from this change. The primary purpose of this
is to allow more advanced opclases in the future.

Author: Tomas Vondra <[email protected]>
Reviewed-by: Alvaro Herrera <[email protected]>
Reviewed-by: Mark Dilger <[email protected]>
Reviewed-by: Alexander Korotkov <[email protected]>
Discussion: https://postgr.es/m/[email protected]
---
 src/backend/access/brin/brin.c           | 150 +++++++++++++++-----
 src/backend/access/brin/brin_inclusion.c | 170 +++++++++++++++--------
 src/backend/access/brin/brin_minmax.c    | 121 +++++++++++-----
 src/backend/access/brin/brin_validate.c  |   4 +-
 src/include/catalog/pg_proc.dat          |   4 +-
 5 files changed, 324 insertions(+), 125 deletions(-)

diff --git a/src/backend/access/brin/brin.c b/src/backend/access/brin/brin.c
index 27ba596c6e..dc187153aa 100644
--- a/src/backend/access/brin/brin.c
+++ b/src/backend/access/brin/brin.c
@@ -390,6 +390,9 @@ bringetbitmap(IndexScanDesc scan, TIDBitmap *tbm)
 	BrinMemTuple *dtup;
 	BrinTuple  *btup = NULL;
 	Size		btupsz = 0;
+	ScanKey	  **keys;
+	int		   *nkeys;
+	int			keyno;
 
 	opaque = (BrinOpaque *) scan->opaque;
 	bdesc = opaque->bo_bdesc;
@@ -411,6 +414,61 @@ bringetbitmap(IndexScanDesc scan, TIDBitmap *tbm)
 	 */
 	consistentFn = palloc0(sizeof(FmgrInfo) * bdesc->bd_tupdesc->natts);
 
+	/*
+	 * Make room for per-attribute lists of scan keys that we'll pass to the
+	 * consistent support procedure.
+	 */
+	keys = palloc0(sizeof(ScanKey *) * bdesc->bd_tupdesc->natts);
+	nkeys = palloc0(sizeof(int) * bdesc->bd_tupdesc->natts);
+
+	/*
+	 * Preprocess the scan keys - split them into per-attribute arrays.
+	 */
+	for (keyno = 0; keyno < scan->numberOfKeys; keyno++)
+	{
+		ScanKey		key = &scan->keyData[keyno];
+		AttrNumber	keyattno = key->sk_attno;
+
+		/*
+		 * The collation of the scan key must match the collation
+		 * used in the index column (but only if the search is not
+		 * IS NULL/ IS NOT NULL).  Otherwise we shouldn't be using
+		 * this index ...
+		 */
+		Assert((key->sk_flags & SK_ISNULL) ||
+			   (key->sk_collation ==
+				TupleDescAttr(bdesc->bd_tupdesc,
+							  keyattno - 1)->attcollation));
+
+		/* First time we see this index attribute, so init as needed. */
+		if (!keys[keyattno-1])
+		{
+			FmgrInfo   *tmp;
+
+			/*
+			 * This is a bit of an overkill - we don't know how many
+			 * scan keys are there for this attribute, so we simply
+			 * allocate the largest number possible. This may waste
+			 * a bit of memory, but we only expect small number of
+			 * scan keys in general, so this should be negligible,
+			 * and it's cheaper than having to repalloc repeatedly.
+			 */
+			keys[keyattno - 1] = palloc0(sizeof(ScanKey) * scan->numberOfKeys);
+
+			/* First time this column, so look up consistent function */
+			Assert(consistentFn[keyattno - 1].fn_oid == InvalidOid);
+
+			tmp = index_getprocinfo(idxRel, keyattno,
+									BRIN_PROCNUM_CONSISTENT);
+			fmgr_info_copy(&consistentFn[keyattno - 1], tmp,
+						   CurrentMemoryContext);
+		}
+
+		/* Add key to the per-attribute array. */
+		keys[keyattno - 1][nkeys[keyattno - 1]] = key;
+		nkeys[keyattno - 1]++;
+	}
+
 	/* allocate an initial in-memory tuple, out of the per-range memcxt */
 	dtup = brin_new_memtuple(bdesc);
 
@@ -471,7 +529,7 @@ bringetbitmap(IndexScanDesc scan, TIDBitmap *tbm)
 			}
 			else
 			{
-				int			keyno;
+				int		attno;
 
 				/*
 				 * Compare scan keys with summary values stored for the range.
@@ -481,51 +539,75 @@ bringetbitmap(IndexScanDesc scan, TIDBitmap *tbm)
 				 * no keys.
 				 */
 				addrange = true;
-				for (keyno = 0; keyno < scan->numberOfKeys; keyno++)
+				for (attno = 1; attno <= bdesc->bd_tupdesc->natts; attno++)
 				{
-					ScanKey		key = &scan->keyData[keyno];
-					AttrNumber	keyattno = key->sk_attno;
-					BrinValues *bval = &dtup->bt_columns[keyattno - 1];
+					BrinValues *bval;
 					Datum		add;
 
-					/*
-					 * The collation of the scan key must match the collation
-					 * used in the index column (but only if the search is not
-					 * IS NULL/ IS NOT NULL).  Otherwise we shouldn't be using
-					 * this index ...
-					 */
-					Assert((key->sk_flags & SK_ISNULL) ||
-						   (key->sk_collation ==
-							TupleDescAttr(bdesc->bd_tupdesc,
-										  keyattno - 1)->attcollation));
+					/* skip attributes without any san keys */
+					if (nkeys[attno - 1] == 0)
+						continue;
 
-					/* First time this column? look up consistent function */
-					if (consistentFn[keyattno - 1].fn_oid == InvalidOid)
-					{
-						FmgrInfo   *tmp;
+					bval = &dtup->bt_columns[attno - 1];
 
-						tmp = index_getprocinfo(idxRel, keyattno,
-												BRIN_PROCNUM_CONSISTENT);
-						fmgr_info_copy(&consistentFn[keyattno - 1], tmp,
-									   CurrentMemoryContext);
-					}
+					Assert((nkeys[attno - 1] > 0) &&
+						   (nkeys[attno - 1] <= scan->numberOfKeys));
 
 					/*
 					 * Check whether the scan key is consistent with the page
 					 * range values; if so, have the pages in the range added
 					 * to the output bitmap.
 					 *
-					 * When there are multiple scan keys, failure to meet the
-					 * criteria for a single one of them is enough to discard
-					 * the range as a whole, so break out of the loop as soon
-					 * as a false return value is obtained.
+					 * The opclass may or may not support processing of multiple
+					 * scan keys. We can determine that based on the number of
+					 * arguments - functions with extra parameter (number of scan
+					 * keys) do support this, otherwise we have to simply pass the
+					 * scan keys one by one,
 					 */
-					add = FunctionCall3Coll(&consistentFn[keyattno - 1],
-											key->sk_collation,
-											PointerGetDatum(bdesc),
-											PointerGetDatum(bval),
-											PointerGetDatum(key));
-					addrange = DatumGetBool(add);
+					if (consistentFn[attno - 1].fn_nargs >= 4)
+					{
+						Oid			collation;
+
+						/*
+						 * Collation from the first key (has to be the same for
+						 * all keys for the same attribue).
+						 */
+						collation = keys[attno - 1][0]->sk_collation;
+
+						/* Check all keys at once */
+						add = FunctionCall4Coll(&consistentFn[attno - 1],
+												collation,
+												PointerGetDatum(bdesc),
+												PointerGetDatum(bval),
+												PointerGetDatum(keys[attno - 1]),
+												Int32GetDatum(nkeys[attno - 1]));
+						addrange = DatumGetBool(add);
+					}
+					else
+					{
+						/*
+						 * Check keys one by one
+						 *
+						 * When there are multiple scan keys, failure to meet the
+						 * criteria for a single one of them is enough to discard
+						 * the range as a whole, so break out of the loop as soon
+						 * as a false return value is obtained.
+						 */
+						int			keyno;
+
+						for (keyno = 0; keyno < nkeys[attno - 1]; keyno++)
+						{
+							add = FunctionCall3Coll(&consistentFn[attno - 1],
+													keys[attno - 1][keyno]->sk_collation,
+													PointerGetDatum(bdesc),
+													PointerGetDatum(bval),
+													PointerGetDatum(keys[attno - 1][keyno]));
+							addrange = DatumGetBool(add);
+							if (!addrange)
+								break;
+						}
+					}
+
 					if (!addrange)
 						break;
 				}
diff --git a/src/backend/access/brin/brin_inclusion.c b/src/backend/access/brin/brin_inclusion.c
index 12e5bddd1f..215bc794d3 100644
--- a/src/backend/access/brin/brin_inclusion.c
+++ b/src/backend/access/brin/brin_inclusion.c
@@ -85,6 +85,8 @@ static FmgrInfo *inclusion_get_procinfo(BrinDesc *bdesc, uint16 attno,
 										uint16 procnum);
 static FmgrInfo *inclusion_get_strategy_procinfo(BrinDesc *bdesc, uint16 attno,
 												 Oid subtype, uint16 strategynum);
+static bool inclusion_consistent_key(BrinDesc *bdesc, BrinValues *column,
+									 ScanKey key, Oid colloid);
 
 
 /*
@@ -258,53 +260,109 @@ brin_inclusion_consistent(PG_FUNCTION_ARGS)
 {
 	BrinDesc   *bdesc = (BrinDesc *) PG_GETARG_POINTER(0);
 	BrinValues *column = (BrinValues *) PG_GETARG_POINTER(1);
-	ScanKey		key = (ScanKey) PG_GETARG_POINTER(2);
-	Oid			colloid = PG_GET_COLLATION(),
-				subtype;
-	Datum		unionval;
-	AttrNumber	attno;
-	Datum		query;
-	FmgrInfo   *finfo;
-	Datum		result;
-
-	Assert(key->sk_attno == column->bv_attno);
+	ScanKey	   *keys = (ScanKey *) PG_GETARG_POINTER(2);
+	int			nkeys = PG_GETARG_INT32(3);
+	Oid			colloid = PG_GET_COLLATION();
+	int			keyno;
+	bool		regular_keys = false;
 
-	/* Handle IS NULL/IS NOT NULL tests. */
-	if (key->sk_flags & SK_ISNULL)
+	/*
+	 * First check if there are any IS NULL scan keys, and if we're
+	 * violating them. In that case we can terminate early, without
+	 * inspecting the ranges.
+	 */
+	for (keyno = 0; keyno < nkeys; keyno++)
 	{
-		if (key->sk_flags & SK_SEARCHNULL)
+		ScanKey	key = keys[keyno];
+
+		Assert(key->sk_attno == column->bv_attno);
+
+		/* handle IS NULL/IS NOT NULL tests */
+		if (key->sk_flags & SK_ISNULL)
 		{
-			if (column->bv_allnulls || column->bv_hasnulls)
-				PG_RETURN_BOOL(true);
-			PG_RETURN_BOOL(false);
-		}
+			if (key->sk_flags & SK_SEARCHNULL)
+			{
+				if (column->bv_allnulls || column->bv_hasnulls)
+					continue;	/* this key is fine, continue */
 
-		/*
-		 * For IS NOT NULL, we can only skip ranges that are known to have
-		 * only nulls.
-		 */
-		if (key->sk_flags & SK_SEARCHNOTNULL)
-			PG_RETURN_BOOL(!column->bv_allnulls);
+				PG_RETURN_BOOL(false);
+			}
 
-		/*
-		 * Neither IS NULL nor IS NOT NULL was used; assume all indexable
-		 * operators are strict and return false.
-		 */
-		PG_RETURN_BOOL(false);
+			/*
+			 * For IS NOT NULL, we can only skip ranges that are known to have
+			 * only nulls.
+			 */
+			if (key->sk_flags & SK_SEARCHNOTNULL)
+			{
+				if (column->bv_allnulls)
+					PG_RETURN_BOOL(false);
+
+				continue;
+			}
+
+			/*
+			 * Neither IS NULL nor IS NOT NULL was used; assume all indexable
+			 * operators are strict and return false.
+			 */
+			PG_RETURN_BOOL(false);
+		}
+		else
+			/* note we have regular (non-NULL) scan keys */
+			regular_keys = true;
 	}
 
-	/* If it is all nulls, it cannot possibly be consistent. */
-	if (column->bv_allnulls)
+	/*
+	 * If the page range is all nulls, it cannot possibly be consistent if
+	 * there are some regular scan keys.
+	 */
+	if (column->bv_allnulls && regular_keys)
 		PG_RETURN_BOOL(false);
 
+	/* If there are no regular keys, the page range is considered consistent. */
+	if (!regular_keys)
+		PG_RETURN_BOOL(true);
+
 	/* It has to be checked, if it contains elements that are not mergeable. */
 	if (DatumGetBool(column->bv_values[INCLUSION_UNMERGEABLE]))
 		PG_RETURN_BOOL(true);
 
-	attno = key->sk_attno;
-	subtype = key->sk_subtype;
-	query = key->sk_argument;
-	unionval = column->bv_values[INCLUSION_UNION];
+	/* Check that the range is consistent with all scan keys. */
+	for (keyno = 0; keyno < nkeys; keyno++)
+	{
+		ScanKey		key = keys[keyno];
+
+		/* ignore IS NULL/IS NOT NULL tests handled above */
+		if (key->sk_flags & SK_ISNULL)
+			continue;
+
+		/*
+		 * When there are multiple scan keys, failure to meet the
+		 * criteria for a single one of them is enough to discard
+		 * the range as a whole, so break out of the loop as soon
+		 * as a false return value is obtained.
+		 */
+		if (!inclusion_consistent_key(bdesc, column, key, colloid))
+			PG_RETURN_BOOL(false);
+	}
+
+	PG_RETURN_BOOL(true);
+}
+
+/*
+ * inclusion_consistent_key
+ *		Determine if the range is consistent with a single scan key.
+ */
+static bool
+inclusion_consistent_key(BrinDesc *bdesc, BrinValues *column, ScanKey key,
+						 Oid colloid)
+{
+	FmgrInfo   *finfo;
+	AttrNumber	attno = key->sk_attno;
+	Oid			subtype = key->sk_subtype;
+	Datum		query = key->sk_argument;
+	Datum		unionval = column->bv_values[INCLUSION_UNION];
+	Datum		result;
+
 	switch (key->sk_strategy)
 	{
 			/*
@@ -324,49 +382,49 @@ brin_inclusion_consistent(PG_FUNCTION_ARGS)
 			finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype,
 													RTOverRightStrategyNumber);
 			result = FunctionCall2Coll(finfo, colloid, unionval, query);
-			PG_RETURN_BOOL(!DatumGetBool(result));
+			return !DatumGetBool(result);
 
 		case RTOverLeftStrategyNumber:
 			finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype,
 													RTRightStrategyNumber);
 			result = FunctionCall2Coll(finfo, colloid, unionval, query);
-			PG_RETURN_BOOL(!DatumGetBool(result));
+			return !DatumGetBool(result);
 
 		case RTOverRightStrategyNumber:
 			finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype,
 													RTLeftStrategyNumber);
 			result = FunctionCall2Coll(finfo, colloid, unionval, query);
-			PG_RETURN_BOOL(!DatumGetBool(result));
+			return !DatumGetBool(result);
 
 		case RTRightStrategyNumber:
 			finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype,
 													RTOverLeftStrategyNumber);
 			result = FunctionCall2Coll(finfo, colloid, unionval, query);
-			PG_RETURN_BOOL(!DatumGetBool(result));
+			return !DatumGetBool(result);
 
 		case RTBelowStrategyNumber:
 			finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype,
 													RTOverAboveStrategyNumber);
 			result = FunctionCall2Coll(finfo, colloid, unionval, query);
-			PG_RETURN_BOOL(!DatumGetBool(result));
+			return !DatumGetBool(result);
 
 		case RTOverBelowStrategyNumber:
 			finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype,
 													RTAboveStrategyNumber);
 			result = FunctionCall2Coll(finfo, colloid, unionval, query);
-			PG_RETURN_BOOL(!DatumGetBool(result));
+			return !DatumGetBool(result);
 
 		case RTOverAboveStrategyNumber:
 			finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype,
 													RTBelowStrategyNumber);
 			result = FunctionCall2Coll(finfo, colloid, unionval, query);
-			PG_RETURN_BOOL(!DatumGetBool(result));
+			return !DatumGetBool(result);
 
 		case RTAboveStrategyNumber:
 			finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype,
 													RTOverBelowStrategyNumber);
 			result = FunctionCall2Coll(finfo, colloid, unionval, query);
-			PG_RETURN_BOOL(!DatumGetBool(result));
+			return !DatumGetBool(result);
 
 			/*
 			 * Overlap and contains strategies
@@ -384,7 +442,7 @@ brin_inclusion_consistent(PG_FUNCTION_ARGS)
 			finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype,
 													key->sk_strategy);
 			result = FunctionCall2Coll(finfo, colloid, unionval, query);
-			PG_RETURN_DATUM(result);
+			return DatumGetBool(result);
 
 			/*
 			 * Contained by strategies
@@ -404,9 +462,9 @@ brin_inclusion_consistent(PG_FUNCTION_ARGS)
 													RTOverlapStrategyNumber);
 			result = FunctionCall2Coll(finfo, colloid, unionval, query);
 			if (DatumGetBool(result))
-				PG_RETURN_BOOL(true);
+				return true;
 
-			PG_RETURN_DATUM(column->bv_values[INCLUSION_CONTAINS_EMPTY]);
+			return DatumGetBool(column->bv_values[INCLUSION_CONTAINS_EMPTY]);
 
 			/*
 			 * Adjacent strategy
@@ -423,12 +481,12 @@ brin_inclusion_consistent(PG_FUNCTION_ARGS)
 													RTOverlapStrategyNumber);
 			result = FunctionCall2Coll(finfo, colloid, unionval, query);
 			if (DatumGetBool(result))
-				PG_RETURN_BOOL(true);
+				return true;
 
 			finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype,
-													RTAdjacentStrategyNumber);
+														RTAdjacentStrategyNumber);
 			result = FunctionCall2Coll(finfo, colloid, unionval, query);
-			PG_RETURN_DATUM(result);
+			return DatumGetBool(result);
 
 			/*
 			 * Basic comparison strategies
@@ -458,9 +516,9 @@ brin_inclusion_consistent(PG_FUNCTION_ARGS)
 													RTRightStrategyNumber);
 			result = FunctionCall2Coll(finfo, colloid, unionval, query);
 			if (!DatumGetBool(result))
-				PG_RETURN_BOOL(true);
+				return true;
 
-			PG_RETURN_DATUM(column->bv_values[INCLUSION_CONTAINS_EMPTY]);
+			return DatumGetBool(column->bv_values[INCLUSION_CONTAINS_EMPTY]);
 
 		case RTSameStrategyNumber:
 		case RTEqualStrategyNumber:
@@ -468,30 +526,30 @@ brin_inclusion_consistent(PG_FUNCTION_ARGS)
 													RTContainsStrategyNumber);
 			result = FunctionCall2Coll(finfo, colloid, unionval, query);
 			if (DatumGetBool(result))
-				PG_RETURN_BOOL(true);
+				return true;
 
-			PG_RETURN_DATUM(column->bv_values[INCLUSION_CONTAINS_EMPTY]);
+			return DatumGetBool(column->bv_values[INCLUSION_CONTAINS_EMPTY]);
 
 		case RTGreaterEqualStrategyNumber:
 			finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype,
 													RTLeftStrategyNumber);
 			result = FunctionCall2Coll(finfo, colloid, unionval, query);
 			if (!DatumGetBool(result))
-				PG_RETURN_BOOL(true);
+				return true;
 
-			PG_RETURN_DATUM(column->bv_values[INCLUSION_CONTAINS_EMPTY]);
+			return DatumGetBool(column->bv_values[INCLUSION_CONTAINS_EMPTY]);
 
 		case RTGreaterStrategyNumber:
 			/* no need to check for empty elements */
 			finfo = inclusion_get_strategy_procinfo(bdesc, attno, subtype,
 													RTLeftStrategyNumber);
 			result = FunctionCall2Coll(finfo, colloid, unionval, query);
-			PG_RETURN_BOOL(!DatumGetBool(result));
+			return !DatumGetBool(result);
 
 		default:
 			/* shouldn't happen */
 			elog(ERROR, "invalid strategy number %d", key->sk_strategy);
-			PG_RETURN_BOOL(false);
+			return false;
 	}
 }
 
diff --git a/src/backend/access/brin/brin_minmax.c b/src/backend/access/brin/brin_minmax.c
index 2ffbd9bf0d..12878ff3a0 100644
--- a/src/backend/access/brin/brin_minmax.c
+++ b/src/backend/access/brin/brin_minmax.c
@@ -30,6 +30,8 @@ typedef struct MinmaxOpaque
 
 static FmgrInfo *minmax_get_strategy_procinfo(BrinDesc *bdesc, uint16 attno,
 											  Oid subtype, uint16 strategynum);
+static bool minmax_consistent_key(BrinDesc *bdesc, BrinValues *column,
+								  ScanKey key, Oid colloid);
 
 
 Datum
@@ -146,47 +148,104 @@ brin_minmax_consistent(PG_FUNCTION_ARGS)
 {
 	BrinDesc   *bdesc = (BrinDesc *) PG_GETARG_POINTER(0);
 	BrinValues *column = (BrinValues *) PG_GETARG_POINTER(1);
-	ScanKey		key = (ScanKey) PG_GETARG_POINTER(2);
-	Oid			colloid = PG_GET_COLLATION(),
-				subtype;
-	AttrNumber	attno;
-	Datum		value;
-	Datum		matches;
-	FmgrInfo   *finfo;
-
-	Assert(key->sk_attno == column->bv_attno);
+	ScanKey	   *keys = (ScanKey *) PG_GETARG_POINTER(2);
+	int			nkeys = PG_GETARG_INT32(3);
+	Oid			colloid = PG_GET_COLLATION();
+	int			keyno;
+	bool		regular_keys = false;
 
-	/* handle IS NULL/IS NOT NULL tests */
-	if (key->sk_flags & SK_ISNULL)
+	/*
+	 * First check if there are any IS NULL scan keys, and if we're
+	 * violating them. In that case we can terminate early, without
+	 * inspecting the ranges.
+	 */
+	for (keyno = 0; keyno < nkeys; keyno++)
 	{
-		if (key->sk_flags & SK_SEARCHNULL)
+		ScanKey	key = keys[keyno];
+
+		Assert(key->sk_attno == column->bv_attno);
+
+		/* handle IS NULL/IS NOT NULL tests */
+		if (key->sk_flags & SK_ISNULL)
 		{
-			if (column->bv_allnulls || column->bv_hasnulls)
-				PG_RETURN_BOOL(true);
+			if (key->sk_flags & SK_SEARCHNULL)
+			{
+				if (column->bv_allnulls || column->bv_hasnulls)
+					continue;	/* this key is fine, continue */
+
+				PG_RETURN_BOOL(false);
+			}
+
+			/*
+			 * For IS NOT NULL, we can only skip ranges that are known to have
+			 * only nulls.
+			 */
+			if (key->sk_flags & SK_SEARCHNOTNULL)
+			{
+				if (column->bv_allnulls)
+					PG_RETURN_BOOL(false);
+
+				continue;
+			}
+
+			/*
+			 * Neither IS NULL nor IS NOT NULL was used; assume all indexable
+			 * operators are strict and return false.
+			 */
 			PG_RETURN_BOOL(false);
 		}
+		else
+			/* note we have regular (non-NULL) scan keys */
+			regular_keys = true;
+	}
 
-		/*
-		 * For IS NOT NULL, we can only skip ranges that are known to have
-		 * only nulls.
-		 */
-		if (key->sk_flags & SK_SEARCHNOTNULL)
-			PG_RETURN_BOOL(!column->bv_allnulls);
+	/*
+	 * If the page range is all nulls, it cannot possibly be consistent if
+	 * there are some regular scan keys.
+	 */
+	if (column->bv_allnulls && regular_keys)
+		PG_RETURN_BOOL(false);
+
+	/* If there are no regular keys, the page range is considered consistent. */
+	if (!regular_keys)
+		PG_RETURN_BOOL(true);
 
-		/*
-		 * Neither IS NULL nor IS NOT NULL was used; assume all indexable
-		 * operators are strict and return false.
+	/* Check that the range is consistent with all scan keys. */
+	for (keyno = 0; keyno < nkeys; keyno++)
+	{
+		ScanKey	key = keys[keyno];
+
+		/* ignore IS NULL/IS NOT NULL tests handled above */
+		if (key->sk_flags & SK_ISNULL)
+			continue;
+
+		 /*
+		 * When there are multiple scan keys, failure to meet the
+		 * criteria for a single one of them is enough to discard
+		 * the range as a whole, so break out of the loop as soon
+		 * as a false return value is obtained.
 		 */
-		PG_RETURN_BOOL(false);
+		if (!minmax_consistent_key(bdesc, column, key, colloid))
+			PG_RETURN_DATUM(false);;
 	}
 
-	/* if the range is all empty, it cannot possibly be consistent */
-	if (column->bv_allnulls)
-		PG_RETURN_BOOL(false);
+	PG_RETURN_DATUM(true);
+}
+
+/*
+ * minmax_consistent_key
+ *		Determine if the range is consistent with a single scan key.
+ */
+static bool
+minmax_consistent_key(BrinDesc *bdesc, BrinValues *column, ScanKey key,
+					  Oid colloid)
+{
+	FmgrInfo   *finfo;
+	AttrNumber	attno = key->sk_attno;
+	Oid			subtype = key->sk_subtype;
+	Datum		value = key->sk_argument;
+	Datum		matches;
 
-	attno = key->sk_attno;
-	subtype = key->sk_subtype;
-	value = key->sk_argument;
 	switch (key->sk_strategy)
 	{
 		case BTLessStrategyNumber:
@@ -229,7 +288,7 @@ brin_minmax_consistent(PG_FUNCTION_ARGS)
 			break;
 	}
 
-	PG_RETURN_DATUM(matches);
+	return DatumGetBool(matches);
 }
 
 /*
diff --git a/src/backend/access/brin/brin_validate.c b/src/backend/access/brin/brin_validate.c
index 6d4253c05e..11835d85cd 100644
--- a/src/backend/access/brin/brin_validate.c
+++ b/src/backend/access/brin/brin_validate.c
@@ -97,8 +97,8 @@ brinvalidate(Oid opclassoid)
 				break;
 			case BRIN_PROCNUM_CONSISTENT:
 				ok = check_amproc_signature(procform->amproc, BOOLOID, true,
-											3, 3, INTERNALOID, INTERNALOID,
-											INTERNALOID);
+											3, 4, INTERNALOID, INTERNALOID,
+											INTERNALOID, INT4OID);
 				break;
 			case BRIN_PROCNUM_UNION:
 				ok = check_amproc_signature(procform->amproc, BOOLOID, true,
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index d27336adcd..f3ead7d53d 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -8145,7 +8145,7 @@
   prosrc => 'brin_minmax_add_value' },
 { oid => '3385', descr => 'BRIN minmax support',
   proname => 'brin_minmax_consistent', prorettype => 'bool',
-  proargtypes => 'internal internal internal',
+  proargtypes => 'internal internal internal int4',
   prosrc => 'brin_minmax_consistent' },
 { oid => '3386', descr => 'BRIN minmax support',
   proname => 'brin_minmax_union', prorettype => 'bool',
@@ -8161,7 +8161,7 @@
   prosrc => 'brin_inclusion_add_value' },
 { oid => '4107', descr => 'BRIN inclusion support',
   proname => 'brin_inclusion_consistent', prorettype => 'bool',
-  proargtypes => 'internal internal internal',
+  proargtypes => 'internal internal internal int4',
   prosrc => 'brin_inclusion_consistent' },
 { oid => '4108', descr => 'BRIN inclusion support',
   proname => 'brin_inclusion_union', prorettype => 'bool',
-- 
2.26.2


--------------310A2AE1CC4C2E2E77559E3D
Content-Type: text/x-patch; charset=UTF-8;
 name="0002-Move-IS-NOT-NULL-handling-from-BRIN-suppo-20210114-2.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
 filename*0="0002-Move-IS-NOT-NULL-handling-from-BRIN-suppo-20210114-2.pa";
 filename*1="tch"



^ permalink  raw  reply  [nested|flat] 8+ messages in thread

* Re: Error for WITH options on partitioned tables
@ 2022-10-14 15:16 Karina Litskevich <[email protected]>
  2022-10-28 21:21 ` Re: Error for WITH options on partitioned tables David Zhang <[email protected]>
  0 siblings, 1 reply; 8+ messages in thread

From: Karina Litskevich @ 2022-10-14 15:16 UTC (permalink / raw)
  To: Simon Riggs <[email protected]>; Japin Li <[email protected]>; pgsql-hackers; Postgres hackers <[email protected]>

Hi, Simon!

The new error message looks better. But I believe it is better to use
"parameters" instead of "options" as it is called "storage parameters"
in the documentation. I also believe it is better to report error in
partitioned_table_reloptions() (thanks to Japin Li for mentioning this
function) as it also fixes the error message in the following situation:

test=# CREATE TABLE parted_col_comment (a int, b text) PARTITION BY LIST
(a);
CREATE TABLE
test=# ALTER TABLE parted_col_comment SET (fillfactor=100);
ERROR:  unrecognized parameter "fillfactor"

I attached the new versions of patches.

I'm not sure about the errcode. May be it is better to report error with
ERRCODE_INVALID_OBJECT_DEFINITION for CREATE TABLE and with
ERRCODE_WRONG_OBJECT_TYPE for ALTER TABLE (as when you try "ALTER TABLE
partitioned INHERIT nonpartitioned;" an ERROR with ERRCODE_WRONG_OBJECT_TYPE
is reported). Then either the desired code should be passed to
partitioned_table_reloptions() or similar checks and ereport calls should be
placed in two different places. I'm not sure whether it is a good idea to
change the code in one of these ways just to change the error code.

Best regards,
Karina Litskevich
Postgres Professional: http://postgrespro.com/


Attachments:

  [text/x-patch] v2-0002-better-error-message-for-setting-parameters-for-p.patch (2.9K, ../../CACiT8iaj5o8mdros-E6FDsZPjM-RDefbOowu7dcrnN+iOft_LQ@mail.gmail.com/3-v2-0002-better-error-message-for-setting-parameters-for-p.patch)
  download | inline diff:
From 6407538f0bfd3eb56f5a4574d8893f9494f2a810 Mon Sep 17 00:00:00 2001
From: Karina Litskevich <[email protected]>
Date: Fri, 14 Oct 2022 17:48:27 +0300
Subject: [PATCH v2 2/2] better error message for setting parameters for
 partitioned table

---
 src/backend/access/common/reloptions.c     | 13 ++++++-------
 src/test/regress/expected/alter_table.out  |  3 ++-
 src/test/regress/expected/create_table.out |  3 ++-
 3 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/src/backend/access/common/reloptions.c b/src/backend/access/common/reloptions.c
index 6458a9c276..75d6ff5040 100644
--- a/src/backend/access/common/reloptions.c
+++ b/src/backend/access/common/reloptions.c
@@ -1984,13 +1984,12 @@ build_local_reloptions(local_relopts *relopts, Datum options, bool validate)
 bytea *
 partitioned_table_reloptions(Datum reloptions, bool validate)
 {
-	/*
-	 * There are no options for partitioned tables yet, but this is able to do
-	 * some validation.
-	 */
-	return (bytea *) build_reloptions(reloptions, validate,
-									  RELOPT_KIND_PARTITIONED,
-									  0, NULL, 0);
+	if (validate && reloptions)
+		ereport(ERROR,
+				errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+				errmsg("cannot specify storage parameters for a partitioned table"),
+				errhint("specify storage parameters on leaf partitions instead"));
+	return NULL;
 }
 
 /*
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index cec7bfa1f1..a719ef22c7 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -3803,7 +3803,8 @@ ALTER TABLE partitioned ALTER COLUMN b TYPE char(5);
 ERROR:  cannot alter column "b" because it is part of the partition key of relation "partitioned"
 -- specifying storage parameters for partitioned tables is not supported
 ALTER TABLE partitioned SET (fillfactor=100);
-ERROR:  unrecognized parameter "fillfactor"
+ERROR:  cannot specify storage parameters for a partitioned table
+HINT:  specify storage parameters on leaf partitions instead
 -- partitioned table cannot participate in regular inheritance
 CREATE TABLE nonpartitioned (
 	a int,
diff --git a/src/test/regress/expected/create_table.out b/src/test/regress/expected/create_table.out
index 45c1a0a23e..f16be87729 100644
--- a/src/test/regress/expected/create_table.out
+++ b/src/test/regress/expected/create_table.out
@@ -989,7 +989,8 @@ Number of partitions: 0
 DROP TABLE parted_col_comment;
 -- specifying storage parameters for partitioned tables is not supported
 CREATE TABLE parted_col_comment (a int, b text) PARTITION BY LIST (a) WITH (fillfactor=100);
-ERROR:  unrecognized parameter "fillfactor"
+ERROR:  cannot specify storage parameters for a partitioned table
+HINT:  specify storage parameters on leaf partitions instead
 -- list partitioning on array type column
 CREATE TABLE arrlp (a int[]) PARTITION BY LIST (a);
 CREATE TABLE arrlp12 PARTITION OF arrlp FOR VALUES IN ('{1}', '{2}');
-- 
2.25.1



  [text/x-patch] v2-0001-test-parameters-for-partitioned-table.patch (3.2K, ../../CACiT8iaj5o8mdros-E6FDsZPjM-RDefbOowu7dcrnN+iOft_LQ@mail.gmail.com/4-v2-0001-test-parameters-for-partitioned-table.patch)
  download | inline diff:
From 580566815bc5abd6193f86ddbd55fac1ac941873 Mon Sep 17 00:00:00 2001
From: Karina Litskevich <[email protected]>
Date: Fri, 14 Oct 2022 16:22:57 +0300
Subject: [PATCH v2 1/2] test parameters for partitioned table

---
 src/test/regress/expected/alter_table.out  | 3 +++
 src/test/regress/expected/create_table.out | 3 +++
 src/test/regress/sql/alter_table.sql       | 3 +++
 src/test/regress/sql/create_table.sql      | 3 +++
 4 files changed, 12 insertions(+)

diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index 346f594ad0..cec7bfa1f1 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -3801,6 +3801,9 @@ ALTER TABLE partitioned DROP COLUMN b;
 ERROR:  cannot drop column "b" because it is part of the partition key of relation "partitioned"
 ALTER TABLE partitioned ALTER COLUMN b TYPE char(5);
 ERROR:  cannot alter column "b" because it is part of the partition key of relation "partitioned"
+-- specifying storage parameters for partitioned tables is not supported
+ALTER TABLE partitioned SET (fillfactor=100);
+ERROR:  unrecognized parameter "fillfactor"
 -- partitioned table cannot participate in regular inheritance
 CREATE TABLE nonpartitioned (
 	a int,
diff --git a/src/test/regress/expected/create_table.out b/src/test/regress/expected/create_table.out
index 4407a017a9..45c1a0a23e 100644
--- a/src/test/regress/expected/create_table.out
+++ b/src/test/regress/expected/create_table.out
@@ -987,6 +987,9 @@ Partition key: LIST (a)
 Number of partitions: 0
 
 DROP TABLE parted_col_comment;
+-- specifying storage parameters for partitioned tables is not supported
+CREATE TABLE parted_col_comment (a int, b text) PARTITION BY LIST (a) WITH (fillfactor=100);
+ERROR:  unrecognized parameter "fillfactor"
 -- list partitioning on array type column
 CREATE TABLE arrlp (a int[]) PARTITION BY LIST (a);
 CREATE TABLE arrlp12 PARTITION OF arrlp FOR VALUES IN ('{1}', '{2}');
diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql
index 9f773aeeb9..5a71961f02 100644
--- a/src/test/regress/sql/alter_table.sql
+++ b/src/test/regress/sql/alter_table.sql
@@ -2325,6 +2325,9 @@ ALTER TABLE partitioned ALTER COLUMN a TYPE char(5);
 ALTER TABLE partitioned DROP COLUMN b;
 ALTER TABLE partitioned ALTER COLUMN b TYPE char(5);
 
+-- specifying storage parameters for partitioned tables is not supported
+ALTER TABLE partitioned SET (fillfactor=100);
+
 -- partitioned table cannot participate in regular inheritance
 CREATE TABLE nonpartitioned (
 	a int,
diff --git a/src/test/regress/sql/create_table.sql b/src/test/regress/sql/create_table.sql
index 5175f404f7..93ccf77d4a 100644
--- a/src/test/regress/sql/create_table.sql
+++ b/src/test/regress/sql/create_table.sql
@@ -652,6 +652,9 @@ SELECT obj_description('parted_col_comment'::regclass);
 \d+ parted_col_comment
 DROP TABLE parted_col_comment;
 
+-- specifying storage parameters for partitioned tables is not supported
+CREATE TABLE parted_col_comment (a int, b text) PARTITION BY LIST (a) WITH (fillfactor=100);
+
 -- list partitioning on array type column
 CREATE TABLE arrlp (a int[]) PARTITION BY LIST (a);
 CREATE TABLE arrlp12 PARTITION OF arrlp FOR VALUES IN ('{1}', '{2}');
-- 
2.25.1



^ permalink  raw  reply  [nested|flat] 8+ messages in thread

* Re: Error for WITH options on partitioned tables
  2022-10-14 15:16 Re: Error for WITH options on partitioned tables Karina Litskevich <[email protected]>
@ 2022-10-28 21:21 ` David Zhang <[email protected]>
  2022-11-01 23:58   ` Re: Error for WITH options on partitioned tables Simon Riggs <[email protected]>
  2022-11-07 08:55   ` Re: Error for WITH options on partitioned tables Karina Litskevich <[email protected]>
  0 siblings, 2 replies; 8+ messages in thread

From: David Zhang @ 2022-10-28 21:21 UTC (permalink / raw)
  To: Karina Litskevich <[email protected]>; Simon Riggs <[email protected]>; Japin Li <[email protected]>; pgsql-hackers; Postgres hackers <[email protected]>

Hi Karina,

I am not very clear about why `build_reloptions` is removed in patch 
`v2-0002-better-error-message-for-setting-parameters-for-p.patch`, if 
you can help explain would be great.

 From my observation, it seems the WITH option has different behavior 
when creating partitioned table and index. For example,

pgbench -i --partitions=2 --partition-method=range -d postgres

postgres=# create index idx_bid on pgbench_accounts using btree(bid) 
with (fillfactor = 90);
CREATE INDEX

postgres=# select relname, relkind, reloptions from pg_class where 
relnamespace=2200 order by oid;
           relname           | relkind |    reloptions
----------------------------+---------+------------------
  idx_bid                    | I       | {fillfactor=90}
  pgbench_accounts_1_bid_idx | i       | {fillfactor=90}
  pgbench_accounts_2_bid_idx | i       | {fillfactor=90}

I can see the `fillfactor` parameter has been added to the indexes, 
however, if I try to alter `fillfactor`, I got an error like below.
postgres=# alter index idx_bid set (fillfactor=40);
ERROR:  ALTER action SET cannot be performed on relation "idx_bid"
DETAIL:  This operation is not supported for partitioned indexes.

This generic error message is reported by 
`errdetail_relkind_not_supported`, and there is a similar error message 
for partitioned tables. Anyone knows if this can be an option for 
reporting this `fillfactor` parameter not supported error.


Best regards,

David

On 2022-10-14 8:16 a.m., Karina Litskevich wrote:
> Hi, Simon!
>
> The new error message looks better. But I believe it is better to use
> "parameters" instead of "options" as it is called "storage parameters"
> in the documentation. I also believe it is better to report error in
> partitioned_table_reloptions() (thanks to Japin Li for mentioning this
> function) as it also fixes the error message in the following situation:
>
> test=# CREATE TABLE parted_col_comment (a int, b text) PARTITION BY 
> LIST (a);
> CREATE TABLE
> test=# ALTER TABLE parted_col_comment SET (fillfactor=100);
> ERROR:  unrecognized parameter "fillfactor"
>
> I attached the new versions of patches.
>
> I'm not sure about the errcode. May be it is better to report error with
> ERRCODE_INVALID_OBJECT_DEFINITION for CREATE TABLE and with
> ERRCODE_WRONG_OBJECT_TYPE for ALTER TABLE (as when you try "ALTER TABLE
> partitioned INHERIT nonpartitioned;" an ERROR with 
> ERRCODE_WRONG_OBJECT_TYPE
> is reported). Then either the desired code should be passed to
> partitioned_table_reloptions() or similar checks and ereport calls 
> should be
> placed in two different places. I'm not sure whether it is a good idea to
> change the code in one of these ways just to change the error code.
>
> Best regards,
> Karina Litskevich
> Postgres Professional: http://postgrespro.com/






^ permalink  raw  reply  [nested|flat] 8+ messages in thread

* Re: Error for WITH options on partitioned tables
  2022-10-14 15:16 Re: Error for WITH options on partitioned tables Karina Litskevich <[email protected]>
  2022-10-28 21:21 ` Re: Error for WITH options on partitioned tables David Zhang <[email protected]>
@ 2022-11-01 23:58   ` Simon Riggs <[email protected]>
  1 sibling, 0 replies; 8+ messages in thread

From: Simon Riggs @ 2022-11-01 23:58 UTC (permalink / raw)
  To: David Zhang <[email protected]>; +Cc: Karina Litskevich <[email protected]>; Japin Li <[email protected]>; pgsql-hackers; Postgres hackers <[email protected]>

Apologies, I only just noticed these messages. I have set WoA until I
have read, understood and can respond to your detailed input, thanks.

On Fri, 28 Oct 2022 at 22:21, David Zhang <[email protected]> wrote:
>
> Hi Karina,
>
> I am not very clear about why `build_reloptions` is removed in patch
> `v2-0002-better-error-message-for-setting-parameters-for-p.patch`, if
> you can help explain would be great.
>
>  From my observation, it seems the WITH option has different behavior
> when creating partitioned table and index. For example,
>
> pgbench -i --partitions=2 --partition-method=range -d postgres
>
> postgres=# create index idx_bid on pgbench_accounts using btree(bid)
> with (fillfactor = 90);
> CREATE INDEX
>
> postgres=# select relname, relkind, reloptions from pg_class where
> relnamespace=2200 order by oid;
>            relname           | relkind |    reloptions
> ----------------------------+---------+------------------
>   idx_bid                    | I       | {fillfactor=90}
>   pgbench_accounts_1_bid_idx | i       | {fillfactor=90}
>   pgbench_accounts_2_bid_idx | i       | {fillfactor=90}
>
> I can see the `fillfactor` parameter has been added to the indexes,
> however, if I try to alter `fillfactor`, I got an error like below.
> postgres=# alter index idx_bid set (fillfactor=40);
> ERROR:  ALTER action SET cannot be performed on relation "idx_bid"
> DETAIL:  This operation is not supported for partitioned indexes.
>
> This generic error message is reported by
> `errdetail_relkind_not_supported`, and there is a similar error message
> for partitioned tables. Anyone knows if this can be an option for
> reporting this `fillfactor` parameter not supported error.
>
>
> Best regards,
>
> David
>
> On 2022-10-14 8:16 a.m., Karina Litskevich wrote:
> > Hi, Simon!
> >
> > The new error message looks better. But I believe it is better to use
> > "parameters" instead of "options" as it is called "storage parameters"
> > in the documentation. I also believe it is better to report error in
> > partitioned_table_reloptions() (thanks to Japin Li for mentioning this
> > function) as it also fixes the error message in the following situation:
> >
> > test=# CREATE TABLE parted_col_comment (a int, b text) PARTITION BY
> > LIST (a);
> > CREATE TABLE
> > test=# ALTER TABLE parted_col_comment SET (fillfactor=100);
> > ERROR:  unrecognized parameter "fillfactor"
> >
> > I attached the new versions of patches.
> >
> > I'm not sure about the errcode. May be it is better to report error with
> > ERRCODE_INVALID_OBJECT_DEFINITION for CREATE TABLE and with
> > ERRCODE_WRONG_OBJECT_TYPE for ALTER TABLE (as when you try "ALTER TABLE
> > partitioned INHERIT nonpartitioned;" an ERROR with
> > ERRCODE_WRONG_OBJECT_TYPE
> > is reported). Then either the desired code should be passed to
> > partitioned_table_reloptions() or similar checks and ereport calls
> > should be
> > placed in two different places. I'm not sure whether it is a good idea to
> > change the code in one of these ways just to change the error code.
> >
> > Best regards,
> > Karina Litskevich
> > Postgres Professional: http://postgrespro.com/
>


-- 
Simon Riggs                http://www.EnterpriseDB.com/





^ permalink  raw  reply  [nested|flat] 8+ messages in thread

* Re: Error for WITH options on partitioned tables
  2022-10-14 15:16 Re: Error for WITH options on partitioned tables Karina Litskevich <[email protected]>
  2022-10-28 21:21 ` Re: Error for WITH options on partitioned tables David Zhang <[email protected]>
@ 2022-11-07 08:55   ` Karina Litskevich <[email protected]>
  2022-11-07 11:10     ` Re: Error for WITH options on partitioned tables Simon Riggs <[email protected]>
  1 sibling, 1 reply; 8+ messages in thread

From: Karina Litskevich @ 2022-11-07 08:55 UTC (permalink / raw)
  To: David Zhang <[email protected]>; +Cc: Simon Riggs <[email protected]>; Japin Li <[email protected]>; pgsql-hackers; Postgres hackers <[email protected]>

Hi David,

> I am not very clear about why `build_reloptions` is removed in patch
> `v2-0002-better-error-message-for-setting-parameters-for-p.patch`, if
> you can help explain would be great.

"build_reloptions" parses "reloptions" and takes for it a list of allowed
options defined by the 5th argument "relopt_elems" and the 6th argument
"num_relopt_elems", which are NULL and 0 in the removed call. If "validate"
is false, it ignores options, which are not in the list, while parsing. If
"validate" is true, it "elog"s ERROR when it meets option, which is not in
the
allowed list.

As in the deleted call "build_reloptions" is called with an empty list of
allowed options, it does nothing (returns NULL) when "validate" is false,
and
"elog"s ERROR when "validate" is true and "reloptions" is not empty. That is
what the deleted comment above the deleted call about. This call is there
only
for validation. So as I wanted to make a specific error message for the
case of
partitioned tables, I added the validation in "partitioned_table_reloptions"
and saw no reason to call "build_reloptions" any more because it would just
return NULL in other cases.

> This generic error message is reported by
> `errdetail_relkind_not_supported`, and there is a similar error message
> for partitioned tables. Anyone knows if this can be an option for
> reporting this `fillfactor` parameter not supported error.

This error is reported by "ATSimplePermissions" and, as we can see in the
beginning of this function, it makes no difference between regular relations
and partitioned tables now. To make it report errors for partitioned tables
we
should add new "alter table target-type flag" and add it to a mask of each
"AlterTableType" if partitioned table is an allowed target for it (see that
huge switch-case in function "ATPrepCmd"). Then
"partitioned_table_reloptions"
may become useless and we also should check weather some other functions
become
useless. Maybe that is the right way, but it looks much harder than the
existing solutions, so I believe, before anyone began going this way, it's
better to know whether there are any pitfalls there.

Best regards,
Karina Litskevich
Postgres Professional: http://postgrespro.com/


^ permalink  raw  reply  [nested|flat] 8+ messages in thread

* Re: Error for WITH options on partitioned tables
  2022-10-14 15:16 Re: Error for WITH options on partitioned tables Karina Litskevich <[email protected]>
  2022-10-28 21:21 ` Re: Error for WITH options on partitioned tables David Zhang <[email protected]>
  2022-11-07 08:55   ` Re: Error for WITH options on partitioned tables Karina Litskevich <[email protected]>
@ 2022-11-07 11:10     ` Simon Riggs <[email protected]>
  2022-11-09 17:33       ` Re: Error for WITH options on partitioned tables Tom Lane <[email protected]>
  0 siblings, 1 reply; 8+ messages in thread

From: Simon Riggs @ 2022-11-07 11:10 UTC (permalink / raw)
  To: Karina Litskevich <[email protected]>; +Cc: David Zhang <[email protected]>; Japin Li <[email protected]>; pgsql-hackers; Postgres hackers <[email protected]>

On Mon, 7 Nov 2022 at 08:55, Karina Litskevich
<[email protected]> wrote:
>
> Hi David,
>
> > I am not very clear about why `build_reloptions` is removed in patch
> > `v2-0002-better-error-message-for-setting-parameters-for-p.patch`, if
> > you can help explain would be great.
>
> "build_reloptions" parses "reloptions" and takes for it a list of allowed
> options defined by the 5th argument "relopt_elems" and the 6th argument
> "num_relopt_elems", which are NULL and 0 in the removed call. If "validate"
> is false, it ignores options, which are not in the list, while parsing. If
> "validate" is true, it "elog"s ERROR when it meets option, which is not in the
> allowed list.

Karina's changes make sense to me, so +1.

This is a minor patch, so I will set this as Ready For Committer.

-- 
Simon Riggs                http://www.EnterpriseDB.com/





^ permalink  raw  reply  [nested|flat] 8+ messages in thread

* Re: Error for WITH options on partitioned tables
  2022-10-14 15:16 Re: Error for WITH options on partitioned tables Karina Litskevich <[email protected]>
  2022-10-28 21:21 ` Re: Error for WITH options on partitioned tables David Zhang <[email protected]>
  2022-11-07 08:55   ` Re: Error for WITH options on partitioned tables Karina Litskevich <[email protected]>
  2022-11-07 11:10     ` Re: Error for WITH options on partitioned tables Simon Riggs <[email protected]>
@ 2022-11-09 17:33       ` Tom Lane <[email protected]>
  0 siblings, 0 replies; 8+ messages in thread

From: Tom Lane @ 2022-11-09 17:33 UTC (permalink / raw)
  To: Simon Riggs <[email protected]>; +Cc: Karina Litskevich <[email protected]>; David Zhang <[email protected]>; Japin Li <[email protected]>; Postgres hackers <[email protected]>

Simon Riggs <[email protected]> writes:
> Karina's changes make sense to me, so +1.
> This is a minor patch, so I will set this as Ready For Committer.

Pushed with minor fiddling:

* I concur with Karina's thought that ERRCODE_WRONG_OBJECT_TYPE
is the most on-point errcode for this.  The complaint is specifically
about the table relkind and has nothing to do with the storage
parameter per se.  I also agree that it's not worth trying to use
a different errcode for CREATE vs. ALTER.

* The HINT message wasn't per project style (it should be formatted as
a complete sentence), and I thought using "parameters for" in the
main message but "parameters on" in the hint was oddly inconsistent.

			regards, tom lane





^ permalink  raw  reply  [nested|flat] 8+ messages in thread

* [PATCH v10 1/7] Row pattern recognition patch for raw parser.
@ 2023-10-22 02:22 Tatsuo Ishii <[email protected]>
  0 siblings, 0 replies; 8+ messages in thread

From: Tatsuo Ishii @ 2023-10-22 02:22 UTC (permalink / raw)

---
 src/backend/parser/gram.y       | 222 ++++++++++++++++++++++++++++++--
 src/include/nodes/parsenodes.h  |  56 ++++++++
 src/include/parser/kwlist.h     |   8 ++
 src/include/parser/parse_node.h |   1 +
 4 files changed, 273 insertions(+), 14 deletions(-)

diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index c224df4ecc..e09eb061f8 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -251,6 +251,8 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
 	DefElem	   *defelt;
 	SortBy	   *sortby;
 	WindowDef  *windef;
+	RPCommonSyntax	*rpcom;
+	RPSubsetItem	*rpsubset;
 	JoinExpr   *jexpr;
 	IndexElem  *ielem;
 	StatsElem  *selem;
@@ -278,6 +280,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
 	MergeWhenClause *mergewhen;
 	struct KeyActions *keyactions;
 	struct KeyAction *keyaction;
+	RPSkipTo	skipto;
 }
 
 %type <node>	stmt toplevel_stmt schema_stmt routine_body_stmt
@@ -453,8 +456,12 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
 				TriggerTransitions TriggerReferencing
 				vacuum_relation_list opt_vacuum_relation_list
 				drop_option_list pub_obj_list
-
-%type <node>	opt_routine_body
+				row_pattern_measure_list row_pattern_definition_list
+				opt_row_pattern_subset_clause
+				row_pattern_subset_list row_pattern_subset_rhs
+				row_pattern
+%type <rpsubset>	 row_pattern_subset_item
+%type <node>	opt_routine_body row_pattern_term
 %type <groupclause> group_clause
 %type <list>	group_by_list
 %type <node>	group_by_item empty_grouping_set rollup_clause cube_clause
@@ -551,6 +558,8 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
 %type <range>	relation_expr_opt_alias
 %type <node>	tablesample_clause opt_repeatable_clause
 %type <target>	target_el set_target insert_column_item
+				row_pattern_measure_item row_pattern_definition
+%type <skipto>	first_or_last
 
 %type <str>		generic_option_name
 %type <node>	generic_option_arg
@@ -633,6 +642,9 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
 %type <list>	window_clause window_definition_list opt_partition_clause
 %type <windef>	window_definition over_clause window_specification
 				opt_frame_clause frame_extent frame_bound
+%type <rpcom>	opt_row_pattern_common_syntax opt_row_pattern_skip_to
+%type <boolean>	opt_row_pattern_initial_or_seek
+%type <list>	opt_row_pattern_measures
 %type <ival>	opt_window_exclusion_clause
 %type <str>		opt_existing_window_name
 %type <boolean> opt_if_not_exists
@@ -659,7 +671,6 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
 				json_object_constructor_null_clause_opt
 				json_array_constructor_null_clause_opt
 
-
 /*
  * Non-keyword token types.  These are hard-wired into the "flex" lexer.
  * They must be listed first so that their numeric codes do not depend on
@@ -702,7 +713,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
 	CURRENT_TIME CURRENT_TIMESTAMP CURRENT_USER CURSOR CYCLE
 
 	DATA_P DATABASE DAY_P DEALLOCATE DEC DECIMAL_P DECLARE DEFAULT DEFAULTS
-	DEFERRABLE DEFERRED DEFINER DELETE_P DELIMITER DELIMITERS DEPENDS DEPTH DESC
+	DEFERRABLE DEFERRED DEFINE DEFINER DELETE_P DELIMITER DELIMITERS DEPENDS DEPTH DESC
 	DETACH DICTIONARY DISABLE_P DISCARD DISTINCT DO DOCUMENT_P DOMAIN_P
 	DOUBLE_P DROP
 
@@ -718,7 +729,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
 	HANDLER HAVING HEADER_P HOLD HOUR_P
 
 	IDENTITY_P IF_P ILIKE IMMEDIATE IMMUTABLE IMPLICIT_P IMPORT_P IN_P INCLUDE
-	INCLUDING INCREMENT INDENT INDEX INDEXES INHERIT INHERITS INITIALLY INLINE_P
+	INCLUDING INCREMENT INDENT INDEX INDEXES INHERIT INHERITS INITIAL INITIALLY INLINE_P
 	INNER_P INOUT INPUT_P INSENSITIVE INSERT INSTEAD INT_P INTEGER
 	INTERSECT INTERVAL INTO INVOKER IS ISNULL ISOLATION
 
@@ -731,7 +742,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
 	LEADING LEAKPROOF LEAST LEFT LEVEL LIKE LIMIT LISTEN LOAD LOCAL
 	LOCALTIME LOCALTIMESTAMP LOCATION LOCK_P LOCKED LOGGED
 
-	MAPPING MATCH MATCHED MATERIALIZED MAXVALUE MERGE METHOD
+	MAPPING MATCH MATCHED MATERIALIZED MAXVALUE MEASURES MERGE METHOD
 	MINUTE_P MINVALUE MODE MONTH_P MOVE
 
 	NAME_P NAMES NATIONAL NATURAL NCHAR NEW NEXT NFC NFD NFKC NFKD NO NONE
@@ -743,8 +754,8 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
 	ORDER ORDINALITY OTHERS OUT_P OUTER_P
 	OVER OVERLAPS OVERLAY OVERRIDING OWNED OWNER
 
-	PARALLEL PARAMETER PARSER PARTIAL PARTITION PASSING PASSWORD
-	PLACING PLANS POLICY
+	PARALLEL PARAMETER PARSER PARTIAL PARTITION PASSING PASSWORD PAST
+	PATTERN_P PERMUTE PLACING PLANS POLICY
 	POSITION PRECEDING PRECISION PRESERVE PREPARE PREPARED PRIMARY
 	PRIOR PRIVILEGES PROCEDURAL PROCEDURE PROCEDURES PROGRAM PUBLICATION
 
@@ -755,12 +766,13 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
 	RESET RESTART RESTRICT RETURN RETURNING RETURNS REVOKE RIGHT ROLE ROLLBACK ROLLUP
 	ROUTINE ROUTINES ROW ROWS RULE
 
-	SAVEPOINT SCALAR SCHEMA SCHEMAS SCROLL SEARCH SECOND_P SECURITY SELECT
+	SAVEPOINT SCALAR SCHEMA SCHEMAS SCROLL SEARCH SECOND_P SECURITY SEEK SELECT
 	SEQUENCE SEQUENCES
+
 	SERIALIZABLE SERVER SESSION SESSION_USER SET SETS SETOF SHARE SHOW
 	SIMILAR SIMPLE SKIP SMALLINT SNAPSHOT SOME SQL_P STABLE STANDALONE_P
 	START STATEMENT STATISTICS STDIN STDOUT STORAGE STORED STRICT_P STRIP_P
-	SUBSCRIPTION SUBSTRING SUPPORT SYMMETRIC SYSID SYSTEM_P SYSTEM_USER
+	SUBSCRIPTION SUBSET SUBSTRING SUPPORT SYMMETRIC SYSID SYSTEM_P SYSTEM_USER
 
 	TABLE TABLES TABLESAMPLE TABLESPACE TEMP TEMPLATE TEMPORARY TEXT_P THEN
 	TIES TIME TIMESTAMP TO TRAILING TRANSACTION TRANSFORM
@@ -853,6 +865,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
  */
 %nonassoc	UNBOUNDED		/* ideally would have same precedence as IDENT */
 %nonassoc	IDENT PARTITION RANGE ROWS GROUPS PRECEDING FOLLOWING CUBE ROLLUP
+%nonassoc	MEASURES AFTER INITIAL SEEK PATTERN_P
 %left		Op OPERATOR		/* multi-character ops and user-defined operators */
 %left		'+' '-'
 %left		'*' '/' '%'
@@ -15901,7 +15914,8 @@ over_clause: OVER window_specification
 		;
 
 window_specification: '(' opt_existing_window_name opt_partition_clause
-						opt_sort_clause opt_frame_clause ')'
+						opt_sort_clause opt_row_pattern_measures opt_frame_clause
+						opt_row_pattern_common_syntax ')'
 				{
 					WindowDef  *n = makeNode(WindowDef);
 
@@ -15909,10 +15923,12 @@ window_specification: '(' opt_existing_window_name opt_partition_clause
 					n->refname = $2;
 					n->partitionClause = $3;
 					n->orderClause = $4;
+					n->rowPatternMeasures = $5;
 					/* copy relevant fields of opt_frame_clause */
-					n->frameOptions = $5->frameOptions;
-					n->startOffset = $5->startOffset;
-					n->endOffset = $5->endOffset;
+					n->frameOptions = $6->frameOptions;
+					n->startOffset = $6->startOffset;
+					n->endOffset = $6->endOffset;
+					n->rpCommonSyntax = $7;
 					n->location = @1;
 					$$ = n;
 				}
@@ -15936,6 +15952,31 @@ opt_partition_clause: PARTITION BY expr_list		{ $$ = $3; }
 			| /*EMPTY*/								{ $$ = NIL; }
 		;
 
+/*
+ * ROW PATTERN_P MEASURES
+ */
+opt_row_pattern_measures: MEASURES row_pattern_measure_list	{ $$ = $2; }
+			| /*EMPTY*/								{ $$ = NIL; }
+		;
+
+row_pattern_measure_list:
+			row_pattern_measure_item
+					{ $$ = list_make1($1); }
+			| row_pattern_measure_list ',' row_pattern_measure_item
+					{ $$ = lappend($1, $3); }
+		;
+
+row_pattern_measure_item:
+			a_expr AS ColLabel
+				{
+					$$ = makeNode(ResTarget);
+					$$->name = $3;
+					$$->indirection = NIL;
+					$$->val = (Node *) $1;
+					$$->location = @1;
+				}
+		;
+
 /*
  * For frame clauses, we return a WindowDef, but only some fields are used:
  * frameOptions, startOffset, and endOffset.
@@ -16095,6 +16136,143 @@ opt_window_exclusion_clause:
 			| /*EMPTY*/				{ $$ = 0; }
 		;
 
+opt_row_pattern_common_syntax:
+opt_row_pattern_skip_to opt_row_pattern_initial_or_seek
+				PATTERN_P '(' row_pattern ')'
+				opt_row_pattern_subset_clause
+				DEFINE row_pattern_definition_list
+			{
+				RPCommonSyntax *n = makeNode(RPCommonSyntax);
+				n->rpSkipTo = $1->rpSkipTo;
+				n->rpSkipVariable = $1->rpSkipVariable;
+				n->initial = $2;
+				n->rpPatterns = $5;
+				n->rpSubsetClause = $7;
+				n->rpDefs = $9;
+				$$ = n;
+			}
+			| /*EMPTY*/		{ $$ = NULL; }
+	;
+
+opt_row_pattern_skip_to:
+			AFTER MATCH SKIP TO NEXT ROW
+				{
+					RPCommonSyntax *n = makeNode(RPCommonSyntax);
+					n->rpSkipTo = ST_NEXT_ROW;
+					n->rpSkipVariable = NULL;
+					$$ = n;
+			}
+			| AFTER MATCH SKIP PAST LAST_P ROW
+				{
+					RPCommonSyntax *n = makeNode(RPCommonSyntax);
+					n->rpSkipTo = ST_PAST_LAST_ROW;
+					n->rpSkipVariable = NULL;
+					$$ = n;
+				}
+			| AFTER MATCH SKIP TO first_or_last ColId
+				{
+					RPCommonSyntax *n = makeNode(RPCommonSyntax);
+					n->rpSkipTo = $5;
+					n->rpSkipVariable = $6;
+					$$ = n;
+				}
+/*
+			| AFTER MATCH SKIP TO LAST_P ColId		%prec LAST_P
+				{
+					RPCommonSyntax *n = makeNode(RPCommonSyntax);
+					n->rpSkipTo = ST_LAST_VARIABLE;
+					n->rpSkipVariable = $6;
+					$$ = n;
+				}
+			| AFTER MATCH SKIP TO ColId
+				{
+					RPCommonSyntax *n = makeNode(RPCommonSyntax);
+					n->rpSkipTo = ST_VARIABLE;
+					n->rpSkipVariable = $5;
+					$$ = n;
+				}
+*/
+			| /*EMPTY*/
+				{
+					RPCommonSyntax *n = makeNode(RPCommonSyntax);
+					/* temporary set default to ST_NEXT_ROW */
+					n->rpSkipTo = ST_PAST_LAST_ROW;
+					n->rpSkipVariable = NULL;
+					$$ = n;
+				}
+	;
+
+first_or_last:
+			FIRST_P		{ $$ = ST_FIRST_VARIABLE; }
+			| LAST_P	{ $$ = ST_LAST_VARIABLE; }
+	;
+
+opt_row_pattern_initial_or_seek:
+			INITIAL			{ $$ = true; }
+			| SEEK
+				{
+					ereport(ERROR,
+							(errcode(ERRCODE_SYNTAX_ERROR),
+							 errmsg("SEEK is not supported"),
+							 errhint("Use INITIAL."),
+							 parser_errposition(@1)));
+				}
+			| /*EMPTY*/		{ $$ = true; }
+		;
+
+row_pattern:
+			row_pattern_term							{ $$ = list_make1($1); }
+			| row_pattern row_pattern_term				{ $$ = lappend($1, $2); }
+		;
+
+row_pattern_term:
+			ColId	{ $$ = (Node *) makeSimpleA_Expr(AEXPR_OP, "", (Node *)makeString($1), NULL, @1); }
+			| ColId '*'	{ $$ = (Node *) makeSimpleA_Expr(AEXPR_OP, "*", (Node *)makeString($1), NULL, @1); }
+			| ColId '+'	{ $$ = (Node *) makeSimpleA_Expr(AEXPR_OP, "+", (Node *)makeString($1), NULL, @1); }
+			| ColId '?'	{ $$ = (Node *) makeSimpleA_Expr(AEXPR_OP, "?", (Node *)makeString($1), NULL, @1); }
+		;
+
+opt_row_pattern_subset_clause:
+			SUBSET row_pattern_subset_list	{ $$ = $2; }
+			| /*EMPTY*/												{ $$ = NIL; }
+		;
+
+row_pattern_subset_list:
+			row_pattern_subset_item									{ $$ = list_make1($1); }
+			| row_pattern_subset_list ',' row_pattern_subset_item	{ $$ = lappend($1, $3); }
+			| /*EMPTY*/												{ $$ = NIL; }
+		;
+
+row_pattern_subset_item: ColId '=' '(' row_pattern_subset_rhs ')'
+			{
+				RPSubsetItem *n = makeNode(RPSubsetItem);
+				n->name = $1;
+				n->rhsVariable = $4;
+				$$ = n;
+			}
+		;
+
+row_pattern_subset_rhs:
+			ColId								{ $$ = list_make1(makeStringConst($1, @1)); }
+			| row_pattern_subset_rhs ',' ColId	{ $$ = lappend($1, makeStringConst($3, @1)); }
+			| /*EMPTY*/							{ $$ = NIL; }
+		;
+
+row_pattern_definition_list:
+			row_pattern_definition										{ $$ = list_make1($1); }
+			| row_pattern_definition_list ',' row_pattern_definition	{ $$ = lappend($1, $3); }
+		;
+
+row_pattern_definition:
+			ColId AS a_expr
+				{
+					$$ = makeNode(ResTarget);
+					$$->name = $1;
+					$$->indirection = NIL;
+					$$->val = (Node *) $3;
+					$$->location = @1;
+				}
+		;
 
 /*
  * Supporting nonterminals for expressions.
@@ -17190,6 +17368,7 @@ unreserved_keyword:
 			| INDEXES
 			| INHERIT
 			| INHERITS
+			| INITIAL
 			| INLINE_P
 			| INPUT_P
 			| INSENSITIVE
@@ -17217,6 +17396,7 @@ unreserved_keyword:
 			| MATCHED
 			| MATERIALIZED
 			| MAXVALUE
+			| MEASURES
 			| MERGE
 			| METHOD
 			| MINUTE_P
@@ -17259,6 +17439,9 @@ unreserved_keyword:
 			| PARTITION
 			| PASSING
 			| PASSWORD
+			| PAST
+			| PATTERN_P
+			| PERMUTE
 			| PLANS
 			| POLICY
 			| PRECEDING
@@ -17309,6 +17492,7 @@ unreserved_keyword:
 			| SEARCH
 			| SECOND_P
 			| SECURITY
+			| SEEK
 			| SEQUENCE
 			| SEQUENCES
 			| SERIALIZABLE
@@ -17334,6 +17518,7 @@ unreserved_keyword:
 			| STRICT_P
 			| STRIP_P
 			| SUBSCRIPTION
+			| SUBSET
 			| SUPPORT
 			| SYSID
 			| SYSTEM_P
@@ -17521,6 +17706,7 @@ reserved_keyword:
 			| CURRENT_USER
 			| DEFAULT
 			| DEFERRABLE
+			| DEFINE
 			| DESC
 			| DISTINCT
 			| DO
@@ -17683,6 +17869,7 @@ bare_label_keyword:
 			| DEFAULTS
 			| DEFERRABLE
 			| DEFERRED
+			| DEFINE
 			| DEFINER
 			| DELETE_P
 			| DELIMITER
@@ -17758,6 +17945,7 @@ bare_label_keyword:
 			| INDEXES
 			| INHERIT
 			| INHERITS
+			| INITIAL
 			| INITIALLY
 			| INLINE_P
 			| INNER_P
@@ -17807,6 +17995,7 @@ bare_label_keyword:
 			| MATCHED
 			| MATERIALIZED
 			| MAXVALUE
+			| MEASURES
 			| MERGE
 			| METHOD
 			| MINVALUE
@@ -17860,6 +18049,9 @@ bare_label_keyword:
 			| PARTITION
 			| PASSING
 			| PASSWORD
+			| PAST
+			| PATTERN_P
+			| PERMUTE
 			| PLACING
 			| PLANS
 			| POLICY
@@ -17916,6 +18108,7 @@ bare_label_keyword:
 			| SCROLL
 			| SEARCH
 			| SECURITY
+			| SEEK
 			| SELECT
 			| SEQUENCE
 			| SEQUENCES
@@ -17947,6 +18140,7 @@ bare_label_keyword:
 			| STRICT_P
 			| STRIP_P
 			| SUBSCRIPTION
+			| SUBSET
 			| SUBSTRING
 			| SUPPORT
 			| SYMMETRIC
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index f637937cd2..31b04d6919 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -547,6 +547,44 @@ typedef struct SortBy
 	int			location;		/* operator location, or -1 if none/unknown */
 } SortBy;
 
+/*
+ * AFTER MATCH row pattern skip to types in row pattern common syntax
+ */
+typedef enum RPSkipTo
+{
+	ST_NONE,					/* AFTER MATCH omitted */
+	ST_NEXT_ROW,				/* SKIP TO NEXT ROW */
+	ST_PAST_LAST_ROW,			/* SKIP TO PAST LAST ROW */
+	ST_FIRST_VARIABLE,			/* SKIP TO FIRST variable name */
+	ST_LAST_VARIABLE,			/* SKIP TO LAST variable name */
+	ST_VARIABLE					/* SKIP TO variable name */
+} RPSkipTo;
+
+/*
+ * Row Pattern SUBSET clause item
+ */
+typedef struct RPSubsetItem
+{
+	NodeTag		type;
+	char	   *name;			/* Row Pattern SUBSET clause variable name */
+	List	   *rhsVariable;	/* Row Pattern SUBSET rhs variables (list of char *string) */
+} RPSubsetItem;
+
+/*
+ * RowPatternCommonSyntax - raw representation of row pattern common syntax
+ *
+ */
+typedef struct RPCommonSyntax
+{
+	NodeTag		type;
+	RPSkipTo	rpSkipTo;		/* Row Pattern AFTER MATCH SKIP type */
+	char	   *rpSkipVariable;	/* Row Pattern Skip To variable name, if any */
+	bool		initial;		/* true if <row pattern initial or seek> is initial */
+	List	   *rpPatterns;		/* PATTERN variables (list of A_Expr) */
+	List	   *rpSubsetClause;	/* row pattern subset clause (list of RPSubsetItem), if any */
+	List	   *rpDefs;			/* row pattern definitions clause (list of ResTarget) */
+} RPCommonSyntax;
+
 /*
  * WindowDef - raw representation of WINDOW and OVER clauses
  *
@@ -562,6 +600,8 @@ typedef struct WindowDef
 	char	   *refname;		/* referenced window name, if any */
 	List	   *partitionClause;	/* PARTITION BY expression list */
 	List	   *orderClause;	/* ORDER BY (list of SortBy) */
+	List	   *rowPatternMeasures;	/* row pattern measures (list of ResTarget) */
+	RPCommonSyntax *rpCommonSyntax;	/* row pattern common syntax */
 	int			frameOptions;	/* frame_clause options, see below */
 	Node	   *startOffset;	/* expression for starting bound, if any */
 	Node	   *endOffset;		/* expression for ending bound, if any */
@@ -1483,6 +1523,11 @@ typedef struct GroupingSet
  * the orderClause might or might not be copied (see copiedOrder); the framing
  * options are never copied, per spec.
  *
+ * "defineClause" is Row Pattern Recognition DEFINE clause (list of
+ * TargetEntry). TargetEntry.resname represents row pattern definition
+ * variable name. "patternVariable" and "patternRegexp" represents PATTERN
+ * clause.
+ *
  * The information relevant for the query jumbling is the partition clause
  * type and its bounds.
  */
@@ -1514,6 +1559,17 @@ typedef struct WindowClause
 	Index		winref;			/* ID referenced by window functions */
 	/* did we copy orderClause from refname? */
 	bool		copiedOrder pg_node_attr(query_jumble_ignore);
+	/* Row Pattern AFTER MACH SKIP clause */
+	RPSkipTo	rpSkipTo;		/* Row Pattern Skip To type */
+	bool		initial;		/* true if <row pattern initial or seek> is initial */
+	/* Row Pattern DEFINE clause (list of TargetEntry) */
+	List		*defineClause;
+	/* Row Pattern DEFINE variable initial names (list of String) */
+	List		*defineInitial;
+	/* Row Pattern PATTERN variable name (list of String) */
+	List		*patternVariable;
+	/* Row Pattern PATTERN regular expression quantifier ('+' or ''. list of String) */
+	List		*patternRegexp;
 } WindowClause;
 
 /*
diff --git a/src/include/parser/kwlist.h b/src/include/parser/kwlist.h
index 5984dcfa4b..2804333b53 100644
--- a/src/include/parser/kwlist.h
+++ b/src/include/parser/kwlist.h
@@ -128,6 +128,7 @@ PG_KEYWORD("default", DEFAULT, RESERVED_KEYWORD, BARE_LABEL)
 PG_KEYWORD("defaults", DEFAULTS, UNRESERVED_KEYWORD, BARE_LABEL)
 PG_KEYWORD("deferrable", DEFERRABLE, RESERVED_KEYWORD, BARE_LABEL)
 PG_KEYWORD("deferred", DEFERRED, UNRESERVED_KEYWORD, BARE_LABEL)
+PG_KEYWORD("define", DEFINE, RESERVED_KEYWORD, BARE_LABEL)
 PG_KEYWORD("definer", DEFINER, UNRESERVED_KEYWORD, BARE_LABEL)
 PG_KEYWORD("delete", DELETE_P, UNRESERVED_KEYWORD, BARE_LABEL)
 PG_KEYWORD("delimiter", DELIMITER, UNRESERVED_KEYWORD, BARE_LABEL)
@@ -212,6 +213,7 @@ PG_KEYWORD("index", INDEX, UNRESERVED_KEYWORD, BARE_LABEL)
 PG_KEYWORD("indexes", INDEXES, UNRESERVED_KEYWORD, BARE_LABEL)
 PG_KEYWORD("inherit", INHERIT, UNRESERVED_KEYWORD, BARE_LABEL)
 PG_KEYWORD("inherits", INHERITS, UNRESERVED_KEYWORD, BARE_LABEL)
+PG_KEYWORD("initial", INITIAL, UNRESERVED_KEYWORD, BARE_LABEL)
 PG_KEYWORD("initially", INITIALLY, RESERVED_KEYWORD, BARE_LABEL)
 PG_KEYWORD("inline", INLINE_P, UNRESERVED_KEYWORD, BARE_LABEL)
 PG_KEYWORD("inner", INNER_P, TYPE_FUNC_NAME_KEYWORD, BARE_LABEL)
@@ -265,6 +267,7 @@ PG_KEYWORD("match", MATCH, UNRESERVED_KEYWORD, BARE_LABEL)
 PG_KEYWORD("matched", MATCHED, UNRESERVED_KEYWORD, BARE_LABEL)
 PG_KEYWORD("materialized", MATERIALIZED, UNRESERVED_KEYWORD, BARE_LABEL)
 PG_KEYWORD("maxvalue", MAXVALUE, UNRESERVED_KEYWORD, BARE_LABEL)
+PG_KEYWORD("measures", MEASURES, UNRESERVED_KEYWORD, BARE_LABEL)
 PG_KEYWORD("merge", MERGE, UNRESERVED_KEYWORD, BARE_LABEL)
 PG_KEYWORD("method", METHOD, UNRESERVED_KEYWORD, BARE_LABEL)
 PG_KEYWORD("minute", MINUTE_P, UNRESERVED_KEYWORD, AS_LABEL)
@@ -326,6 +329,9 @@ PG_KEYWORD("partial", PARTIAL, UNRESERVED_KEYWORD, BARE_LABEL)
 PG_KEYWORD("partition", PARTITION, UNRESERVED_KEYWORD, BARE_LABEL)
 PG_KEYWORD("passing", PASSING, UNRESERVED_KEYWORD, BARE_LABEL)
 PG_KEYWORD("password", PASSWORD, UNRESERVED_KEYWORD, BARE_LABEL)
+PG_KEYWORD("past", PAST, UNRESERVED_KEYWORD, BARE_LABEL)
+PG_KEYWORD("pattern", PATTERN_P, UNRESERVED_KEYWORD, BARE_LABEL)
+PG_KEYWORD("permute", PERMUTE, UNRESERVED_KEYWORD, BARE_LABEL)
 PG_KEYWORD("placing", PLACING, RESERVED_KEYWORD, BARE_LABEL)
 PG_KEYWORD("plans", PLANS, UNRESERVED_KEYWORD, BARE_LABEL)
 PG_KEYWORD("policy", POLICY, UNRESERVED_KEYWORD, BARE_LABEL)
@@ -385,6 +391,7 @@ PG_KEYWORD("scroll", SCROLL, UNRESERVED_KEYWORD, BARE_LABEL)
 PG_KEYWORD("search", SEARCH, UNRESERVED_KEYWORD, BARE_LABEL)
 PG_KEYWORD("second", SECOND_P, UNRESERVED_KEYWORD, AS_LABEL)
 PG_KEYWORD("security", SECURITY, UNRESERVED_KEYWORD, BARE_LABEL)
+PG_KEYWORD("seek", SEEK, UNRESERVED_KEYWORD, BARE_LABEL)
 PG_KEYWORD("select", SELECT, RESERVED_KEYWORD, BARE_LABEL)
 PG_KEYWORD("sequence", SEQUENCE, UNRESERVED_KEYWORD, BARE_LABEL)
 PG_KEYWORD("sequences", SEQUENCES, UNRESERVED_KEYWORD, BARE_LABEL)
@@ -416,6 +423,7 @@ PG_KEYWORD("stored", STORED, UNRESERVED_KEYWORD, BARE_LABEL)
 PG_KEYWORD("strict", STRICT_P, UNRESERVED_KEYWORD, BARE_LABEL)
 PG_KEYWORD("strip", STRIP_P, UNRESERVED_KEYWORD, BARE_LABEL)
 PG_KEYWORD("subscription", SUBSCRIPTION, UNRESERVED_KEYWORD, BARE_LABEL)
+PG_KEYWORD("subset", SUBSET, UNRESERVED_KEYWORD, BARE_LABEL)
 PG_KEYWORD("substring", SUBSTRING, COL_NAME_KEYWORD, BARE_LABEL)
 PG_KEYWORD("support", SUPPORT, UNRESERVED_KEYWORD, BARE_LABEL)
 PG_KEYWORD("symmetric", SYMMETRIC, RESERVED_KEYWORD, BARE_LABEL)
diff --git a/src/include/parser/parse_node.h b/src/include/parser/parse_node.h
index f589112d5e..6640090910 100644
--- a/src/include/parser/parse_node.h
+++ b/src/include/parser/parse_node.h
@@ -51,6 +51,7 @@ typedef enum ParseExprKind
 	EXPR_KIND_WINDOW_FRAME_RANGE,	/* window frame clause with RANGE */
 	EXPR_KIND_WINDOW_FRAME_ROWS,	/* window frame clause with ROWS */
 	EXPR_KIND_WINDOW_FRAME_GROUPS,	/* window frame clause with GROUPS */
+	EXPR_KIND_RPR_DEFINE,		/* DEFINE */
 	EXPR_KIND_SELECT_TARGET,	/* SELECT target list item */
 	EXPR_KIND_INSERT_TARGET,	/* INSERT target list item */
 	EXPR_KIND_UPDATE_SOURCE,	/* UPDATE assignment source item */
-- 
2.25.1


----Next_Part(Sun_Oct_22_11_39_20_2023_140)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="v10-0002-Row-pattern-recognition-patch-parse-analysis.patch"



^ permalink  raw  reply  [nested|flat] 8+ messages in thread


end of thread, other threads:[~2023-10-22 02:22 UTC | newest]

Thread overview: 8+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-09-12 13:07 [PATCH 1/6] Pass all scan keys to BRIN consistent function at once Tomas Vondra <[email protected]>
2022-10-14 15:16 Re: Error for WITH options on partitioned tables Karina Litskevich <[email protected]>
2022-10-28 21:21 ` Re: Error for WITH options on partitioned tables David Zhang <[email protected]>
2022-11-01 23:58   ` Re: Error for WITH options on partitioned tables Simon Riggs <[email protected]>
2022-11-07 08:55   ` Re: Error for WITH options on partitioned tables Karina Litskevich <[email protected]>
2022-11-07 11:10     ` Re: Error for WITH options on partitioned tables Simon Riggs <[email protected]>
2022-11-09 17:33       ` Re: Error for WITH options on partitioned tables Tom Lane <[email protected]>
2023-10-22 02:22 [PATCH v10 1/7] Row pattern recognition patch for raw parser. Tatsuo Ishii <[email protected]>

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox