Thread: INFORMATION_SCHEMA.routines column routine_definition does not show the source

"David G. Johnston" <david.g.johnston@gmail.com> writes:
> The information schema query consults pg_proc.prosrc directly instead of
> calling pg_get_functiondef(...) (which didn't exist when the original query
> was written, and hasn't been wrong until now).

pg_get_functiondef would produce more than we want, but it looks like
pg_get_function_sqlbody() would do.

BTW, while researching this I noted the header comment for
pg_get_functiondef:

 * Note: if you change the output format of this function, be careful not
 * to break psql's rules (in \ef and \sf) for identifying the start of the
 * function body.  To wit: the function body starts on a line that begins
 * with "AS ", and no preceding line will look like that.

Needless to say, the SQL-function-body patch has ignored this advice
totally.  At the very least this comment needs to be adjusted, but
I wonder if it's not telling us that \ef and/or \sf are broken.

            regards, tom lane

"David G. Johnston" <david.g.johnston@gmail.com> writes:
> Decided to ping this as I don't recall or see it getting patched and the
> recent discussion regarding the prosrc field.

>> * Note: if you change the output format of this function, be careful not
>> * to break psql's rules (in \ef and \sf) for identifying the start of the
>> * function body.  To wit: the function body starts on a line that begins
>> * with "AS ", and no preceding line will look like that.
>> 
>> Needless to say, the SQL-function-body patch has ignored this advice
>> totally.  At the very least this comment needs to be adjusted, but
>> I wonder if it's not telling us that \ef and/or \sf are broken.

Indeed, if you experiment with "\sf+" or "\ef" with a line number,
those features fail miserably for new-style SQL functions.

We could partially fix that by teaching psql to also recognize
"BEGIN" as the start of the function body.  I say "partially"
because the whole point of that line-numbers feature is so that
you can figure out which line an error report is complaining about.
However, SQL functions don't report in terms of line numbers,
and never have, so maybe that's moot.  It's fortunate that they
don't, because the reconstructed function text is likely to have
different line breaks than the original.

            regards, tom lane

I wrote:
> We could partially fix that by teaching psql to also recognize
> "BEGIN" as the start of the function body.

Looks like we need "RETURN " too, so more or less as attached.

(I didn't bother with a test case yet.  \sf wouldn't be too
hard to exercise, but I wonder how we could test \ef portably.)

            regards, tom lane

diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index a20a1b069b..641df1cabe 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -2867,8 +2867,8 @@ pg_get_serial_sequence(PG_FUNCTION_ARGS)
  *
  * Note: if you change the output format of this function, be careful not
  * to break psql's rules (in \ef and \sf) for identifying the start of the
- * function body.  To wit: the function body starts on a line that begins
- * with "AS ", and no preceding line will look like that.
+ * function body.  To wit: the function body starts on a line that begins with
+ * "AS ", "BEGIN ", or "RETURN ", and no preceding line will look like that.
  */
 Datum
 pg_get_functiondef(PG_FUNCTION_ARGS)
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 7672ed9e9d..de6a3a71f8 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -168,8 +168,7 @@ static bool get_create_object_cmd(EditableObjectType obj_type, Oid oid,
                                   PQExpBuffer buf);
 static int    strip_lineno_from_objdesc(char *obj);
 static int    count_lines_in_buf(PQExpBuffer buf);
-static void print_with_linenumbers(FILE *output, char *lines,
-                                   const char *header_keyword);
+static void print_with_linenumbers(FILE *output, char *lines, bool is_func);
 static void minimal_error_message(PGresult *res);

 static void printSSLInfo(void);
@@ -1201,17 +1200,19 @@ exec_command_ef_ev(PsqlScanState scan_state, bool active_branch,
                 /*
                  * lineno "1" should correspond to the first line of the
                  * function body.  We expect that pg_get_functiondef() will
-                 * emit that on a line beginning with "AS ", and that there
-                 * can be no such line before the real start of the function
-                 * body.  Increment lineno by the number of lines before that
-                 * line, so that it becomes relative to the first line of the
-                 * function definition.
+                 * emit that on a line beginning with "AS ", "BEGIN ", or
+                 * "RETURN ", and that there can be no such line before the
+                 * real start of the function body.  Increment lineno by the
+                 * number of lines before that line, so that it becomes
+                 * relative to the first line of the function definition.
                  */
                 const char *lines = query_buf->data;

                 while (*lines != '\0')
                 {
-                    if (strncmp(lines, "AS ", 3) == 0)
+                    if (strncmp(lines, "AS ", 3) == 0 ||
+                        strncmp(lines, "BEGIN ", 6) == 0 ||
+                        strncmp(lines, "RETURN ", 7) == 0)
                         break;
                     lineno++;
                     /* find start of next line */
@@ -2528,15 +2529,8 @@ exec_command_sf_sv(PsqlScanState scan_state, bool active_branch,

             if (show_linenumbers)
             {
-                /*
-                 * For functions, lineno "1" should correspond to the first
-                 * line of the function body.  We expect that
-                 * pg_get_functiondef() will emit that on a line beginning
-                 * with "AS ", and that there can be no such line before the
-                 * real start of the function body.
-                 */
-                print_with_linenumbers(output, buf->data,
-                                       is_func ? "AS " : NULL);
+                /* add line numbers */
+                print_with_linenumbers(output, buf->data, is_func);
             }
             else
             {
@@ -5611,24 +5605,28 @@ count_lines_in_buf(PQExpBuffer buf)
 /*
  * Write text at *lines to output with line numbers.
  *
- * If header_keyword isn't NULL, then line 1 should be the first line beginning
- * with header_keyword; lines before that are unnumbered.
+ * For functions, lineno "1" should correspond to the first line of the
+ * function body; lines before that are unnumbered.  We expect that
+ * pg_get_functiondef() will emit that on a line beginning with "AS ",
+ * "BEGIN ", or "RETURN ", and that there can be no such line before
+ * the real start of the function body.
  *
  * Caution: this scribbles on *lines.
  */
 static void
-print_with_linenumbers(FILE *output, char *lines,
-                       const char *header_keyword)
+print_with_linenumbers(FILE *output, char *lines, bool is_func)
 {
-    bool        in_header = (header_keyword != NULL);
-    size_t        header_sz = in_header ? strlen(header_keyword) : 0;
+    bool        in_header = is_func;
     int            lineno = 0;

     while (*lines != '\0')
     {
         char       *eol;

-        if (in_header && strncmp(lines, header_keyword, header_sz) == 0)
+        if (in_header &&
+            (strncmp(lines, "AS ", 3) == 0 ||
+             strncmp(lines, "BEGIN ", 6) == 0 ||
+             strncmp(lines, "RETURN ", 7) == 0))
             in_header = false;

         /* increment lineno only for body's lines */

I wrote:
> We could partially fix that by teaching psql to also recognize
> "BEGIN" as the start of the function body.

Looks like we need "RETURN " too, so more or less as attached.

(I didn't bother with a test case yet.  \sf wouldn't be too
hard to exercise, but I wonder how we could test \ef portably.)                

"David G. Johnston" <david.g.johnston@gmail.com> writes:
> Ping again as a Reddit poster just pointed this out.

Huh?  The patch you're replying to was committed two years ago.

            regards, tom lane

"David G. Johnston" <david.g.johnston@gmail.com> writes:
> Ping again as a Reddit poster just pointed this out.

Huh?  The patch you're replying to was committed two years ago.