Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit774171c

Browse files
committed
Improve reporting of errors in extension script files.
Previously, CREATE/ALTER EXTENSION gave basically no usefulcontext about errors reported while executing script files.I think the idea was that you could run the same commandsmanually to see the error, but that's often quite inconvenient.Let's improve that.If we get an error during raw parsing, we won't have a currentstatement identified by a RawStmt node, but we should always geta syntax error position. Show the portion of the script fromthe last semicolon-newline before the error position to the firstone after it. There are cases where this might show only afragment of a statement, but that should be uncommon, and itseems better than showing the whole script file.Without an error cursor, if we have gotten past raw parsing (whichwe probably have), we can report just the current SQL statement asan item of error context.In any case also report the script file name as error context,since it might not be entirely obvious which of a series ofupdate scripts failed. We can also show an approximate scriptline number in case whatever we printed of the query isn'tsufficiently identifiable.The error-context code path is already exercised by sometest_extensions test cases, but add tests for the syntax-errorpath.Discussion:https://postgr.es/m/ZvV1ClhnbJLCz7Sm@msg.df7cb.de
1 parent14e5680 commit774171c

File tree

8 files changed

+244
-3
lines changed

8 files changed

+244
-3
lines changed

‎src/backend/commands/extension.c

Lines changed: 165 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@
5454
#include"funcapi.h"
5555
#include"mb/pg_wchar.h"
5656
#include"miscadmin.h"
57+
#include"nodes/queryjumble.h"
5758
#include"storage/fd.h"
5859
#include"tcop/utility.h"
5960
#include"utils/acl.h"
@@ -107,6 +108,17 @@ typedef struct ExtensionVersionInfo
107108
structExtensionVersionInfo*previous;/* current best predecessor */
108109
}ExtensionVersionInfo;
109110

