hi.
it would be better to make all ereport in gram.y print out the
relative location.
one benefit is quickly locating the error.
in insertSelectOptions, some ereport won't call parser_errposition.
To handle that
I added a location to struct SelectLimit, so we can catch the location
in the LIMIT/FETCH/OFFSET clause.
in insertSelectOptions the error is
errmsg("WITH TIES cannot be specified without
ORDER BY clause"),
errmsg("%s and %s options cannot be used
together","SKIP LOCKED", "WITH TIES"),
to capture the exact location, i use the following way:
| FETCH first_or_next select_fetch_first_value
row_or_rows WITH TIES
@@ -13222,6 +13247,7 @@ limit_clause:
n->limitOffset = NULL;
n->limitCount = $3;
n->limitOption = LIMIT_OPTION_WITH_TIES;
+ n->location = @5;
$$ = n;
but normally we do `n->location = @1;`
not sure my change in insertSelectOptions is correct.
For other places in gram.y, I guess my change is sane.
i manually checked, after my patch, all ereport function calls in
gram.y will print out the relative location.
From c8f95f5b681b6871e76b25bc2619ad9cbcc3f688 Mon Sep 17 00:00:00 2001
From: jian he <jian.universality@gmail.com>
Date: Sat, 26 Oct 2024 23:04:22 +0800
Subject: [PATCH v1 1/1] add cursor position in various ereport function in
gram.y
inspired by this[1],
[1]: https://www.postgresql.org/message-id/3482022.1728839631%40sss.pgh.pa.us
but i am not so sure the handing in insertSelectOptions is correct.
discussion:
---
src/backend/parser/gram.y | 90 +++++++++++--------
src/include/nodes/parsenodes.h | 1 +
.../regress/expected/create_function_sql.out | 11 +++
src/test/regress/expected/create_table.out | 2 +
src/test/regress/expected/foreign_key.out | 4 +
src/test/regress/expected/limit.out | 4 +
src/test/regress/expected/sqljson.out | 4 +
src/test/regress/sql/create_function_sql.sql | 7 ++
src/test/regress/sql/foreign_key.sql | 1 +
9 files changed, 86 insertions(+), 38 deletions(-)
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index baca4059d2..33f4cdb0b0 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -115,6 +115,7 @@ typedef struct SelectLimit
Node *limitOffset;
Node *limitCount;
LimitOption limitOption;
+ ParseLoc location;
} SelectLimit;
/* Private struct for the result of group_clause production */
@@ -188,7 +189,7 @@ static Node *makeSQLValueFunction(SQLValueFunctionOp op, int32 typmod,
int location);
static Node *makeXmlExpr(XmlExprOp op, char *name, List *named_args,
List *args, int location);
-static List *mergeTableFuncParameters(List *func_args, List *columns);
+static List *mergeTableFuncParameters(List *func_args, List *columns, core_yyscan_t yyscanner);
static TypeName *TableFuncTypeName(List *columns);
static RangeVar *makeRangeVarFromAnyName(List *names, int position, core_yyscan_t yyscanner);
static RangeVar *makeRangeVarFromQualifiedName(char *name, List *namelist, int location,
@@ -199,7 +200,6 @@ static void SplitColQualList(List *qualList,
static void processCASbits(int cas_bits, int location, const char *constrType,
bool *deferrable, bool *initdeferred, bool *not_valid,
bool *no_inherit, core_yyscan_t yyscanner);
-static PartitionStrategy parsePartitionStrategy(char *strategy);
static void preprocess_pubobj_list(List *pubobjspec_list,
core_yyscan_t yyscanner);
static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
@@ -3149,11 +3149,14 @@ PartitionBoundSpec:
if (n->modulus == -1)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("modulus for hash partition must be specified")));
+ errmsg("modulus for hash partition must be specified"),
+ parser_errposition(@4)));
+
if (n->remainder == -1)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("remainder for hash partition must be specified")));
+ errmsg("remainder for hash partition must be specified"),
+ parser_errposition(@4)));
n->location = @3;
@@ -4532,7 +4535,18 @@ PartitionSpec: PARTITION BY ColId '(' part_params ')'
{
PartitionSpec *n = makeNode(PartitionSpec);
- n->strategy = parsePartitionStrategy($3);
+ if (!pg_strcasecmp($3, "list"))
+ n->strategy = PARTITION_STRATEGY_LIST;
+ else if (!pg_strcasecmp($3, "range"))
+ n->strategy = PARTITION_STRATEGY_RANGE;
+ else if (!pg_strcasecmp($3, "hash"))
+ n->strategy = PARTITION_STRATEGY_HASH;
+ else
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("unrecognized partitioning strategy \"%s\"", $3),
+ parser_errposition(@3)));
+
n->partParams = $5;
n->location = @1;
@@ -5966,7 +5980,8 @@ CreateTrigStmt:
if (n->replace) /* not supported, see CreateTrigger */
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("CREATE OR REPLACE CONSTRAINT TRIGGER is not supported")));
+ errmsg("CREATE OR REPLACE CONSTRAINT TRIGGER is not supported"),
+ parser_errposition(@2)));
n->isconstraint = true;
n->trigname = $5;
n->relation = $9;
@@ -6251,7 +6266,8 @@ CreateAssertionStmt:
{
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("CREATE ASSERTION is not yet implemented")));
+ errmsg("CREATE ASSERTION is not yet implemented"),
+ parser_errposition(@2)));
$$ = NULL;
}
@@ -8294,7 +8310,7 @@ CreateFunctionStmt:
n->is_procedure = false;
n->replace = $2;
n->funcname = $4;
- n->parameters = mergeTableFuncParameters($5, $9);
+ n->parameters = mergeTableFuncParameters($5, $9, yyscanner);
n->returnType = TableFuncTypeName($9);
n->returnType->location = @7;
n->options = $11;
@@ -8427,6 +8443,7 @@ func_arg:
n->argType = $3;
n->mode = $1;
n->defexpr = NULL;
+ n->location = @1;
$$ = n;
}
| param_name arg_class func_type
@@ -8437,6 +8454,7 @@ func_arg:
n->argType = $3;
n->mode = $2;
n->defexpr = NULL;
+ n->location = @1;
$$ = n;
}
| param_name func_type
@@ -8447,6 +8465,7 @@ func_arg:
n->argType = $2;
n->mode = FUNC_PARAM_DEFAULT;
n->defexpr = NULL;
+ n->location = @1;
$$ = n;
}
| arg_class func_type
@@ -8457,6 +8476,7 @@ func_arg:
n->argType = $2;
n->mode = $1;
n->defexpr = NULL;
+ n->location = @1;
$$ = n;
}
| func_type
@@ -8467,6 +8487,7 @@ func_arg:
n->argType = $1;
n->mode = FUNC_PARAM_DEFAULT;
n->defexpr = NULL;
+ n->location = @1;
$$ = n;
}
;
@@ -8803,6 +8824,7 @@ table_func_column: param_name func_type
n->argType = $2;
n->mode = FUNC_PARAM_TABLE;
n->defexpr = NULL;
+ n->location = @1;
$$ = n;
}
;
@@ -13171,6 +13193,7 @@ select_limit:
n->limitOffset = $1;
n->limitCount = NULL;
n->limitOption = LIMIT_OPTION_COUNT;
+ n->location = @1;
$$ = n;
}
;
@@ -13188,6 +13211,7 @@ limit_clause:
n->limitOffset = NULL;
n->limitCount = $2;
n->limitOption = LIMIT_OPTION_COUNT;
+ n->location = @1;
$$ = n;
}
| LIMIT select_limit_value ',' select_offset_value
@@ -13213,6 +13237,7 @@ limit_clause:
n->limitOffset = NULL;
n->limitCount = $3;
n->limitOption = LIMIT_OPTION_COUNT;
+ n->location = @1;
$$ = n;
}
| FETCH first_or_next select_fetch_first_value row_or_rows WITH TIES
@@ -13222,6 +13247,7 @@ limit_clause:
n->limitOffset = NULL;
n->limitCount = $3;
n->limitOption = LIMIT_OPTION_WITH_TIES;
+ n->location = @5;
$$ = n;
}
| FETCH first_or_next row_or_rows ONLY
@@ -13231,6 +13257,7 @@ limit_clause:
n->limitOffset = NULL;
n->limitCount = makeIntConst(1, -1);
n->limitOption = LIMIT_OPTION_COUNT;
+ n->location = @1;
$$ = n;
}
| FETCH first_or_next row_or_rows WITH TIES
@@ -13240,6 +13267,7 @@ limit_clause:
n->limitOffset = NULL;
n->limitCount = makeIntConst(1, -1);
n->limitOption = LIMIT_OPTION_WITH_TIES;
+ n->location = @1;
$$ = n;
}
;
@@ -16952,8 +16980,9 @@ json_format_clause:
encoding = JS_ENC_UTF32;
else
ereport(ERROR,
- errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("unrecognized JSON encoding: %s", $4));
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("unrecognized JSON encoding: %s", $4),
+ parser_errposition(@4)));
$$ = (Node *) makeJsonFormat(JS_FORMAT_JSON, encoding, @1);
}
@@ -17455,7 +17484,8 @@ PLpgSQL_Expr: opt_distinct_clause opt_target_list
$9->limitOption == LIMIT_OPTION_WITH_TIES)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("WITH TIES cannot be specified without ORDER BY clause")));
+ errmsg("WITH TIES cannot be specified without ORDER BY clause"),
+ parser_errposition(@9)));
n->limitOption = $9->limitOption;
}
n->lockingClause = $10;
@@ -18977,11 +19007,15 @@ insertSelectOptions(SelectStmt *stmt,
if (stmt->limitOption)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("multiple limit options not allowed")));
+ errmsg("multiple limit options not allowed"),
+ parser_errposition(limitClause->location)));
+
if (!stmt->sortClause && limitClause->limitOption == LIMIT_OPTION_WITH_TIES)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("WITH TIES cannot be specified without ORDER BY clause")));
+ errmsg("WITH TIES cannot be specified without ORDER BY clause"),
+ parser_errposition(limitClause->location)));
+
if (limitClause->limitOption == LIMIT_OPTION_WITH_TIES && stmt->lockingClause)
{
ListCell *lc;
@@ -18994,7 +19028,8 @@ insertSelectOptions(SelectStmt *stmt,
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
errmsg("%s and %s options cannot be used together",
- "SKIP LOCKED", "WITH TIES")));
+ "SKIP LOCKED", "WITH TIES"),
+ parser_errposition(limitClause->location)));
}
}
stmt->limitOption = limitClause->limitOption;
@@ -19187,7 +19222,7 @@ makeXmlExpr(XmlExprOp op, char *name, List *named_args, List *args,
* Merge the input and output parameters of a table function.
*/
static List *
-mergeTableFuncParameters(List *func_args, List *columns)
+mergeTableFuncParameters(List *func_args, List *columns, core_yyscan_t yyscanner)
{
ListCell *lc;
@@ -19201,7 +19236,8 @@ mergeTableFuncParameters(List *func_args, List *columns)
p->mode != FUNC_PARAM_VARIADIC)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("OUT and INOUT arguments aren't allowed in TABLE functions")));
+ errmsg("OUT and INOUT arguments aren't allowed in TABLE functions"),
+ parser_errposition(p->location)));
}
return list_concat(func_args, columns);
@@ -19417,28 +19453,6 @@ processCASbits(int cas_bits, int location, const char *constrType,
}
}
-/*
- * Parse a user-supplied partition strategy string into parse node
- * PartitionStrategy representation, or die trying.
- */
-static PartitionStrategy
-parsePartitionStrategy(char *strategy)
-{
- if (pg_strcasecmp(strategy, "list") == 0)
- return PARTITION_STRATEGY_LIST;
- else if (pg_strcasecmp(strategy, "range") == 0)
- return PARTITION_STRATEGY_RANGE;
- else if (pg_strcasecmp(strategy, "hash") == 0)
- return PARTITION_STRATEGY_HASH;
-
- ereport(ERROR,
- (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("unrecognized partitioning strategy \"%s\"",
- strategy)));
- return PARTITION_STRATEGY_LIST; /* keep compiler quiet */
-
-}
-
/*
* Process pubobjspec_list to check for errors in any of the objects and
* convert PUBLICATIONOBJ_CONTINUATION into appropriate PublicationObjSpecType.
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index b40b661ec8..0d96db5638 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -3482,6 +3482,7 @@ typedef struct FunctionParameter
TypeName *argType; /* TypeName for parameter type */
FunctionParameterMode mode; /* IN/OUT/etc */
Node *defexpr; /* raw default expr, or NULL if not given */
+ ParseLoc location; /* token location, or -1 if unknown */
} FunctionParameter;
typedef struct AlterFunctionStmt
diff --git a/src/test/regress/expected/create_function_sql.out b/src/test/regress/expected/create_function_sql.out
index 50aca5940f..a44986afa3 100644
--- a/src/test/regress/expected/create_function_sql.out
+++ b/src/test/regress/expected/create_function_sql.out
@@ -49,6 +49,17 @@ SELECT functest_A_3();
f
(1 row)
+-- OUT and INOUT arguments aren't allowed in TABLE functions
+CREATE FUNCTION fn_intout(out c int)
+RETURNS TABLE(a int) LANGUAGE sql AS $BODY$ select 1 $BODY$;
+ERROR: OUT and INOUT arguments aren't allowed in TABLE functions
+LINE 1: CREATE FUNCTION fn_intout(out c int)
+ ^
+CREATE FUNCTION fn_intout(inout c int)
+RETURNS TABLE(a int) LANGUAGE sql AS $BODY$ select 1 $BODY$;
+ERROR: OUT and INOUT arguments aren't allowed in TABLE functions
+LINE 1: CREATE FUNCTION fn_intout(inout c int)
+ ^
--
-- IMMUTABLE | STABLE | VOLATILE
--
diff --git a/src/test/regress/expected/create_table.out b/src/test/regress/expected/create_table.out
index c45e02d42f..57a24050ab 100644
--- a/src/test/regress/expected/create_table.out
+++ b/src/test/regress/expected/create_table.out
@@ -198,6 +198,8 @@ CREATE TABLE partitioned (
a int
) PARTITION BY MAGIC (a);
ERROR: unrecognized partitioning strategy "magic"
+LINE 3: ) PARTITION BY MAGIC (a);
+ ^
-- specified column must be present in the table
CREATE TABLE partitioned (
a int
diff --git a/src/test/regress/expected/foreign_key.out b/src/test/regress/expected/foreign_key.out
index b73e7dced8..ed16dea924 100644
--- a/src/test/regress/expected/foreign_key.out
+++ b/src/test/regress/expected/foreign_key.out
@@ -2916,6 +2916,10 @@ $$;
CREATE TRIGGER trig_upd_pk AFTER UPDATE ON fkpart11.pk FOR EACH ROW EXECUTE FUNCTION fkpart11.print_row();
CREATE TRIGGER trig_del_pk AFTER DELETE ON fkpart11.pk FOR EACH ROW EXECUTE FUNCTION fkpart11.print_row();
CREATE TRIGGER trig_ins_pk AFTER INSERT ON fkpart11.pk FOR EACH ROW EXECUTE FUNCTION fkpart11.print_row();
+CREATE OR REPLACE CONSTRAINT TRIGGER trig_upd_fk_parted AFTER UPDATE ON fkpart11.fk_parted INITIALLY DEFERRED FOR EACH ROW EXECUTE FUNCTION fkpart11.print_row();
+ERROR: CREATE OR REPLACE CONSTRAINT TRIGGER is not supported
+LINE 1: CREATE OR REPLACE CONSTRAINT TRIGGER trig_upd_fk_parted AFTE...
+ ^
CREATE CONSTRAINT TRIGGER trig_upd_fk_parted AFTER UPDATE ON fkpart11.fk_parted INITIALLY DEFERRED FOR EACH ROW EXECUTE FUNCTION fkpart11.print_row();
CREATE CONSTRAINT TRIGGER trig_del_fk_parted AFTER DELETE ON fkpart11.fk_parted INITIALLY DEFERRED FOR EACH ROW EXECUTE FUNCTION fkpart11.print_row();
CREATE CONSTRAINT TRIGGER trig_ins_fk_parted AFTER INSERT ON fkpart11.fk_parted INITIALLY DEFERRED FOR EACH ROW EXECUTE FUNCTION fkpart11.print_row();
diff --git a/src/test/regress/expected/limit.out b/src/test/regress/expected/limit.out
index a2cd0f9f5b..f4267c002d 100644
--- a/src/test/regress/expected/limit.out
+++ b/src/test/regress/expected/limit.out
@@ -624,11 +624,15 @@ SELECT thousand
FROM onek WHERE thousand < 5
ORDER BY thousand FETCH FIRST 1 ROW WITH TIES FOR UPDATE SKIP LOCKED;
ERROR: SKIP LOCKED and WITH TIES options cannot be used together
+LINE 3: ORDER BY thousand FETCH FIRST 1 ROW WITH TIES FOR UPDATE S...
+ ^
-- should fail
SELECT ''::text AS two, unique1, unique2, stringu1
FROM onek WHERE unique1 > 50
FETCH FIRST 2 ROW WITH TIES;
ERROR: WITH TIES cannot be specified without ORDER BY clause
+LINE 3: FETCH FIRST 2 ROW WITH TIES;
+ ^
-- test ruleutils
CREATE VIEW limit_thousand_v_1 AS SELECT thousand FROM onek WHERE thousand < 995
ORDER BY thousand FETCH FIRST 5 ROWS WITH TIES OFFSET 10;
diff --git a/src/test/regress/expected/sqljson.out b/src/test/regress/expected/sqljson.out
index 70721c9a5a..435d61dd4a 100644
--- a/src/test/regress/expected/sqljson.out
+++ b/src/test/regress/expected/sqljson.out
@@ -337,6 +337,8 @@ LINE 1: SELECT JSON_OBJECT(RETURNING text FORMAT JSON ENCODING UTF8)...
^
SELECT JSON_OBJECT(RETURNING text FORMAT JSON ENCODING INVALID_ENCODING);
ERROR: unrecognized JSON encoding: invalid_encoding
+LINE 1: ...T JSON_OBJECT(RETURNING text FORMAT JSON ENCODING INVALID_EN...
+ ^
SELECT JSON_OBJECT(RETURNING bytea);
json_object
-------------
@@ -620,6 +622,8 @@ LINE 1: SELECT JSON_ARRAY(RETURNING text FORMAT JSON ENCODING UTF8);
^
SELECT JSON_ARRAY(RETURNING text FORMAT JSON ENCODING INVALID_ENCODING);
ERROR: unrecognized JSON encoding: invalid_encoding
+LINE 1: ...CT JSON_ARRAY(RETURNING text FORMAT JSON ENCODING INVALID_EN...
+ ^
SELECT JSON_ARRAY(RETURNING bytea);
json_array
------------
diff --git a/src/test/regress/sql/create_function_sql.sql b/src/test/regress/sql/create_function_sql.sql
index 89e9af3a49..384a7c0c3a 100644
--- a/src/test/regress/sql/create_function_sql.sql
+++ b/src/test/regress/sql/create_function_sql.sql
@@ -35,6 +35,13 @@ SELECT functest_A_1('abcd', '2020-01-01');
SELECT functest_A_2(ARRAY['1', '2', '3']);
SELECT functest_A_3();
+-- OUT and INOUT arguments aren't allowed in TABLE functions
+CREATE FUNCTION fn_intout(out c int)
+RETURNS TABLE(a int) LANGUAGE sql AS $BODY$ select 1 $BODY$;
+
+CREATE FUNCTION fn_intout(inout c int)
+RETURNS TABLE(a int) LANGUAGE sql AS $BODY$ select 1 $BODY$;
+
--
-- IMMUTABLE | STABLE | VOLATILE
--
diff --git a/src/test/regress/sql/foreign_key.sql b/src/test/regress/sql/foreign_key.sql
index 9b2a6b6bff..393e8524f8 100644
--- a/src/test/regress/sql/foreign_key.sql
+++ b/src/test/regress/sql/foreign_key.sql
@@ -2079,6 +2079,7 @@ $$;
CREATE TRIGGER trig_upd_pk AFTER UPDATE ON fkpart11.pk FOR EACH ROW EXECUTE FUNCTION fkpart11.print_row();
CREATE TRIGGER trig_del_pk AFTER DELETE ON fkpart11.pk FOR EACH ROW EXECUTE FUNCTION fkpart11.print_row();
CREATE TRIGGER trig_ins_pk AFTER INSERT ON fkpart11.pk FOR EACH ROW EXECUTE FUNCTION fkpart11.print_row();
+CREATE OR REPLACE CONSTRAINT TRIGGER trig_upd_fk_parted AFTER UPDATE ON fkpart11.fk_parted INITIALLY DEFERRED FOR EACH ROW EXECUTE FUNCTION fkpart11.print_row();
CREATE CONSTRAINT TRIGGER trig_upd_fk_parted AFTER UPDATE ON fkpart11.fk_parted INITIALLY DEFERRED FOR EACH ROW EXECUTE FUNCTION fkpart11.print_row();
CREATE CONSTRAINT TRIGGER trig_del_fk_parted AFTER DELETE ON fkpart11.fk_parted INITIALLY DEFERRED FOR EACH ROW EXECUTE FUNCTION fkpart11.print_row();
CREATE CONSTRAINT TRIGGER trig_ins_fk_parted AFTER INSERT ON fkpart11.fk_parted INITIALLY DEFERRED FOR EACH ROW EXECUTE FUNCTION fkpart11.print_row();
--
2.34.1
