Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 2bfb50b

Browse files
deanrasheeddeanrasheed
committed
Improve reporting of "conflicting or redundant options" errors.
When reporting "conflicting or redundant options" errors, try to ensure that errposition() is used, to help the user identify the offending option. Formerly, errposition() was invoked in less than 60% of cases. This patch raises that to over 90%, but there remain a few places where the ParseState is not readily available. Using errdetail() might improve the error in such cases, but that is left as a task for the future. Additionally, since this error is thrown from over 100 places in the codebase, introduce a dedicated function to throw it, reducing code duplication. Extracted from a slightly larger patch by Vignesh C. Reviewed by Bharath Rupireddy, Alvaro Herrera, Dilip Kumar, Hou Zhijie, Peter Smith, Daniel Gustafsson, Julien Rouhaud and me. Discussion: https://postgr.es/m/CALDaNm33FFSS5tVyvmkoK2cCMuDVxcui=gFrjti9ROfynqSAGA@mail.gmail.com
1 parent ffc9dda commit 2bfb50b

22 files changed

+180
-395
lines changed

‎src/backend/catalog/aclchk.c

+3-8
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@
5959
#include "catalog/pg_ts_template.h"
6060
#include "catalog/pg_type.h"
6161
#include "commands/dbcommands.h"
62+
#include "commands/defrem.h"
6263
#include "commands/event_trigger.h"
6364
#include "commands/extension.h"
6465
#include "commands/proclang.h"
@@ -921,19 +922,13 @@ ExecAlterDefaultPrivilegesStmt(ParseState *pstate, AlterDefaultPrivilegesStmt *s
921922
if (strcmp(defel->defname, "schemas") == 0)
922923
{
923924
if (dnspnames)
924-
ereport(ERROR,
925-
(errcode(ERRCODE_SYNTAX_ERROR),
926-
errmsg("conflicting or redundant options"),
927-
parser_errposition(pstate, defel->location)));
925+
errorConflictingDefElem(defel, pstate);
928926
dnspnames = defel;
929927
}
930928
else if (strcmp(defel->defname, "roles") == 0)
931929
{
932930
if (drolespecs)
933-
ereport(ERROR,
934-
(errcode(ERRCODE_SYNTAX_ERROR),
935-
errmsg("conflicting or redundant options"),
936-
parser_errposition(pstate, defel->location)));
931+
errorConflictingDefElem(defel, pstate);
937932
drolespecs = defel;
938933
}
939934
else

‎src/backend/commands/copy.c

+12-47
Original file line numberDiff line numberDiff line change
@@ -357,10 +357,7 @@ ProcessCopyOptions(ParseState *pstate,
357357
char *fmt = defGetString(defel);
358358

359359
if (format_specified)
360-
ereport(ERROR,
361-
(errcode(ERRCODE_SYNTAX_ERROR),
362-
errmsg("conflicting or redundant options"),
363-
parser_errposition(pstate, defel->location)));
360+
errorConflictingDefElem(defel, pstate);
364361
format_specified = true;
365362
if (strcmp(fmt, "text") == 0)
366363
/* default format */ ;
@@ -377,66 +374,45 @@ ProcessCopyOptions(ParseState *pstate,
377374
else if (strcmp(defel->defname, "freeze") == 0)
378375
{
379376
if (freeze_specified)
380-
ereport(ERROR,
381-
(errcode(ERRCODE_SYNTAX_ERROR),
382-
errmsg("conflicting or redundant options"),
383-
parser_errposition(pstate, defel->location)));
377+
errorConflictingDefElem(defel, pstate);
384378
freeze_specified = true;
385379
opts_out->freeze = defGetBoolean(defel);
386380
}
387381
else if (strcmp(defel->defname, "delimiter") == 0)
388382
{
389383
if (opts_out->delim)
390-
ereport(ERROR,
391-
(errcode(ERRCODE_SYNTAX_ERROR),
392-
errmsg("conflicting or redundant options"),
393-
parser_errposition(pstate, defel->location)));
384+
errorConflictingDefElem(defel, pstate);
394385
opts_out->delim = defGetString(defel);
395386
}
396387
else if (strcmp(defel->defname, "null") == 0)
397388
{
398389
if (opts_out->null_print)
399-
ereport(ERROR,
400-
(errcode(ERRCODE_SYNTAX_ERROR),
401-
errmsg("conflicting or redundant options"),
402-
parser_errposition(pstate, defel->location)));
390+
errorConflictingDefElem(defel, pstate);
403391
opts_out->null_print = defGetString(defel);
404392
}
405393
else if (strcmp(defel->defname, "header") == 0)
406394
{
407395
if (header_specified)
408-
ereport(ERROR,
409-
(errcode(ERRCODE_SYNTAX_ERROR),
410-
errmsg("conflicting or redundant options"),
411-
parser_errposition(pstate, defel->location)));
396+
errorConflictingDefElem(defel, pstate);
412397
header_specified = true;
413398
opts_out->header_line = defGetBoolean(defel);
414399
}
415400
else if (strcmp(defel->defname, "quote") == 0)
416401
{
417402
if (opts_out->quote)
418-
ereport(ERROR,
419-
(errcode(ERRCODE_SYNTAX_ERROR),
420-
errmsg("conflicting or redundant options"),
421-
parser_errposition(pstate, defel->location)));
403+
errorConflictingDefElem(defel, pstate);
422404
opts_out->quote = defGetString(defel);
423405
}
424406
else if (strcmp(defel->defname, "escape") == 0)
425407
{
426408
if (opts_out->escape)
427-
ereport(ERROR,
428-
(errcode(ERRCODE_SYNTAX_ERROR),
429-
errmsg("conflicting or redundant options"),
430-
parser_errposition(pstate, defel->location)));
409+
errorConflictingDefElem(defel, pstate);
431410
opts_out->escape = defGetString(defel);
432411
}
433412
else if (strcmp(defel->defname, "force_quote") == 0)
434413
{
435414
if (opts_out->force_quote || opts_out->force_quote_all)
436-
ereport(ERROR,
437-
(errcode(ERRCODE_SYNTAX_ERROR),
438-
errmsg("conflicting or redundant options"),
439-
parser_errposition(pstate, defel->location)));
415+
errorConflictingDefElem(defel, pstate);
440416
if (defel->arg && IsA(defel->arg, A_Star))
441417
opts_out->force_quote_all = true;
442418
else if (defel->arg && IsA(defel->arg, List))
@@ -451,10 +427,7 @@ ProcessCopyOptions(ParseState *pstate,
451427
else if (strcmp(defel->defname, "force_not_null") == 0)
452428
{
453429
if (opts_out->force_notnull)
454-
ereport(ERROR,
455-
(errcode(ERRCODE_SYNTAX_ERROR),
456-
errmsg("conflicting or redundant options"),
457-
parser_errposition(pstate, defel->location)));
430+
errorConflictingDefElem(defel, pstate);
458431
if (defel->arg && IsA(defel->arg, List))
459432
opts_out->force_notnull = castNode(List, defel->arg);
460433
else
@@ -467,9 +440,7 @@ ProcessCopyOptions(ParseState *pstate,
467440
else if (strcmp(defel->defname, "force_null") == 0)
468441
{
469442
if (opts_out->force_null)
470-
ereport(ERROR,
471-
(errcode(ERRCODE_SYNTAX_ERROR),
472-
errmsg("conflicting or redundant options")));
443+
errorConflictingDefElem(defel, pstate);
473444
if (defel->arg && IsA(defel->arg, List))
474445
opts_out->force_null = castNode(List, defel->arg);
475446
else
@@ -487,10 +458,7 @@ ProcessCopyOptions(ParseState *pstate,
487458
* allowed for the column list to be NIL.
488459
*/
489460
if (opts_out->convert_selectively)
490-
ereport(ERROR,
491-
(errcode(ERRCODE_SYNTAX_ERROR),
492-
errmsg("conflicting or redundant options"),
493-
parser_errposition(pstate, defel->location)));
461+
errorConflictingDefElem(defel, pstate);
494462
opts_out->convert_selectively = true;
495463
if (defel->arg == NULL || IsA(defel->arg, List))
496464
opts_out->convert_select = castNode(List, defel->arg);
@@ -504,10 +472,7 @@ ProcessCopyOptions(ParseState *pstate,
504472
else if (strcmp(defel->defname, "encoding") == 0)
505473
{
506474
if (opts_out->file_encoding >= 0)
507-
ereport(ERROR,
508-
(errcode(ERRCODE_SYNTAX_ERROR),
509-
errmsg("conflicting or redundant options"),
510-
parser_errposition(pstate, defel->location)));
475+
errorConflictingDefElem(defel, pstate);
511476
opts_out->file_encoding = pg_char_to_encoding(defGetString(defel));
512477
if (opts_out->file_encoding < 0)
513478
ereport(ERROR,

‎src/backend/commands/dbcommands.c

+14-56
Original file line numberDiff line numberDiff line change
@@ -152,91 +152,61 @@ createdb(ParseState *pstate, const CreatedbStmt *stmt)
152152
if (strcmp(defel->defname, "tablespace") == 0)
153153
{
154154
if (dtablespacename)
155-
ereport(ERROR,
156-
(errcode(ERRCODE_SYNTAX_ERROR),
157-
errmsg("conflicting or redundant options"),
158-
parser_errposition(pstate, defel->location)));
155+
errorConflictingDefElem(defel, pstate);
159156
dtablespacename = defel;
160157
}
161158
else if (strcmp(defel->defname, "owner") == 0)
162159
{
163160
if (downer)
164-
ereport(ERROR,
165-
(errcode(ERRCODE_SYNTAX_ERROR),
166-
errmsg("conflicting or redundant options"),
167-
parser_errposition(pstate, defel->location)));
161+
errorConflictingDefElem(defel, pstate);
168162
downer = defel;
169163
}
170164
else if (strcmp(defel->defname, "template") == 0)
171165
{
172166
if (dtemplate)
173-
ereport(ERROR,
174-
(errcode(ERRCODE_SYNTAX_ERROR),
175-
errmsg("conflicting or redundant options"),
176-
parser_errposition(pstate, defel->location)));
167+
errorConflictingDefElem(defel, pstate);
177168
dtemplate = defel;
178169
}
179170
else if (strcmp(defel->defname, "encoding") == 0)
180171
{
181172
if (dencoding)
182-
ereport(ERROR,
183-
(errcode(ERRCODE_SYNTAX_ERROR),
184-
errmsg("conflicting or redundant options"),
185-
parser_errposition(pstate, defel->location)));
173+
errorConflictingDefElem(defel, pstate);
186174
dencoding = defel;
187175
}
188176
else if (strcmp(defel->defname, "locale") == 0)
189177
{
190178
if (dlocale)
191-
ereport(ERROR,
192-
(errcode(ERRCODE_SYNTAX_ERROR),
193-
errmsg("conflicting or redundant options"),
194-
parser_errposition(pstate, defel->location)));
179+
errorConflictingDefElem(defel, pstate);
195180
dlocale = defel;
196181
}
197182
else if (strcmp(defel->defname, "lc_collate") == 0)
198183
{
199184
if (dcollate)
200-
ereport(ERROR,
201-
(errcode(ERRCODE_SYNTAX_ERROR),
202-
errmsg("conflicting or redundant options"),
203-
parser_errposition(pstate, defel->location)));
185+
errorConflictingDefElem(defel, pstate);
204186
dcollate = defel;
205187
}
206188
else if (strcmp(defel->defname, "lc_ctype") == 0)
207189
{
208190
if (dctype)
209-
ereport(ERROR,
210-
(errcode(ERRCODE_SYNTAX_ERROR),
211-
errmsg("conflicting or redundant options"),
212-
parser_errposition(pstate, defel->location)));
191+
errorConflictingDefElem(defel, pstate);
213192
dctype = defel;
214193
}
215194
else if (strcmp(defel->defname, "is_template") == 0)
216195
{
217196
if (distemplate)
218-
ereport(ERROR,
219-
(errcode(ERRCODE_SYNTAX_ERROR),
220-
errmsg("conflicting or redundant options"),
221-
parser_errposition(pstate, defel->location)));
197+
errorConflictingDefElem(defel, pstate);
222198
distemplate = defel;
223199
}
224200
else if (strcmp(defel->defname, "allow_connections") == 0)
225201
{
226202
if (dallowconnections)
227-
ereport(ERROR,
228-
(errcode(ERRCODE_SYNTAX_ERROR),
229-
errmsg("conflicting or redundant options"),
230-
parser_errposition(pstate, defel->location)));
203+
errorConflictingDefElem(defel, pstate);
231204
dallowconnections = defel;
232205
}
233206
else if (strcmp(defel->defname, "connection_limit") == 0)
234207
{
235208
if (dconnlimit)
236-
ereport(ERROR,
237-
(errcode(ERRCODE_SYNTAX_ERROR),
238-
errmsg("conflicting or redundant options"),
239-
parser_errposition(pstate, defel->location)));
209+
errorConflictingDefElem(defel, pstate);
240210
dconnlimit = defel;
241211
}
242212
else if (strcmp(defel->defname, "location") == 0)
@@ -1497,37 +1467,25 @@ AlterDatabase(ParseState *pstate, AlterDatabaseStmt *stmt, bool isTopLevel)
14971467
if (strcmp(defel->defname, "is_template") == 0)
14981468
{
14991469
if (distemplate)
1500-
ereport(ERROR,
1501-
(errcode(ERRCODE_SYNTAX_ERROR),
1502-
errmsg("conflicting or redundant options"),
1503-
parser_errposition(pstate, defel->location)));
1470+
errorConflictingDefElem(defel, pstate);
15041471
distemplate = defel;
15051472
}
15061473
else if (strcmp(defel->defname, "allow_connections") == 0)
15071474
{
15081475
if (dallowconnections)
1509-
ereport(ERROR,
1510-
(errcode(ERRCODE_SYNTAX_ERROR),
1511-
errmsg("conflicting or redundant options"),
1512-
parser_errposition(pstate, defel->location)));
1476+
errorConflictingDefElem(defel, pstate);
15131477
dallowconnections = defel;
15141478
}
15151479
else if (strcmp(defel->defname, "connection_limit") == 0)
15161480
{
15171481
if (dconnlimit)
1518-
ereport(ERROR,
1519-
(errcode(ERRCODE_SYNTAX_ERROR),
1520-
errmsg("conflicting or redundant options"),
1521-
parser_errposition(pstate, defel->location)));
1482+
errorConflictingDefElem(defel, pstate);
15221483
dconnlimit = defel;
15231484
}
15241485
else if (strcmp(defel->defname, "tablespace") == 0)
15251486
{
15261487
if (dtablespace)
1527-
ereport(ERROR,
1528-
(errcode(ERRCODE_SYNTAX_ERROR),
1529-
errmsg("conflicting or redundant options"),
1530-
parser_errposition(pstate, defel->location)));
1488+
errorConflictingDefElem(defel, pstate);
15311489
dtablespace = defel;
15321490
}
15331491
else

‎src/backend/commands/define.c

+12
Original file line numberDiff line numberDiff line change
@@ -347,3 +347,15 @@ defGetStringList(DefElem *def)
347347

348348
return (List *) def->arg;
349349
}
350+
351+
/*
352+
* Raise an error about a conflicting DefElem.
353+
*/
354+
void
355+
errorConflictingDefElem(DefElem *defel, ParseState *pstate)
356+
{
357+
ereport(ERROR,
358+
errcode(ERRCODE_SYNTAX_ERROR),
359+
errmsg("conflicting or redundant options"),
360+
parser_errposition(pstate, defel->location));
361+
}

‎src/backend/commands/extension.c

+4-16
Original file line numberDiff line numberDiff line change
@@ -1731,30 +1731,21 @@ CreateExtension(ParseState *pstate, CreateExtensionStmt *stmt)
17311731
if (strcmp(defel->defname, "schema") == 0)
17321732
{
17331733
if (d_schema)
1734-
ereport(ERROR,
1735-
(errcode(ERRCODE_SYNTAX_ERROR),
1736-
errmsg("conflicting or redundant options"),
1737-
parser_errposition(pstate, defel->location)));
1734+
errorConflictingDefElem(defel, pstate);
17381735
d_schema = defel;
17391736
schemaName = defGetString(d_schema);
17401737
}
17411738
else if (strcmp(defel->defname, "new_version") == 0)
17421739
{
17431740
if (d_new_version)
1744-
ereport(ERROR,
1745-
(errcode(ERRCODE_SYNTAX_ERROR),
1746-
errmsg("conflicting or redundant options"),
1747-
parser_errposition(pstate, defel->location)));
1741+
errorConflictingDefElem(defel, pstate);
17481742
d_new_version = defel;
17491743
versionName = defGetString(d_new_version);
17501744
}
17511745
else if (strcmp(defel->defname, "cascade") == 0)
17521746
{
17531747
if (d_cascade)
1754-
ereport(ERROR,
1755-
(errcode(ERRCODE_SYNTAX_ERROR),
1756-
errmsg("conflicting or redundant options"),
1757-
parser_errposition(pstate, defel->location)));
1748+
errorConflictingDefElem(defel, pstate);
17581749
d_cascade = defel;
17591750
cascade = defGetBoolean(d_cascade);
17601751
}
@@ -3051,10 +3042,7 @@ ExecAlterExtensionStmt(ParseState *pstate, AlterExtensionStmt *stmt)
30513042
if (strcmp(defel->defname, "new_version") == 0)
30523043
{
30533044
if (d_new_version)
3054-
ereport(ERROR,
3055-
(errcode(ERRCODE_SYNTAX_ERROR),
3056-
errmsg("conflicting or redundant options"),
3057-
parser_errposition(pstate, defel->location)));
3045+
errorConflictingDefElem(defel, pstate);
30583046
d_new_version = defel;
30593047
}
30603048
else

0 commit comments

Comments
 (0)