111+
/*
112+
* Information for script_error_callback()
113+
*/
114+
typedefstruct
115+
{
116+
constchar*sql;/* entire script file contents */
117+
constchar*filename;/* script file pathname */
118+
ParseLocstmt_location;/* current stmt start loc, or -1 if unknown */
119+
ParseLocstmt_len;/* length in bytes; 0 means "rest of string" */
120+
}script_error_callback_arg;
121+
110122
/* Local functions */
111123
staticList*find_update_path(List*evi_list,
112124
ExtensionVersionInfo*evi_start,
@@ -670,9 +682,139 @@ read_extension_script_file(const ExtensionControlFile *control,
670682
returndest_str;
671683
}
672684

685+
/*
686+
* error context callback for failures in script-file execution
687+
*/
688+
staticvoid
689+
script_error_callback(void*arg)
690+
{
691+
script_error_callback_arg*callback_arg= (script_error_callback_arg*)arg;
692+
constchar*query=callback_arg->sql;
693+
intlocation=callback_arg->stmt_location;
694+
intlen=callback_arg->stmt_len;
695+
intsyntaxerrposition;
696+
constchar*lastslash;
697+
698+
/*
699+
* If there is a syntax error position, convert to internal syntax error;
700+
* otherwise report the current query as an item of context stack.
701+
*
702+
* Note: we'll provide no context except the filename if there's neither
703+
* an error position nor any known current query. That shouldn't happen
704+
* though: all errors reported during raw parsing should come with an
705+
* error position.
706+
*/
707+
syntaxerrposition=geterrposition();
708+
if (syntaxerrposition>0)
709+
{
710+
/*
711+
* If we do not know the bounds of the current statement (as would
712+
* happen for an error occurring during initial raw parsing), we have
713+
* to use a heuristic to decide how much of the script to show. We'll
714+
* also use the heuristic in the unlikely case that syntaxerrposition
715+
* is outside what we think the statement bounds are.
716+
*/
717+
if (location<0||syntaxerrposition<location||
718+
(len>0&&syntaxerrposition>location+len))
719+
{
720+
/*
721+
* Our heuristic is pretty simple: look for semicolon-newline
722+
* sequences, and break at the last one strictly before
723+
* syntaxerrposition and the first one strictly after. It's
724+
* certainly possible to fool this with semicolon-newline embedded
725+
* in a string literal, but it seems better to do this than to
726+
* show the entire extension script.
727+
*/
728+
intslen=strlen(query);
729+
730+
location=len=0;
731+
for (intloc=0;loc<slen;loc++)
732+
{
733+
if (query[loc]!=';')
734+
continue;
735+
if (query[loc+1]=='\r')
736+
loc++;
737+
if (query[loc+1]=='\n')
738+
{
739+
intbkpt=loc+2;
740+
741+
if (bkpt<syntaxerrposition)
742+
location=bkpt;
743+
elseif (bkpt>syntaxerrposition)
744+
{
745+
len=bkpt-location;
746+
break;/* no need to keep searching */
747+
}
748+
}
749+
}
750+
}
751+
752+
/* Trim leading/trailing whitespace, for consistency */
753+
query=CleanQuerytext(query,&location,&len);
754+
755+
/*
756+
* Adjust syntaxerrposition. It shouldn't be pointing into the
757+
* whitespace we just trimmed, but cope if it is.
758+
*/
759+
syntaxerrposition-=location;
760+
if (syntaxerrposition<0)
761+
syntaxerrposition=0;
762+
elseif (syntaxerrposition>len)
763+
syntaxerrposition=len;
764+
765+
/* And report. */
766+
errposition(0);
767+
internalerrposition(syntaxerrposition);
768+
internalerrquery(pnstrdup(query,len));
769+
}
770+
elseif (location >=0)
771+
{
772+
/*
773+
* Since no syntax cursor will be shown, it's okay and helpful to trim
774+
* the reported query string to just the current statement.
775+
*/
776+
query=CleanQuerytext(query,&location,&len);
777+
errcontext("SQL statement \"%.*s\"",len,query);
778+
}
779+
780+
/*
781+
* Trim the reported file name to remove the path. We know that
782+
* get_extension_script_filename() inserted a '/', regardless of whether
783+
* we're on Windows.
784+
*/
785+
lastslash=strrchr(callback_arg->filename,'/');
786+
if (lastslash)
787+
lastslash++;
788+
else
789+
lastslash=callback_arg->filename;/* shouldn't happen, but cope */
790+
791+
/*
792+
* If we have a location (which, as said above, we really always should)
793+
* then report a line number to aid in localizing problems in big scripts.
794+
*/
795+
if (location >=0)
796+
{
797+
intlinenumber=1;
798+
799+
for (query=callback_arg->sql;*query;query++)
800+
{
801+
if (--location<0)
802+
break;
803+
if (*query=='\n')
804+
linenumber++;
805+
}
806+
errcontext("extension script file \"%s\", near line %d",
807+
lastslash,linenumber);
808+
}
809+
else
810+
errcontext("extension script file \"%s\"",lastslash);
811+
}
812+
673813
/*
674814
* Execute given SQL string.
675815
*
816+
* The filename the string came from is also provided, for error reporting.
817+
*
676818
* Note: it's tempting to just use SPI to execute the string, but that does
677819
* not work very well. The really serious problem is that SPI will parse,
678820
* analyze, and plan the whole string before executing any of it; of course
@@ -682,12 +824,27 @@ read_extension_script_file(const ExtensionControlFile *control,
682824
* could be very long.
683825
*/
684826
staticvoid
685-
execute_sql_string(constchar*sql)
827+
execute_sql_string(constchar*sql,constchar*filename)
686828
{
829+
script_error_callback_argcallback_arg;
830+
ErrorContextCallbackscripterrcontext;
687831
List*raw_parsetree_list;
688832
DestReceiver*dest;
689833
ListCell*lc1;
690834

835+
/*
836+
* Setup error traceback support for ereport().
837+
*/
838+
callback_arg.sql=sql;
839+
callback_arg.filename=filename;
840+
callback_arg.stmt_location=-1;
841+
callback_arg.stmt_len=-1;
842+
843+
scripterrcontext.callback=script_error_callback;
844+
scripterrcontext.arg= (void*)&callback_arg;
845+
scripterrcontext.previous=error_context_stack;
846+
error_context_stack=&scripterrcontext;
847+
691848
/*
692849
* Parse the SQL string into a list of raw parse trees.
693850
*/
@@ -709,6 +866,10 @@ execute_sql_string(const char *sql)
709866
List*stmt_list;
710867
ListCell*lc2;
711868

869+
/* Report location of this query for error context callback */
870+
callback_arg.stmt_location=parsetree->stmt_location;
871+
callback_arg.stmt_len=parsetree->stmt_len;
872+
712873
/*
713874
* We do the work for each parsetree in a short-lived context, to
714875
* limit the memory used when there are many commands in the string.
@@ -778,6 +939,8 @@ execute_sql_string(const char *sql)
778939
MemoryContextDelete(per_parsetree_context);
779940
}
780941

942+
error_context_stack=scripterrcontext.previous;
943+
781944
/* Be sure to advance the command counter after the last script command */
782945
CommandCounterIncrement();
783946
}
@@ -1054,7 +1217,7 @@ execute_extension_script(Oid extensionOid, ExtensionControlFile *control,
10541217
/* And now back to C string */
10551218
c_sql=text_to_cstring(DatumGetTextPP(t_sql));
10561219

1057-
execute_sql_string(c_sql);
1220+
execute_sql_string(c_sql,filename);
10581221
}
10591222
PG_FINALLY();
10601223
{

‎src/test/modules/test_extensions/Makefile

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,9 @@ EXTENSION = test_ext1 test_ext2 test_ext3 test_ext4 test_ext5 test_ext6 \
1313

1414
DATA = test_ext1--1.0.sql test_ext2--1.0.sql test_ext3--1.0.sql\
1515
test_ext4--1.0.sql test_ext5--1.0.sql test_ext6--1.0.sql\
16-
test_ext7--1.0.sql test_ext7--1.0--2.0.sql test_ext8--1.0.sql\
16+
test_ext7--1.0.sql test_ext7--1.0--2.0.sql\
17+
test_ext7--2.0--2.1bad.sql test_ext7--2.0--2.2bad.sql\
18+
test_ext8--1.0.sql\
1719
test_ext9--1.0.sql\
1820
test_ext_cine--1.0.sql test_ext_cine--1.0--1.1.sql\
1921
test_ext_cor--1.0.sql\

‎src/test/modules/test_extensions/expected/test_extensions.out

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,22 @@ Objects in extension "test_ext7"
7272
type ext7_table2[]
7373
(4 rows)
7474

75+
-- test reporting of errors in extension scripts
76+
alter extension test_ext7 update to '2.1bad';
77+
ERROR: syntax error at or near "FUNCTIN"
78+
LINE 1: CREATE FUNCTIN my_erroneous_func(int) RETURNS int LANGUAGE S...
79+
^
80+
QUERY: CREATE FUNCTIN my_erroneous_func(int) RETURNS int LANGUAGE SQL
81+
AS $$ SELECT $1 + 1 $$;
82+
CONTEXT: extension script file "test_ext7--2.0--2.1bad.sql", near line 10
83+
alter extension test_ext7 update to '2.2bad';
84+
ERROR: syntax error at or near ","
85+
LINE 1: SELECT $1 + , 1
86+
^
87+
QUERY: SELECT $1 + , 1
88+
CONTEXT: SQL statement "CREATE FUNCTION my_erroneous_func(int) RETURNS int LANGUAGE SQL
89+
AS $$ SELECT $1 + , 1 $$"
90+
extension script file "test_ext7--2.0--2.2bad.sql", near line 9
7591
-- test handling of temp objects created by extensions
7692
create extension test_ext8;
7793
-- \dx+ would expose a variable pg_temp_nn schema name, so we can't use it here
@@ -295,6 +311,9 @@ CREATE FUNCTION ext_cor_func() RETURNS text
295311
CREATE EXTENSION test_ext_cor; -- fail
296312
ERROR: function ext_cor_func() is not a member of extension "test_ext_cor"
297313
DETAIL: An extension is not allowed to replace an object that it does not own.
314+
CONTEXT: SQL statement "CREATE OR REPLACE FUNCTION ext_cor_func() RETURNS text
315+
AS $$ SELECT 'ext_cor_func: from extension'::text $$ LANGUAGE sql"
316+
extension script file "test_ext_cor--1.0.sql", near line 8
298317
SELECT ext_cor_func();
299318
ext_cor_func
300319
------------------------
@@ -307,6 +326,9 @@ CREATE VIEW ext_cor_view AS
307326
CREATE EXTENSION test_ext_cor; -- fail
308327
ERROR: view ext_cor_view is not a member of extension "test_ext_cor"
309328
DETAIL: An extension is not allowed to replace an object that it does not own.
329+
CONTEXT: SQL statement "CREATE OR REPLACE VIEW ext_cor_view AS
330+
SELECT 'ext_cor_view: from extension'::text AS col"
331+
extension script file "test_ext_cor--1.0.sql", near line 11
310332
SELECT ext_cor_func();
311333
ERROR: function ext_cor_func() does not exist
312334
LINE 1: SELECT ext_cor_func();
@@ -323,6 +345,8 @@ CREATE TYPE test_ext_type;
323345
CREATE EXTENSION test_ext_cor; -- fail
324346
ERROR: type test_ext_type is not a member of extension "test_ext_cor"
325347
DETAIL: An extension is not allowed to replace an object that it does not own.
348+
CONTEXT: SQL statement "CREATE TYPE test_ext_type AS ENUM('x', 'y')"
349+
extension script file "test_ext_cor--1.0.sql", near line 17
326350
DROP TYPE test_ext_type;
327351
-- this makes a shell "point <<@@ polygon" operator too
328352
CREATE OPERATOR @@>> ( PROCEDURE = poly_contain_pt,
@@ -331,6 +355,9 @@ CREATE OPERATOR @@>> ( PROCEDURE = poly_contain_pt,
331355
CREATE EXTENSION test_ext_cor; -- fail
332356
ERROR: operator <<@@(point,polygon) is not a member of extension "test_ext_cor"
333357
DETAIL: An extension is not allowed to replace an object that it does not own.
358+
CONTEXT: SQL statement "CREATE OPERATOR <<@@ ( PROCEDURE = pt_contained_poly,
359+
LEFTARG = point, RIGHTARG = polygon )"
360+
extension script file "test_ext_cor--1.0.sql", near line 19
334361
DROP OPERATOR <<@@ (point, polygon);
335362
CREATE EXTENSION test_ext_cor; -- now it should work
336363
SELECT ext_cor_func();
@@ -379,37 +406,52 @@ CREATE COLLATION ext_cine_coll
379406
CREATE EXTENSION test_ext_cine; -- fail
380407
ERROR: collation ext_cine_coll is not a member of extension "test_ext_cine"
381408
DETAIL: An extension may only use CREATE ... IF NOT EXISTS to skip object creation if the conflicting object is one that it already owns.
409+
CONTEXT: SQL statement "CREATE COLLATION IF NOT EXISTS ext_cine_coll
410+
( LC_COLLATE = "POSIX", LC_CTYPE = "POSIX" )"
411+
extension script file "test_ext_cine--1.0.sql", near line 10
382412
DROP COLLATION ext_cine_coll;
383413
CREATE MATERIALIZED VIEW ext_cine_mv AS SELECT 11 AS f1;
384414
CREATE EXTENSION test_ext_cine; -- fail
385415
ERROR: materialized view ext_cine_mv is not a member of extension "test_ext_cine"
386416
DETAIL: An extension may only use CREATE ... IF NOT EXISTS to skip object creation if the conflicting object is one that it already owns.
417+
CONTEXT: SQL statement "CREATE MATERIALIZED VIEW IF NOT EXISTS ext_cine_mv AS SELECT 42 AS f1"
418+
extension script file "test_ext_cine--1.0.sql", near line 13
387419
DROP MATERIALIZED VIEW ext_cine_mv;
388420
CREATE FOREIGN DATA WRAPPER dummy;
389421
CREATE SERVER ext_cine_srv FOREIGN DATA WRAPPER dummy;
390422
CREATE EXTENSION test_ext_cine; -- fail
391423
ERROR: server ext_cine_srv is not a member of extension "test_ext_cine"
392424
DETAIL: An extension may only use CREATE ... IF NOT EXISTS to skip object creation if the conflicting object is one that it already owns.
425+
CONTEXT: SQL statement "CREATE SERVER IF NOT EXISTS ext_cine_srv FOREIGN DATA WRAPPER ext_cine_fdw"
426+
extension script file "test_ext_cine--1.0.sql", near line 17
393427
DROP SERVER ext_cine_srv;
394428
CREATE SCHEMA ext_cine_schema;
395429
CREATE EXTENSION test_ext_cine; -- fail
396430
ERROR: schema ext_cine_schema is not a member of extension "test_ext_cine"
397431
DETAIL: An extension may only use CREATE ... IF NOT EXISTS to skip object creation if the conflicting object is one that it already owns.
432+
CONTEXT: SQL statement "CREATE SCHEMA IF NOT EXISTS ext_cine_schema"
433+
extension script file "test_ext_cine--1.0.sql", near line 19
398434
DROP SCHEMA ext_cine_schema;
399435
CREATE SEQUENCE ext_cine_seq;
400436
CREATE EXTENSION test_ext_cine; -- fail
401437
ERROR: sequence ext_cine_seq is not a member of extension "test_ext_cine"
402438
DETAIL: An extension may only use CREATE ... IF NOT EXISTS to skip object creation if the conflicting object is one that it already owns.
439+
CONTEXT: SQL statement "CREATE SEQUENCE IF NOT EXISTS ext_cine_seq"
440+
extension script file "test_ext_cine--1.0.sql", near line 21
403441
DROP SEQUENCE ext_cine_seq;
404442
CREATE TABLE ext_cine_tab1 (x int);
405443
CREATE EXTENSION test_ext_cine; -- fail
406444
ERROR: table ext_cine_tab1 is not a member of extension "test_ext_cine"
407445
DETAIL: An extension may only use CREATE ... IF NOT EXISTS to skip object creation if the conflicting object is one that it already owns.
446+
CONTEXT: SQL statement "CREATE TABLE IF NOT EXISTS ext_cine_tab1 (x int)"
447+
extension script file "test_ext_cine--1.0.sql", near line 23
408448
DROP TABLE ext_cine_tab1;
409449
CREATE TABLE ext_cine_tab2 AS SELECT 42 AS y;
410450
CREATE EXTENSION test_ext_cine; -- fail
411451
ERROR: table ext_cine_tab2 is not a member of extension "test_ext_cine"
412452
DETAIL: An extension may only use CREATE ... IF NOT EXISTS to skip object creation if the conflicting object is one that it already owns.
453+
CONTEXT: SQL statement "CREATE TABLE IF NOT EXISTS ext_cine_tab2 AS SELECT 42 AS y"
454+
extension script file "test_ext_cine--1.0.sql", near line 25
413455
DROP TABLE ext_cine_tab2;
414456
CREATE EXTENSION test_ext_cine;
415457
\dx+ test_ext_cine

‎src/test/modules/test_extensions/meson.build

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ test_install_data += files(
1515
'test_ext6.control',
1616
'test_ext7--1.0--2.0.sql',
1717
'test_ext7--1.0.sql',
18+
'test_ext7--2.0--2.1bad.sql',
19+
'test_ext7--2.0--2.2bad.sql',
1820
'test_ext7.control',
1921
'test_ext8--1.0.sql',
2022
'test_ext8.control',

‎src/test/modules/test_extensions/sql/test_extensions.sql

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,10 @@ create extension test_ext7;
2828
alter extension test_ext7update to'2.0';
2929
\dx+ test_ext7
3030

31+
-- test reporting of errors in extension scripts
32+
alter extension test_ext7update to'2.1bad';
33+
alter extension test_ext7update to'2.2bad';
34+
3135
-- test handling of temp objects created by extensions
3236
create extension test_ext8;
3337

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
/* src/test/modules/test_extensions/test_ext7--2.0--2.1bad.sql*/
2+
3+
-- complain if script is sourced in psql, rather than via ALTER EXTENSION
4+
\echo Use"ALTER EXTENSION test_ext7 UPDATE TO '2.1bad'" to load this file. \quit
5+
6+
-- test reporting of a simple syntax error in an extension script
7+
CREATEFUNCTIONmy_okay_func(int) RETURNSint LANGUAGE SQL
8+
AS $$SELECT $1+1 $$;
9+
10+
CREATE FUNCTIN my_erroneous_func(int) RETURNSint LANGUAGE SQL
11+
AS $$SELECT $1+1 $$;
12+
13+
CREATEFUNCTIONmy_other_func(int) RETURNSint LANGUAGE SQL
14+
AS $$SELECT $1+1 $$;
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
/* src/test/modules/test_extensions/test_ext7--2.0--2.2bad.sql*/
2+
3+
-- complain if script is sourced in psql, rather than via ALTER EXTENSION
4+
\echo Use"ALTER EXTENSION test_ext7 UPDATE TO '2.2bad'" to load this file. \quit
5+
6+
-- test reporting of a nested syntax error in an extension script
7+
SET LOCAL check_function_bodies=on;
8+
9+
CREATEFUNCTIONmy_erroneous_func(int) RETURNSint LANGUAGE SQL
10+
AS $$SELECT $1+ ,1 $$;
11+
12+
CREATEFUNCTIONmy_other_func(int) RETURNSint LANGUAGE SQL
13+
AS $$SELECT $1+1 $$;

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp