fix: error referencing non-returned vars in ORDER BY

- Fixes AGV2-324

Author: MuhammadTahaNaveed

src/backend/parser/parse_cypher_expr.c

@@ -2526,32 +2526,67 @@ transformCypherOrderBy(ParseState *pstate, List *sortitems, List **targetlist)
 	foreach(lsi, sortitems)
 	{
 		SortBy	   *sortby = lfirst(lsi);
-		Node	   *expr;
+		Node	   *expr = NULL;
 		ListCell   *lt;
 		TargetEntry *te = NULL;
 
-		expr = transformCypherExpr(pstate, sortby->node, exprKind);
-
-		foreach(lt, *targetlist)
+		/*
+		 * First, check if this is a simple column reference that matches
+		 * an alias in the target list. This allows ORDER BY to reference
+		 * RETURN clause aliases (e.g., "RETURN p.name AS name ORDER BY name")
+		 */
+		if (sortby->node && IsA(sortby->node, ColumnRef))
 		{
-			TargetEntry *tmp;
-			Node	   *texpr;
+			ColumnRef  *cref = (ColumnRef *) sortby->node;
 
-			tmp = lfirst(lt);
-			texpr = strip_implicit_coercions((Node *) tmp->expr);
-			if (equal(texpr, expr))
+			if (list_length(cref->fields) == 1 &&
+				IsA(linitial(cref->fields), String))
 			{
-				te = tmp;
-				break;
+				char	   *colname = strVal(linitial(cref->fields));
+
+				/* Try to find a matching alias in the target list */
+				foreach(lt, *targetlist)
+				{
+					TargetEntry *tmp = lfirst(lt);
+
+					if (tmp->resname && strcmp(tmp->resname, colname) == 0)
+					{
+						te = tmp;
+						break;
+					}
+				}
 			}
 		}
 
+		/*
+		 * If not found as an alias, transform the expression and check if
+		 * it matches an expression in the target list or add it as resjunk
+		 */
 		if (te == NULL)
 		{
-			te = transformTargetEntry(pstate, sortby->node, expr, exprKind,
-									  NULL, true);
+			expr = transformCypherExpr(pstate, sortby->node, exprKind);
+
+			foreach(lt, *targetlist)
+			{
+				TargetEntry *tmp;
+				Node	   *texpr;
 
-			*targetlist = lappend(*targetlist, te);
+				tmp = lfirst(lt);
+				texpr = strip_implicit_coercions((Node *) tmp->expr);
+				if (equal(texpr, expr))
+				{
+					te = tmp;
+					break;
+				}
+			}
+
+			if (te == NULL)
+			{
+				te = transformTargetEntry(pstate, sortby->node, expr, exprKind,
+										  NULL, true);
+
+				*targetlist = lappend(*targetlist, te);
+			}
 		}
 
 		sortgroups = addTargetToSortList(pstate, te, sortgroups, *targetlist,

src/backend/parser/parse_graph.c

@@ -11,6 +11,7 @@
 #include "postgres.h"
 
 #include "access/htup_details.h"
+#include "access/stratnum.h"
 #include "ag_const.h"
 #include "catalog/ag_graph_fn.h"
 #include "catalog/ag_label.h"
@@ -48,6 +49,7 @@
 #include "access/genam.h"
 #include "parser/parse_shortestpath.h"
 #include "catalog/ag_vertex_d.h"
+#include "optimizer/optimizer.h"
 #include "catalog/ag_edge_d.h"
 
 #define EDGE_UNION_START_ID		"_start"
@@ -140,6 +142,7 @@ typedef struct
 
 /* projection (RETURN and WITH) */
 static void checkNameInItems(ParseState *pstate, List *items, List *targetList);
+static void updateSortOperatorsForJsonb(List *sortClause, List *targetList);
 
 /* MATCH - OPTIONAL */
 static ParseNamespaceItem *transformMatchOptional(ParseState *pstate,
@@ -451,6 +454,55 @@ transformCypherSubPattern(ParseState *pstate, CypherSubPattern *subpat)
 	return qry;
 }
 
+static void
+updateSortOperatorsForJsonb(List *sortClause, List *targetList)
+{
+	ListCell   *lc;
+
+	foreach(lc, sortClause)
+	{
+		SortGroupClause *sortcl = (SortGroupClause *) lfirst(lc);
+		TargetEntry *tle;
+		Oid			restype;
+		Oid			sortop;
+		Oid			eqop;
+		bool		hashable;
+		Oid			opfamily;
+		Oid			opcintype;
+		int16		strategy;
+
+		tle = get_sortgroupref_tle(sortcl->tleSortGroupRef, targetList);
+		if (!tle)
+			continue;
+
+		restype = exprType((Node *) tle->expr);
+
+		if (!get_ordering_op_properties(sortcl->sortop,
+										&opfamily, &opcintype, &strategy))
+			continue;
+
+		if (strategy == BTLessStrategyNumber)
+		{
+			get_sort_group_operators(restype,
+									 true, true, false,
+									 &sortop, &eqop, NULL,
+									 &hashable);
+		}
+		else if (strategy == BTGreaterStrategyNumber)
+		{
+			get_sort_group_operators(restype,
+									 false, true, true,
+									 NULL, &eqop, &sortop,
+									 &hashable);
+		}
+		else
+			continue;
+
+		sortcl->eqop = eqop;
+		sortcl->sortop = sortop;
+	}
+}
+
 Query *
 transformCypherProjection(ParseState *pstate, CypherClause *clause)
 {
@@ -478,68 +530,43 @@ transformCypherProjection(ParseState *pstate, CypherClause *clause)
 		qual = transformCypherWhere(pstate, where, EXPR_KIND_WHERE);
 		qual = resolve_future_vertex(pstate, qual, 0);
 	}
-	else if (detail->distinct != NULL || detail->order != NULL ||
-			 detail->skip != NULL || detail->limit != NULL)
+	else
 	{
-		List	   *distinct = detail->distinct;
-		List	   *order = detail->order;
-		Node	   *skip = detail->skip;
-		Node	   *limit = detail->limit;
-
-		/*
-		 * detach options so that this function passes through this if
-		 * statement when the function is called again recursively
-		 */
-		detail->distinct = NIL;
-		detail->order = NIL;
-		detail->skip = NULL;
-		detail->limit = NULL;
-		nsitem = transformClause(pstate, (Node *) clause);
-		detail->distinct = distinct;
-		detail->order = order;
-		detail->skip = skip;
-		detail->limit = limit;
+		if (clause->prev != NULL)
+			transformClause(pstate, clause->prev);
 
-		qry->targetList = makeTargetListFromNSItem(pstate, nsitem);
+		qry->targetList = transformItemList(pstate, detail->items,
+											EXPR_KIND_SELECT_TARGET);
 
-		qry->sortClause = transformCypherOrderBy(pstate, order,
-												 &qry->targetList);
+		if (detail->kind == CP_WITH)
+			checkNameInItems(pstate, detail->items, qry->targetList);
 
-		if (distinct == NIL)
+		if (detail->order)
 		{
-			/* intentionally blank, do nothing */
+			qry->sortClause = transformCypherOrderBy(pstate, detail->order,
+													 &qry->targetList);
 		}
-		else
+
+		if (detail->distinct)
 		{
-			Assert(linitial(distinct) == NULL);
+			Assert(linitial(detail->distinct) == NULL);
 
 			qry->distinctClause = transformDistinctClause(pstate,
 														  &qry->targetList,
 														  qry->sortClause,
 														  false);
 		}
 
-		qry->limitOffset = transformCypherLimit(pstate, skip, EXPR_KIND_OFFSET,
-												"SKIP");
-		qry->limitOffset = resolve_future_vertex(pstate, qry->limitOffset, 0);
-
-		qry->limitCount = transformCypherLimit(pstate, limit, EXPR_KIND_LIMIT,
-											   "LIMIT");
+		qry->limitCount = transformCypherLimit(pstate, detail->limit,
+												EXPR_KIND_LIMIT, "LIMIT");
 		qry->limitCount = resolve_future_vertex(pstate, qry->limitCount, 0);
-	}
-	else
-	{
-		if (clause->prev != NULL)
-			transformClause(pstate, clause->prev);
 
-		qry->targetList = transformItemList(pstate, detail->items,
-											EXPR_KIND_SELECT_TARGET);
-
-		if (detail->kind == CP_WITH)
-			checkNameInItems(pstate, detail->items, qry->targetList);
+		qry->limitOffset = transformCypherLimit(pstate, detail->skip,
+												EXPR_KIND_OFFSET, "SKIP");
+		qry->limitOffset = resolve_future_vertex(pstate, qry->limitOffset, 0);
 
 		qry->groupClause = generateGroupClause(pstate, &qry->targetList,
-											   qry->sortClause);
+												qry->sortClause);
 	}
 
 	if (detail->kind == CP_WITH)
@@ -570,6 +597,7 @@ transformCypherProjection(ParseState *pstate, CypherClause *clause)
 	{
 		flags = 0;
 	}
+
 	qry->targetList = (List *) resolve_future_vertex(pstate,
 													 (Node *) qry->targetList,
 													 flags);
@@ -578,7 +606,16 @@ transformCypherProjection(ParseState *pstate, CypherClause *clause)
 	qual = qualAndExpr(qual, pstate->p_resolved_qual);
 
 	if (detail->kind == CP_RETURN)
+	{
 		resolveItemList(pstate, qry->targetList);
+		/*
+		 * After applying JSONB coercion, update sort operators in
+		 * sortClause to use JSONB comparison operators instead
+		 * of the original type's operators
+		 */
+		if (qry->sortClause)
+			updateSortOperatorsForJsonb(qry->sortClause, qry->targetList);
+	}
 
 	qry->rtable = pstate->p_rtable;
 	qry->jointree = makeFromExpr(pstate->p_joinlist, qual);
@@ -3744,6 +3781,9 @@ resolve_future_vertex(ParseState *pstate, Node *node, int flags)
 {
 	resolve_future_vertex_context ctx;
 
+	if (node == NULL)
+		return NULL;
+
 	ctx.pstate = pstate;
 	ctx.flags = flags;
 	ctx.sublevels_up = 0;

src/test/regress/expected/cypher_dml.out

@@ -4154,6 +4154,54 @@ MATCH (v1:new_v1)-[e1:new_e1]->(v2:new_v1) RETURN v1,e1,v2;
  new_v1[3.3]{} | new_e1[4.2][3.3,3.4]{} | new_v1[3.4]{}
 (2 rows)
 
+--
+-- AGV2-324
+--
+CREATE GRAPH ag324;
+SET graph_path = ag324;
+CREATE (:person {name: 'Alice', age: 30}), 
+       (:person {name: 'Bob', age: 25}), 
+       (:person {name: 'Charlie', age: 35});
+MATCH (p:person)
+RETURN p.name AS name
+ORDER BY p.age;
+   name    
+-----------
+ "Bob"
+ "Alice"
+ "Charlie"
+(3 rows)
+
+MATCH (p:person)
+RETURN p.name AS name
+ORDER BY p.age DESC;
+   name    
+-----------
+ "Charlie"
+ "Alice"
+ "Bob"
+(3 rows)
+
+MATCH (p:person)
+RETURN p.name AS name
+ORDER BY name, p.age;
+   name    
+-----------
+ "Alice"
+ "Bob"
+ "Charlie"
+(3 rows)
+
+MATCH (p:person)
+RETURN p.name AS name
+ORDER BY p.age, name DESC;
+   name    
+-----------
+ "Bob"
+ "Alice"
+ "Charlie"
+(3 rows)
+
 -- cleanup
 DROP GRAPH srf CASCADE;
 NOTICE:  drop cascades to 5 other objects
@@ -4238,6 +4286,12 @@ drop cascades to vlabel ag_vertex
 drop cascades to elabel ag_edge
 drop cascades to vlabel person
 drop cascades to elabel knows
+DROP GRAPH ag324 CASCADE;
+NOTICE:  drop cascades to 4 other objects
+DETAIL:  drop cascades to sequence ag324.ag_label_seq
+drop cascades to vlabel ag_vertex
+drop cascades to elabel ag_edge
+drop cascades to vlabel person
 SET graph_path = agens;
 DROP VLABEL feature;
 DROP ELABEL supported;