Attached is a proposed patch that fixes the
cursor-position-in-CREATE-FUNCTION issue per my earlier suggestion.

Since I complained that your proposal was too grotty, it's only fair to
throw this out to let people take potshots at it ;-).  The main
grottiness here is providing access to the executing Portal so that the
callback function can get at the original command string.  I don't think
this is unreasonably bad, since we already have a global PortalContext
that points to the portal's memory context --- I just added parallel
code that updates a new global ActivePortal.

The re-parsing of the original command is simplistic but will handle all
normal cases.

                        regards, tom lane


*** src/backend/catalog/pg_proc.c.orig  Sat Mar 13 20:58:41 2004
--- src/backend/catalog/pg_proc.c       Thu Mar 18 18:20:20 2004
***************
*** 23,31 ****
--- 23,33 ----
  #include "executor/executor.h"
  #include "fmgr.h"
  #include "miscadmin.h"
+ #include "mb/pg_wchar.h"
  #include "parser/parse_coerce.h"
  #include "parser/parse_expr.h"
  #include "parser/parse_type.h"
+ #include "tcop/pquery.h"
  #include "tcop/tcopprot.h"
  #include "utils/acl.h"
  #include "utils/builtins.h"
***************
*** 45,50 ****
--- 47,56 ----
  static Datum create_parameternames_array(int parameterCount,
                                                                                 const 
char *parameterNames[]);
  static void sql_function_parse_error_callback(void *arg);
+ static int    match_prosrc_to_query(const char *prosrc, const char *queryText,
+                                                                 int cursorpos);
+ static bool match_prosrc_to_literal(const char *prosrc, const char *literal,
+                                                                       int cursorpos, 
int *newcursorpos);
  
  
  /* ----------------------------------------------------------------
***************
*** 768,774 ****
                 * location is inside the function body, not out in CREATE FUNCTION.
                 */
                sqlerrcontext.callback = sql_function_parse_error_callback;
!               sqlerrcontext.arg = proc;
                sqlerrcontext.previous = error_context_stack;
                error_context_stack = &sqlerrcontext;
  
--- 774,780 ----
                 * location is inside the function body, not out in CREATE FUNCTION.
                 */
                sqlerrcontext.callback = sql_function_parse_error_callback;
!               sqlerrcontext.arg = tuple;
                sqlerrcontext.previous = error_context_stack;
                error_context_stack = &sqlerrcontext;
  
***************
*** 800,821 ****
  }
  
  /*
!  * error context callback to let us supply a context marker
   */
  static void
  sql_function_parse_error_callback(void *arg)
  {
!       Form_pg_proc proc = (Form_pg_proc) arg;
  
        /*
!        * XXX it'd be really nice to adjust the syntax error position to
!        * account for the offset from the start of the statement to the
!        * function body string, not to mention any quoting characters in
!        * the string, but I can't see any decent way to do that...
         *
!        * In the meantime, put in a CONTEXT entry that can cue clients
!        * not to trust the syntax error position completely.
         */
!       errcontext("SQL function \"%s\"",
!                          NameStr(proc->proname));
  }
--- 806,976 ----
  }
  
  /*
!  * error context callback to let us adjust syntax errors from SQL functions
   */
  static void
  sql_function_parse_error_callback(void *arg)
  {
!       HeapTuple       tuple = (HeapTuple) arg;
!       Form_pg_proc proc = (Form_pg_proc) GETSTRUCT(tuple);
!       int                     origerrposition;
!       const char *queryText;
!       bool            isnull;
!       Datum           tmp;
!       char       *prosrc;
!       int                     newerrposition;
! 
!       /*
!        * Nothing to do unless we are dealing with a syntax error that has
!        * a cursor position.  In that case, we need to try to adjust the
!        * position to match the original query, not the text of the function.
!        */
!       origerrposition = geterrposition();
!       if (origerrposition <= 0)
!               return;
! 
!       /*
!        * We can get the original query text from the active portal (hack...)
!        */
!       Assert(ActivePortal && ActivePortal->portalActive);
!       queryText = ActivePortal->sourceText;
! 
!       /*
!        * Try to locate the function text in the original query.
!        */
!       tmp = SysCacheGetAttr(PROCOID, tuple, Anum_pg_proc_prosrc, &isnull);
!       if (isnull)
!               elog(ERROR, "null prosrc");
!       prosrc = DatumGetCString(DirectFunctionCall1(textout, tmp));
! 
!       newerrposition = match_prosrc_to_query(prosrc, queryText, origerrposition);
! 
!       if (newerrposition > 0)
!       {
!               /* Successful, so fix the error position */
!               errposition(newerrposition);
!       }
!       else
!       {
!               /*
!                * If unsuccessful, put in a CONTEXT entry that will cue clients
!                * not to treat the error position as relative to the original query.
!                */
!               errcontext("SQL function \"%s\"",
!                                  NameStr(proc->proname));
!       }
! }
! 
! /*
!  * Try to locate the string literal containing the function body in the
!  * given text of the CREATE FUNCTION command.  If successful, return the
!  * character (not byte) index within the command corresponding to the
!  * given character index within the literal.  If not successful, return 0.
!  */
! static int
! match_prosrc_to_query(const char *prosrc, const char *queryText,
!                                         int cursorpos)
! {
!       /*
!        * Rather than fully parsing the CREATE FUNCTION command, we just scan
!        * the command looking for $prosrc$ or 'prosrc'.  This could be fooled
!        * (though not in any very probable scenarios), so fail if we find
!        * more than one match.
!        */
!       int             prosrclen = strlen(prosrc);
!       int             querylen = strlen(queryText);
!       int             matchpos = 0;
!       int             curpos;
!       int             newcursorpos;
! 
!       for (curpos = 0; curpos < querylen-prosrclen; curpos++)
!       {
!               if (queryText[curpos] == '$' &&
!                       strncmp(prosrc, &queryText[curpos+1], prosrclen) == 0 &&
!                       queryText[curpos+1+prosrclen] == '$')
!               {
!                       /*
!                        * Found a $foo$ match.  Since there are no embedded quoting
!                        * characters in a dollar-quoted literal, we don't have to do
!                        * any fancy arithmetic; just offset by the starting position.
!                        */
!                       if (matchpos)
!                               return 0;               /* multiple matches, fail */
!                       matchpos = pg_mbstrlen_with_len(queryText, curpos+1)
!                               + cursorpos;
!               }
!               else if (queryText[curpos] == '\'' &&
!                                match_prosrc_to_literal(prosrc, &queryText[curpos+1],
!                                                                                
cursorpos, &newcursorpos))
!               {
!                       /*
!                        * Found a 'foo' match.  match_prosrc_to_literal() has adjusted
!                        * for any quotes or backslashes embedded in the literal.
!                        */
!                       if (matchpos)
!                               return 0;               /* multiple matches, fail */
!                       matchpos = pg_mbstrlen_with_len(queryText, curpos+1)
!                               + newcursorpos;
!               }
!       }
! 
!       return matchpos;
! }
! 
! /*
!  * Try to match the given source text to a single-quoted literal.
!  * If successful, adjust newcursorpos to correspond to the character
!  * (not byte) index corresponding to cursorpos in the source text.
!  *
!  * At entry, literal points just past a ' character.  We must check for the
!  * trailing quote.
!  */
! static bool
! match_prosrc_to_literal(const char *prosrc, const char *literal,
!                                               int cursorpos, int *newcursorpos)
! {
!       int                     newcp = cursorpos;
!       int                     chlen;
  
        /*
!        * This implementation handles backslashes and doubled quotes in the
!        * string literal.  It does not handle the SQL syntax for literals
!        * continued across line boundaries.
         *
!        * We do the comparison a character at a time, not a byte at a time,
!        * so that we can do the correct cursorpos math.
         */
!       while (*prosrc)
!       {
!               cursorpos--;                    /* characters left before cursor */
!               /*
!                * Check for backslashes and doubled quotes in the literal; adjust
!                * newcp when one is found before the cursor.
!                */
!               if (*literal == '\\')
!               {
!                       literal++;
!                       if (cursorpos > 0)
!                               newcp++;
!               }
!               else if (*literal == '\'')
!               {
!                       if (literal[1] != '\'')
!                               return false;
!                       literal++;
!                       if (cursorpos > 0)
!                               newcp++;
!               }
!               chlen = pg_mblen(prosrc);
!               if (strncmp(prosrc, literal, chlen) != 0)
!                       return false;
!               prosrc += chlen;
!               literal += chlen;
!       }
! 
!       *newcursorpos = newcp;
! 
!       if (*literal == '\'' && literal[1] != '\'')
!               return true;
!       return false;
  }
*** src/backend/commands/portalcmds.c.orig      Sat Nov 29 14:51:47 2003
--- src/backend/commands/portalcmds.c   Thu Mar 18 16:57:10 2004
***************
*** 270,275 ****
--- 270,276 ----
  PersistHoldablePortal(Portal portal)
  {
        QueryDesc  *queryDesc = PortalGetQueryDesc(portal);
+       Portal          saveActivePortal;
        MemoryContext savePortalContext;
        MemoryContext saveQueryContext;
        MemoryContext oldcxt;
***************
*** 311,316 ****
--- 312,319 ----
        /*
         * Set global portal context pointers.
         */
+       saveActivePortal = ActivePortal;
+       ActivePortal = portal;
        savePortalContext = PortalContext;
        PortalContext = PortalGetHeapMemory(portal);
        saveQueryContext = QueryContext;
***************
*** 342,347 ****
--- 345,351 ----
        /* Mark portal not active */
        portal->portalActive = false;
  
+       ActivePortal = saveActivePortal;
        PortalContext = savePortalContext;
        QueryContext = saveQueryContext;
  
*** src/backend/tcop/postgres.c.orig    Mon Mar 15 15:01:58 2004
--- src/backend/tcop/postgres.c Thu Mar 18 16:56:55 2004
***************
*** 2710,2715 ****
--- 2710,2716 ----
                 */
                MemoryContextSwitchTo(TopMemoryContext);
                MemoryContextResetAndDeleteChildren(ErrorContext);
+               ActivePortal = NULL;
                PortalContext = NULL;
                QueryContext = NULL;
  
*** src/backend/tcop/pquery.c.orig      Fri Mar  5 15:57:31 2004
--- src/backend/tcop/pquery.c   Thu Mar 18 16:56:56 2004
***************
*** 23,28 ****
--- 23,35 ----
  #include "utils/memutils.h"
  
  
+ /*
+  * ActivePortal is the currently executing Portal (the most closely nested,
+  * if there are several).
+  */
+ Portal        ActivePortal = NULL;
+ 
+ 
  static uint32 RunFromStore(Portal portal, ScanDirection direction, long count,
                         DestReceiver *dest);
  static long PortalRunSelect(Portal portal, bool forward, long count,
***************
*** 395,400 ****
--- 402,408 ----
                  char *completionTag)
  {
        bool            result;
+       Portal          saveActivePortal;
        MemoryContext savePortalContext;
        MemoryContext saveQueryContext;
        MemoryContext oldContext;
***************
*** 433,438 ****
--- 441,448 ----
        /*
         * Set global portal context pointers.
         */
+       saveActivePortal = ActivePortal;
+       ActivePortal = portal;
        savePortalContext = PortalContext;
        PortalContext = PortalGetHeapMemory(portal);
        saveQueryContext = QueryContext;
***************
*** 508,513 ****
--- 518,524 ----
        /* Mark portal not active */
        portal->portalActive = false;
  
+       ActivePortal = saveActivePortal;
        PortalContext = savePortalContext;
        QueryContext = saveQueryContext;
  
***************
*** 925,930 ****
--- 936,942 ----
                           DestReceiver *dest)
  {
        long            result;
+       Portal          saveActivePortal;
        MemoryContext savePortalContext;
        MemoryContext saveQueryContext;
        MemoryContext oldContext;
***************
*** 948,953 ****
--- 960,967 ----
        /*
         * Set global portal context pointers.
         */
+       saveActivePortal = ActivePortal;
+       ActivePortal = portal;
        savePortalContext = PortalContext;
        PortalContext = PortalGetHeapMemory(portal);
        saveQueryContext = QueryContext;
***************
*** 972,977 ****
--- 986,992 ----
        /* Mark portal not active */
        portal->portalActive = false;
  
+       ActivePortal = saveActivePortal;
        PortalContext = savePortalContext;
        QueryContext = saveQueryContext;
  
*** src/backend/utils/error/elog.c.orig Mon Mar 15 15:01:58 2004
--- src/backend/utils/error/elog.c      Thu Mar 18 17:36:58 2004
***************
*** 808,813 ****
--- 808,829 ----
        return 0;                                       /* return value does not 
matter */
  }
  
+ /*
+  * geterrposition --- return the currently set error position (0 if none)
+  *
+  * This is only intended for use in error callback subroutines, since there
+  * is no other place outside elog.c where the concept is meaningful.
+  */
+ int
+ geterrposition(void)
+ {
+       ErrorData  *edata = &errordata[errordata_stack_depth];
+ 
+       /* we don't bother incrementing recursion_depth */
+       CHECK_STACK_DEPTH();
+ 
+       return edata->cursorpos;
+ }
  
  /*
   * elog_finish --- finish up for old-style API
*** src/include/tcop/pquery.h.orig      Sat Nov 29 17:41:14 2003
--- src/include/tcop/pquery.h   Thu Mar 18 16:56:49 2004
***************
*** 17,22 ****
--- 17,25 ----
  #include "utils/portal.h"
  
  
+ extern DLLIMPORT Portal ActivePortal;
+ 
+ 
  extern void ProcessQuery(Query *parsetree,
                         Plan *plan,
                         ParamListInfo params,
*** src/include/utils/elog.h.orig       Mon Mar 15 15:02:09 2004
--- src/include/utils/elog.h    Thu Mar 18 17:36:51 2004
***************
*** 132,137 ****
--- 132,139 ----
  extern int    errfunction(const char *funcname);
  extern int    errposition(int cursorpos);
  
+ extern int    geterrposition(void);
+ 
  
  /*----------
   * Old-style error reporting API: to be used in this way:

---------------------------(end of broadcast)---------------------------
TIP 5: Have you checked our extensive FAQ?

               http://www.postgresql.org/docs/faqs/FAQ.html

Reply via email to