[HACKERS] Add schema-qualified relnames in constraint error messages.
Hackers, At the moment schemas used as single-level namespaces. It's quite convenient in large databases. But error messages doesn’t contain schema names. I have done a little patch with schema-qualified relation names in error messages that produced by foreign key constraints and triggers. Patch and usage example are attached. Based on real bug hunting. Pre-reviewed by Alexander Korotkov. diff --git a/src/backend/utils/adt/ri_triggers.c b/src/backend/utils/adt/ri_triggers.c index b476500..997e959 100644 --- a/src/backend/utils/adt/ri_triggers.c +++ b/src/backend/utils/adt/ri_triggers.c @@ -343,7 +343,7 @@ RI_FKey_check(TriggerData *trigdata) ereport(ERROR, (errcode(ERRCODE_FOREIGN_KEY_VIOLATION), errmsg("insert or update on table \"%s\" violates foreign key constraint \"%s\"", - RelationGetRelationName(fk_rel), + RelationGetNamespaceQualifiedRelationName(fk_rel), NameStr(riinfo->conname)), errdetail("MATCH FULL does not allow mixing of null and nonnull key values."), errtableconstraint(fk_rel, @@ -2490,7 +2490,7 @@ RI_Initial_Check(Trigger *trigger, Relation fk_rel, Relation pk_rel) ereport(ERROR, (errcode(ERRCODE_FOREIGN_KEY_VIOLATION), errmsg("insert or update on table \"%s\" violates foreign key constraint \"%s\"", - RelationGetRelationName(fk_rel), + RelationGetNamespaceQualifiedRelationName(fk_rel), NameStr(fake_riinfo.conname)), errdetail("MATCH FULL does not allow mixing of null and nonnull key values."), errtableconstraint(fk_rel, @@ -2767,7 +2767,7 @@ ri_FetchConstraintInfo(Trigger *trigger, Relation trig_rel, bool rel_is_pk) ereport(ERROR, (errcode(ERRCODE_INVALID_OBJECT_DEFINITION), errmsg("no pg_constraint entry for trigger \"%s\" on table \"%s\"", -trigger->tgname, RelationGetRelationName(trig_rel)), +trigger->tgname, RelationGetNamespaceQualifiedRelationName(trig_rel)), errhint("Remove this referential integrity trigger and its mates, then do ALTER TABLE ADD CONSTRAINT."))); /* Find or create a hashtable entry for the constraint */ @@ -2779,14 +2779,14 @@ ri_FetchConstraintInfo(Trigger *trigger, Relation trig_rel, bool rel_is_pk) if (riinfo->fk_relid != trigger->tgconstrrelid || riinfo->pk_relid != RelationGetRelid(trig_rel)) elog(ERROR, "wrong pg_constraint entry for trigger \"%s\" on table \"%s\"", -trigger->tgname, RelationGetRelationName(trig_rel)); +trigger->tgname, RelationGetNamespaceQualifiedRelationName(trig_rel)); } else { if (riinfo->fk_relid != RelationGetRelid(trig_rel) || riinfo->pk_relid != trigger->tgconstrrelid) elog(ERROR, "wrong pg_constraint entry for trigger \"%s\" on table \"%s\"", -trigger->tgname, RelationGetRelationName(trig_rel)); +trigger->tgname, RelationGetNamespaceQualifiedRelationName(trig_rel)); } return riinfo; @@ -3225,9 +3225,9 @@ ri_ReportViolation(const RI_ConstraintInfo *riinfo, ereport(ERROR, (errcode(ERRCODE_INTERNAL_ERROR), errmsg("referential integrity query on \"%s\" from constraint \"%s\" on \"%s\" gave unexpected result", - RelationGetRelationName(pk_rel), + RelationGetNamespaceQualifiedRelationName(pk_rel), NameStr(riinfo->conname), - RelationGetRelationName(fk_rel)), + RelationGetNamespaceQualifiedRelationName(fk_rel)), errhint("This is most likely due to a rule having rewritten the query."))); /* @@ -3315,28 +3315,28 @@ ri_ReportViolation(const RI_ConstraintInfo *riinfo,
Re: [HACKERS] Beginner hacker item: Fix to_reg*() input type
2016-01-05 21:04 GMT+03:00 Tom Lane : > (I did make some small adjustments --- in this usage, PG_GETARG_TEXT_PP > is good enough and likely a bit more efficient than PG_GETARG_TEXT_P.) > Also I forgot to bump catalog version up. My mishit.
Re: [HACKERS] Beginner hacker item: Fix to_reg*() input type
> - Modify the functions in regproc.c. Take a look at how other text input > functions work to see what needs to happen here (you'll want to use > text_to_cstring() as part of that.) > > - Modify the appropriate entries in src/include/catalog/pg_proc.h Let me try. `make check` says "All 160 tests passed.". diff --git a/src/backend/utils/adt/regproc.c b/src/backend/utils/adt/regproc.c index 0f17eb8..eed1279 100644 --- a/src/backend/utils/adt/regproc.c +++ b/src/backend/utils/adt/regproc.c @@ -161,7 +161,7 @@ regprocin(PG_FUNCTION_ARGS) Datum to_regproc(PG_FUNCTION_ARGS) { - char *pro_name = PG_GETARG_CSTRING(0); + const char *pro_name = text_to_cstring(PG_GETARG_TEXT_P(0)); List *names; FuncCandidateList clist; @@ -331,7 +331,7 @@ regprocedurein(PG_FUNCTION_ARGS) Datum to_regprocedure(PG_FUNCTION_ARGS) { - char *pro_name = PG_GETARG_CSTRING(0); + const char *pro_name = text_to_cstring(PG_GETARG_TEXT_P(0)); List *names; int nargs; Oid argtypes[FUNC_MAX_ARGS]; @@ -620,7 +620,7 @@ regoperin(PG_FUNCTION_ARGS) Datum to_regoper(PG_FUNCTION_ARGS) { - char *opr_name = PG_GETARG_CSTRING(0); + const char *opr_name = text_to_cstring(PG_GETARG_TEXT_P(0)); List *names; FuncCandidateList clist; @@ -797,7 +797,7 @@ regoperatorin(PG_FUNCTION_ARGS) Datum to_regoperator(PG_FUNCTION_ARGS) { - char *opr_name_or_oid = PG_GETARG_CSTRING(0); + const char *opr_name_or_oid = text_to_cstring(PG_GETARG_TEXT_P(0)); Oid result; List *names; int nargs; @@ -1061,7 +1061,7 @@ regclassin(PG_FUNCTION_ARGS) Datum to_regclass(PG_FUNCTION_ARGS) { - char *class_name = PG_GETARG_CSTRING(0); + const char *class_name = text_to_cstring(PG_GETARG_TEXT_P(0)); Oid result; List *names; @@ -1249,7 +1249,7 @@ regtypein(PG_FUNCTION_ARGS) Datum to_regtype(PG_FUNCTION_ARGS) { - char *typ_name = PG_GETARG_CSTRING(0); + const char *typ_name = text_to_cstring(PG_GETARG_TEXT_P(0)); Oid result; int32 typmod; @@ -1606,7 +1606,7 @@ regrolein(PG_FUNCTION_ARGS) Datum to_regrole(PG_FUNCTION_ARGS) { - char *role_name = PG_GETARG_CSTRING(0); + const char *role_name = text_to_cstring(PG_GETARG_TEXT_P(0)); Oid result; List *names; @@ -1727,7 +1727,7 @@ regnamespacein(PG_FUNCTION_ARGS) Datum to_regnamespace(PG_FUNCTION_ARGS) { - char *nsp_name = PG_GETARG_CSTRING(0); + const char *nsp_name = text_to_cstring(PG_GETARG_TEXT_P(0)); Oid result; List *names; diff --git a/src/include/catalog/pg_proc.h b/src/include/catalog/pg_proc.h index e5d6c77..22d9386 100644 --- a/src/include/catalog/pg_proc.h +++ b/src/include/catalog/pg_proc.h @@ -177,9 +177,9 @@ DATA(insert OID = 44 ( regprocin PGNSP PGUID 12 1 0 0 0 f f f f t f s s 1 DESCR("I/O"); DATA(insert OID = 45 ( regprocout PGNSP PGUID 12 1 0 0 0 f f f f t f s s 1 0 2275 "24" _null_ _null_ _null_ _null_ _null_ regprocout _null_ _null_ _null_ )); DESCR("I/O"); -DATA(insert OID = 3494 ( to_regproc PGNSP PGUID 12 1 0 0 0 f f f f t f s s 1 0 24 "2275" _null_ _null_ _null_ _null_ _null_ to_regproc _null_ _null_ _null_ )); +DATA(insert OID = 3494 ( to_regproc PGNSP PGUID 12 1 0 0 0 f f f f t f s s 1 0 24 "25" _null_ _null_ _null_ _null_ _null_ to_regproc _null_ _null_ _null_ )); DESCR("convert proname to regproc"); -DATA(insert OID = 3479 ( to_regprocedure PGNSP PGUID 12 1 0 0 0 f f f f t f s s 1 0 2202 "2275" _null_ _null_ _null_ _null_ _null_ to_regprocedure _null_ _null_ _null_ )); +DATA(insert OID = 3479 ( to_regprocedure PGNSP PGUID 12 1 0 0 0 f f f f t f s s 1 0 2202 "25" _null_ _null_ _null_ _null_ _null_ to_regprocedure _null_ _null_ _null_ )); DESCR("convert proname to regprocedure"); DATA(insert OID = 46 ( textin PGNSP PGUID 12 1 0 0 0 f f f f t f i s 1 0 25 "2275" _null_ _null_ _null_ _null_ _null_ textin _null_ _null_ _null_ )); DESCR("I/O"); @@ -3483,9 +3483,9 @@ DATA(insert OID = 2214 ( regoperin PGNSP PGUID 12 1 0 0 0 f f f f t f s s 1 0 DESCR("I/O"); DATA(insert OID = 2215 ( regoperout PGNSP PGUID 12 1 0 0 0 f f f f t f s s 1 0 2275 "2203" _null_ _null_ _null_ _null_ _null_ regoperout _null_ _null_ _null_ )); DESCR("I/O"); -DATA(insert OID = 3492 ( to_regoper PGNSP PGUID 12 1 0 0 0 f f f f t f s s 1 0 2203 "2275" _null_ _null_ _null_ _null_ _null_ to_regoper _null_ _null_ _null_ )); +DATA(insert OID = 3492 ( to_regoper PGNSP PGUID 12 1 0 0 0 f f f f t f s
Re: [HACKERS] Comfortably check BackendPID with psql
+1 for Julien's patch.
Re: [HACKERS] psql :: support for \ev viewname and \sv viewname
пт, 3 июля 2015 г. в 19:30, Tom Lane : > So AFAICS, \ev and \sv should just number lines straightforwardly, with > "1" being the first line of the CREATE command text. Am I missing > something? > Fixed. Now both \ev and \sv numbering lines starting with "1". New version attached. As I've already noticed that pg_get_viewdef() does not support full syntax of creating or replacing views. In my opinion, psql source code isn't the place where some formatting hacks should be. So, can you give me an idea how to produce already formatted output supporting "WITH" statement without breaking backward compatibility of pg_get_viewdef() internals? psql-ev-sv-support-v6.diff Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] psql :: support for \ev viewname and \sv viewname
> 1. > make failed with docs > Fixed. > 2. > > \ev vw1 3 > > This syntax is supported. But documentation only says: > \ev [ viewname ] > Missing optional line_number clause > Fixed. Documented. 3. > > strip_lineno_from_objdesc(char *func) > > Can we have parameter name as obj instead of func. > You have renamed the function name, as it is now called in case of views as > well. Better rename the parameter names as well. Renamed. 4. > Also please update the comments above strip_lineno_from_objdesc(). > It is specific to functions which is not the case now. > Comments updated. > 5. > > print_with_linenumbers(FILE *output, > > char *lines, > > const char *header_cmp_keyword, > > size_t header_cmp_sz) > > Can't we calculate the length of header (header_cmp_sz) inside function? > This will avoid any sloppy changes like, change in the keyword but forgot > to > change the size. > Lets just accept the keyword and calculate the size within the function. > Now header_cmp_sz calculated inside function. 6. > >* > >* Note that this loop scribbles on > func_buf. > >*/ > > These lines at commands.c:1357, looks NO more valid now as there is NO loop > there. > Removed. > > 7. > I see few comment lines explaining which is line 1 in case of function, for > which "AS " is used. Similarly, for view "SELECT " is used. > Can you add similar kind of explanation there? > Explanation added. 8. > > get_create_object_cmd_internal > > get_create_function_cmd > > get_create_view_cmd > > Can these three functions grouped together in just get_create_object_cmd(). > This function will take an extra parameter to indicate the object type. > Say O_FUNC and O_VIEW for example. > > For distinct part, just have a switch case over this type. > > This will add a flexibility that if we add another such \e and \s options, > we > don't need new functions, rather just need new enum like O_new and a new > case > in this switch statement. > Also it will look good to read the code as well. > > similarly you can do it for > > lookup_object_oid_internal > > get_create_function_cmd > > lookup_function_oid > Reworked. New enum PgObjType introduced. > > 9. > > static int count_lines_in_buf(PQExpBuffer buf) > > static void print_with_linenumbers(FILE *output, .. ) > > static bool lookup_view_oid(const char *desc, Oid *view_oid) > > static bool lookup_object_oid_internal(PQExpBuffer query, Oid *obj_oid) > > Can we have smaller description, explaining what's the function doing for > these functions at the definition? > Description added. > > 10. > > + "\\e", "\\echo", "\\ef", "\\ev", "\\encoding", > > Can you keep this sorted? > It will be good if it sorted, but I see no such restriction as I see few > out > of order options. But better keep it ordered. > Ignore if you dis-agree. > Hmm, sorted now. Sort is based on my feelings. psql-ev-sv-support-v4.diff Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] psql :: support for \ev viewname and \sv viewname
Just a merge after pgindent run (807b9e0dff663c5da875af7907a5106c0ff90673). psql-ev-sv-support-v3.diff Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] psql :: support for \ev viewname and \sv viewname
Hackers! I'm proposing to add two new subcommands in psql: 1. \ev viewname - edit view definition with external editor (like \ef for function) 2. \sv viewname - show view definition (like \sf for function, for consistency) What's inside: 1. review-ready implementation of \ev and \sv psql subcommands for editing and viewing view's definition. 2. psql's doc update with new subcommands description. 3. a bit of extracting common source code parts into separate functions. 4. psql internal help update. 5. tab completion update. There is one narrow place in this patch: current implementation doesn't support "create" statement formatting for recursive views. Any comments? Suggestions? diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml index 62a3b21..d668777 100644 --- a/doc/src/sgml/ref/psql-ref.sgml +++ b/doc/src/sgml/ref/psql-ref.sgml @@ -1651,6 +1651,28 @@ Tue Oct 26 21:40:57 CEST 1999 +\ev viewname + + + + This command fetches and edits the definition of the named view, + in the form of a CREATE OR REPLACE VIEW command. + Editing is done in the same way as for \edit. + After the editor exits, the updated command waits in the query buffer; + type semicolon or \g to send it, or \r + to cancel. + + + + If no view is specified, a blank CREATE VEIW + template is presented for editing. + + + + + + + \encoding [ encoding ] @@ -2522,6 +2544,25 @@ testdb=> \setenv LESS -imx4F +\sv[+] viewname + + + + This command fetches and shows the definition of the named view, + in the form of a CREATE OR REPLACE VIEW command. + The definition is printed to the current query output channel, + as set by \o. + + + + If + is appended to the command name, then the + output lines are numbered, with the first line of the view definition + being line 1. + + + + + \t diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c index 70b7d3b..948e381 100644 --- a/src/bin/psql/command.c +++ b/src/bin/psql/command.c @@ -60,9 +60,17 @@ static bool do_connect(char *dbname, char *user, char *host, char *port); static bool do_shell(const char *command); static bool do_watch(PQExpBuffer query_buf, long sleep); static bool lookup_function_oid(const char *desc, Oid *foid); +static bool lookup_view_oid(const char *desc, Oid *view_oid); static bool get_create_function_cmd(Oid oid, PQExpBuffer buf); -static int strip_lineno_from_funcdesc(char *func); +static bool get_create_view_cmd(Oid oid, PQExpBuffer buf); +static void format_create_view_cmd(char *view, PQExpBuffer buf); +static int strip_lineno_from_objdesc(char *func); static void minimal_error_message(PGresult *res); +static int count_lines_in_buf(PQExpBuffer buf); +static void print_with_linenumbers(FILE *output, + char *lines, + const char *header_cmp_keyword, + size_t header_cmp_sz); static void printSSLInfo(void); static bool printPsetInfo(const char *param, struct printQueryOpt *popt); @@ -612,7 +620,7 @@ exec_command(const char *cmd, func = psql_scan_slash_option(scan_state, OT_WHOLE_LINE, NULL, true); - lineno = strip_lineno_from_funcdesc(func); + lineno = strip_lineno_from_objdesc(func); if (lineno == 0) { /* error already reported */ @@ -682,6 +690,77 @@ exec_command(const char *cmd, } } + /* +* \ev -- edit the named view, or present a blank CREATE VIEW viewname AS +* template if no argument is given +*/ + else if (strcmp(cmd, "ev") == 0) + { + int lineno = -1; + + if (pset.sversion < 70100) + { + psql_error("The server (version %d.%d) does not support editing view definition.\n", + pset.sversion / 1, (pset.sversion / 100) % 100); + status = PSQL_CMD_ERROR; + } + else if (!query_buf) + { + psql_error("no query buffer\n"); + status = PSQL_CMD_ERROR; + } + else + { + char *view; + Oid view_oid = InvalidOid; + + view = psql_scan_slash_option(scan_state, +