v1-0001-refactor-findNewOrOldColumn.patch

From 81a5eab2596c0e322dee8165ad73343328f496ed Mon Sep 17 00:00:00 2001
From: pgaddict <jian.universality@gmail.com>
Date: Wed, 25 Oct 2023 13:50:38 +0800
Subject: [PATCH v1 1/1] refactor findNewOrOldColumn.

refator findNeworOldColumn to validate WITHOUT OVERLAP constraint
associated key column exists and it's a range data type.

create/alter table add without overlap constraint:
if the key column not exists,
it will report the same as no "without overlap" normal case.

create table add without overlap constraint:
if overlap constraint associated column's data type is not range,
it will report it's not a range error, also print out the
parser position.

alter table add without overlap constraint:
if overlap constraint associated column's data type is not range,
it will report not a range data type error.
it will not print out parser position (I don't know how to do that).
---
 src/backend/parser/parse_utilcmd.c            | 81 ++++++++++---------
 .../regress/expected/without_overlaps.out     |  8 +-
 2 files changed, 47 insertions(+), 42 deletions(-)

diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c
index e195410c..f44de591 100644
--- a/src/backend/parser/parse_utilcmd.c
+++ b/src/backend/parser/parse_utilcmd.c
@@ -124,8 +124,7 @@ static List *get_opclass(Oid opclass, Oid actual_datatype);
 static void transformIndexConstraints(CreateStmtContext *cxt);
 static IndexStmt *transformIndexConstraint(Constraint *constraint,
 										   CreateStmtContext *cxt);
-static bool findNewOrOldColumn(CreateStmtContext *cxt, char *colname, char **typname,
-							   Oid *typid);
+static void validate_without_overlaps(CreateStmtContext *cxt, char *colname);
 static void transformExtendedStatistics(CreateStmtContext *cxt);
 static void transformFKConstraints(CreateStmtContext *cxt,
 								   bool skipValidation,
@@ -2698,34 +2697,19 @@ transformIndexConstraint(Constraint *constraint, CreateStmtContext *cxt)
 		 * Anything in without_overlaps should be included,
 		 * but with the overlaps operator (&&) instead of equality.
 		 */
-		if (constraint->without_overlaps != NULL) {
+		if (constraint->without_overlaps != NULL)
+		{
 			char *without_overlaps_str = strVal(constraint->without_overlaps);
 			IndexElem *iparam = makeNode(IndexElem);
-			char   *typname;
-			Oid		typid;
 
 			/*
 			 * Iterate through the table's columns
 			 * (like just a little bit above).
 			 * If we find one whose name is the same as without_overlaps,
 			 * validate that it's a range type.
-			 *
-			 * Otherwise report an error.
+			 * if it cannot found, raise an error.
 			 */
-
-			if (findNewOrOldColumn(cxt, without_overlaps_str, &typname, &typid))
-			{
-				if (!type_is_range(typid))
-					ereport(ERROR,
-							(errcode(ERRCODE_DATATYPE_MISMATCH),
-							 errmsg("column \"%s\" in WITHOUT OVERLAPS is not a range type",
-									without_overlaps_str)));
-			}
-			else
-				ereport(ERROR,
-						(errcode(ERRCODE_UNDEFINED_COLUMN),
-						 errmsg("range or PERIOD \"%s\" in WITHOUT OVERLAPS does not exist",
-								without_overlaps_str)));
+			validate_without_overlaps(cxt, without_overlaps_str);
 
 			iparam->name = pstrdup(without_overlaps_str);
 			iparam->expr = NULL;
@@ -2744,6 +2728,9 @@ transformIndexConstraint(Constraint *constraint, CreateStmtContext *cxt)
 			{
 				/*
 				 * Force the column to NOT NULL since it is part of the primary key.
+				 * prior loop foreach(lc, constraint->keys) does not loop over
+				 * without_overlap keys. So we need manually add here.
+				 *
 				 */
 				AlterTableCmd *notnullcmd = makeNode(AlterTableCmd);
 
@@ -2872,31 +2859,37 @@ transformIndexConstraint(Constraint *constraint, CreateStmtContext *cxt)
 }
 
 /*
- * Tries to find a column by name among the existing ones (if it's an ALTER TABLE)
- * and the new ones. Sets typname and typid if one is found. Returns false if we
- * couldn't find a match.
+ * Tries to find and validate the typid associated
+ * with a without_overlaps column name, even in ALTER TABLE case.
+ * After check, without_overlaps column should exist, and it's a range type.
+ *
  */
-static bool
-findNewOrOldColumn(CreateStmtContext *cxt, char *colname, char **typname, Oid *typid)
+static void
+validate_without_overlaps(CreateStmtContext *cxt, char *colname)
 {
 	/* Check the new columns first in case their type is changing. */
-
 	ColumnDef  *column = NULL;
 	ListCell   *columns;
+	Oid			typid;
+	bool		found	= false;
 
 	foreach(columns, cxt->columns)
 	{
 		column = lfirst_node(ColumnDef, columns);
 		if (strcmp(column->colname, colname) == 0)
 		{
-			*typid = typenameTypeId(NULL, column->typeName);
-			*typname = TypeNameToString(column->typeName);
-			return true;
+			typid = typenameTypeId(NULL, column->typeName);
+			if (!type_is_range(typid))
+				ereport(ERROR,
+						(errcode(ERRCODE_DATATYPE_MISMATCH),
+						 errmsg("column \"%s\" in WITHOUT OVERLAPS is not a range type",colname),
+						 parser_errposition(cxt->pstate, column->location)));
+			found = true;
+			break;
 		}
 	}
 
 	/* Look up columns on existing table. */
-
 	if (cxt->isalter)
 	{
 		Relation rel = cxt->rel;
@@ -2905,22 +2898,30 @@ findNewOrOldColumn(CreateStmtContext *cxt, char *colname, char **typname, Oid *t
 			Form_pg_attribute attr = TupleDescAttr(rel->rd_att, i);
 			const char *attname;
 
-			if (attr->attisdropped)
-				continue;
-
 			attname = NameStr(attr->attname);
 			if (strcmp(attname, colname) == 0)
 			{
-				Type type = typeidType(attr->atttypid);
-				*typid = attr->atttypid;
-				*typname = pstrdup(typeTypeName(type));
-				ReleaseSysCache(type);
-				return true;
+				typid = attr->atttypid;
+
+				if (attr->attisdropped)
+				{
+					found = false;
+					break;
+				}
+				if (!type_is_range(typid))
+					ereport(ERROR,
+							(errcode(ERRCODE_DATATYPE_MISMATCH),
+							 errmsg("column \"%s\" in WITHOUT OVERLAPS is not a range type", colname)));
+				found = true;
+				break;
 			}
 		}
 	}
 
-	return false;
+	if (!found)
+		ereport(ERROR,
+				(errcode(ERRCODE_UNDEFINED_COLUMN),
+				 errmsg("column \"%s\" named in key does not exist",colname)));
 }
 
 /*
diff --git a/src/test/regress/expected/without_overlaps.out b/src/test/regress/expected/without_overlaps.out
index 4b8abb08..a916e56a 100644
--- a/src/test/regress/expected/without_overlaps.out
+++ b/src/test/regress/expected/without_overlaps.out
@@ -19,7 +19,7 @@ CREATE TABLE temporal_rng (
 	id INTEGER,
 	CONSTRAINT temporal_rng_pk PRIMARY KEY (id, valid_at WITHOUT OVERLAPS)
 );
-ERROR:  range or PERIOD "valid_at" in WITHOUT OVERLAPS does not exist
+ERROR:  column "valid_at" named in key does not exist
 -- PK with a non-range column:
 CREATE TABLE temporal_rng (
 	id INTEGER,
@@ -27,6 +27,8 @@ CREATE TABLE temporal_rng (
 	CONSTRAINT temporal_rng_pk PRIMARY KEY (id, valid_at WITHOUT OVERLAPS)
 );
 ERROR:  column "valid_at" in WITHOUT OVERLAPS is not a range type
+LINE 3:  valid_at TEXT,
+         ^
 -- PK with one column plus a range:
 CREATE TABLE temporal_rng (
 	-- Since we can't depend on having btree_gist here,
@@ -110,7 +112,7 @@ CREATE TABLE temporal_rng2 (
 	id INTEGER,
 	CONSTRAINT temporal_rng2_uq UNIQUE (id, valid_at WITHOUT OVERLAPS)
 );
-ERROR:  range or PERIOD "valid_at" in WITHOUT OVERLAPS does not exist
+ERROR:  column "valid_at" named in key does not exist
 -- UNIQUE with a non-range column:
 CREATE TABLE temporal_rng2 (
 	id INTEGER,
@@ -118,6 +120,8 @@ CREATE TABLE temporal_rng2 (
 	CONSTRAINT temporal_rng2_uq UNIQUE (id, valid_at WITHOUT OVERLAPS)
 );
 ERROR:  column "valid_at" in WITHOUT OVERLAPS is not a range type
+LINE 3:  valid_at TEXT,
+         ^
 -- UNIQUE with one column plus a range:
 CREATE TABLE temporal_rng2 (
 	-- Since we can't depend on having btree_gist here,
-- 
2.34.1