Re: ScanDirections - Mailing list pgsql-patches

From Neil Conway
Subject Re: ScanDirections
Date
Msg-id 43FB9CAC.7070104@samurai.com
Whole thread Raw
In response to Re: ScanDirections  (James William Pye <pgsql@jwp.name>)
List pgsql-patches
James William Pye wrote:
> In any case, some parts of the patch merely makes some code make use of the
> sdir.h macros instead of depending on -1/0/1--heapam in particular. At least
> those portions should be applied, no?

I've applied the attached patch to CVS HEAD that just changes the code
to use the sdir.h macros when appropriate. Thanks for the patch.

-Neil
Index: src/backend/access/heap/heapam.c
===================================================================
RCS file: /Users/neilc/postgres/cvs_root/pgsql/src/backend/access/heap/heapam.c,v
retrieving revision 1.206
diff -c -r1.206 heapam.c
*** src/backend/access/heap/heapam.c    11 Jan 2006 08:43:11 -0000    1.206
--- src/backend/access/heap/heapam.c    21 Feb 2006 22:28:35 -0000
***************
*** 172,178 ****
   *        tuple as indicated by "dir"; return the next tuple in scan->rs_ctup,
   *        or set scan->rs_ctup.t_data = NULL if no more tuples.
   *
!  * dir == 0 means "re-fetch the tuple indicated by scan->rs_ctup".
   *
   * Note: the reason nkeys/key are passed separately, even though they are
   * kept in the scan descriptor, is that the caller may not want us to check
--- 172,179 ----
   *        tuple as indicated by "dir"; return the next tuple in scan->rs_ctup,
   *        or set scan->rs_ctup.t_data = NULL if no more tuples.
   *
!  * dir == NoMovementScanDirection means "re-fetch the tuple indicated
!  * by scan->rs_ctup".
   *
   * Note: the reason nkeys/key are passed separately, even though they are
   * kept in the scan descriptor, is that the caller may not want us to check
***************
*** 189,200 ****
   */
  static void
  heapgettup(HeapScanDesc scan,
!            int dir,
             int nkeys,
             ScanKey key)
  {
      HeapTuple    tuple = &(scan->rs_ctup);
      Snapshot    snapshot = scan->rs_snapshot;
      BlockNumber page;
      Page        dp;
      int            lines;
--- 190,202 ----
   */
  static void
  heapgettup(HeapScanDesc scan,
!            ScanDirection dir,
             int nkeys,
             ScanKey key)
  {
      HeapTuple    tuple = &(scan->rs_ctup);
      Snapshot    snapshot = scan->rs_snapshot;
+     bool        backward = ScanDirectionIsBackward(dir);
      BlockNumber page;
      Page        dp;
      int            lines;
***************
*** 205,215 ****
      /*
       * calculate next starting lineoff, given scan direction
       */
!     if (dir > 0)
      {
-         /*
-          * forward scan direction
-          */
          if (!scan->rs_inited)
          {
              /*
--- 207,214 ----
      /*
       * calculate next starting lineoff, given scan direction
       */
!     if (ScanDirectionIsForward(dir))
      {
          if (!scan->rs_inited)
          {
              /*
***************
*** 242,252 ****

          linesleft = lines - lineoff + 1;
      }
!     else if (dir < 0)
      {
-         /*
-          * reverse scan direction
-          */
          if (!scan->rs_inited)
          {
              /*
--- 241,248 ----

          linesleft = lines - lineoff + 1;
      }
!     else if (backward)
      {
          if (!scan->rs_inited)
          {
              /*
***************
*** 352,358 ****
               * otherwise move to the next item on the page
               */
              --linesleft;
!             if (dir < 0)
              {
                  --lpp;            /* move back in this page's ItemId array */
                  --lineoff;
--- 348,354 ----
               * otherwise move to the next item on the page
               */
              --linesleft;
!             if (backward)
              {
                  --lpp;            /* move back in this page's ItemId array */
                  --lineoff;
***************
*** 373,379 ****
          /*
           * return NULL if we've exhausted all the pages
           */
!         if ((dir < 0) ? (page == 0) : (page + 1 >= scan->rs_nblocks))
          {
              if (BufferIsValid(scan->rs_cbuf))
                  ReleaseBuffer(scan->rs_cbuf);
--- 369,375 ----
          /*
           * return NULL if we've exhausted all the pages
           */
!         if (backward ? (page == 0) : (page + 1 >= scan->rs_nblocks))
          {
              if (BufferIsValid(scan->rs_cbuf))
                  ReleaseBuffer(scan->rs_cbuf);
***************
*** 384,390 ****
              return;
          }

!         page = (dir < 0) ? (page - 1) : (page + 1);

          heapgetpage(scan, page);

--- 380,386 ----
              return;
          }

!         page = backward ? (page - 1) : (page + 1);

          heapgetpage(scan, page);

***************
*** 393,399 ****
          dp = (Page) BufferGetPage(scan->rs_cbuf);
          lines = PageGetMaxOffsetNumber((Page) dp);
          linesleft = lines;
!         if (dir < 0)
          {
              lineoff = lines;
              lpp = PageGetItemId(dp, lines);
--- 389,395 ----
          dp = (Page) BufferGetPage(scan->rs_cbuf);
          lines = PageGetMaxOffsetNumber((Page) dp);
          linesleft = lines;
!         if (backward)
          {
              lineoff = lines;
              lpp = PageGetItemId(dp, lines);
***************
*** 421,431 ****
   */
  static void
  heapgettup_pagemode(HeapScanDesc scan,
!                     int dir,
                      int nkeys,
                      ScanKey key)
  {
      HeapTuple    tuple = &(scan->rs_ctup);
      BlockNumber page;
      Page        dp;
      int            lines;
--- 417,428 ----
   */
  static void
  heapgettup_pagemode(HeapScanDesc scan,
!                     ScanDirection dir,
                      int nkeys,
                      ScanKey key)
  {
      HeapTuple    tuple = &(scan->rs_ctup);
+     bool        backward = ScanDirectionIsBackward(dir);
      BlockNumber page;
      Page        dp;
      int            lines;
***************
*** 437,447 ****
      /*
       * calculate next starting lineindex, given scan direction
       */
!     if (dir > 0)
      {
-         /*
-          * forward scan direction
-          */
          if (!scan->rs_inited)
          {
              /*
--- 434,441 ----
      /*
       * calculate next starting lineindex, given scan direction
       */
!     if (ScanDirectionIsForward(dir))
      {
          if (!scan->rs_inited)
          {
              /*
***************
*** 471,481 ****

          linesleft = lines - lineindex;
      }
!     else if (dir < 0)
      {
-         /*
-          * reverse scan direction
-          */
          if (!scan->rs_inited)
          {
              /*
--- 465,472 ----

          linesleft = lines - lineindex;
      }
!     else if (backward)
      {
          if (!scan->rs_inited)
          {
              /*
***************
*** 584,597 ****
               * otherwise move to the next item on the page
               */
              --linesleft;
!             if (dir < 0)
!             {
                  --lineindex;
-             }
              else
-             {
                  ++lineindex;
-             }
          }

          /*
--- 575,584 ----
               * otherwise move to the next item on the page
               */
              --linesleft;
!             if (backward)
                  --lineindex;
              else
                  ++lineindex;
          }

          /*
***************
*** 602,608 ****
          /*
           * return NULL if we've exhausted all the pages
           */
!         if ((dir < 0) ? (page == 0) : (page + 1 >= scan->rs_nblocks))
          {
              if (BufferIsValid(scan->rs_cbuf))
                  ReleaseBuffer(scan->rs_cbuf);
--- 589,595 ----
          /*
           * return NULL if we've exhausted all the pages
           */
!         if (backward ? (page == 0) : (page + 1 >= scan->rs_nblocks))
          {
              if (BufferIsValid(scan->rs_cbuf))
                  ReleaseBuffer(scan->rs_cbuf);
***************
*** 613,626 ****
              return;
          }

!         page = (dir < 0) ? (page - 1) : (page + 1);
!
          heapgetpage(scan, page);

          dp = (Page) BufferGetPage(scan->rs_cbuf);
          lines = scan->rs_ntuples;
          linesleft = lines;
!         if (dir < 0)
              lineindex = lines - 1;
          else
              lineindex = 0;
--- 600,612 ----
              return;
          }

!         page = backward ? (page - 1) : (page + 1);
          heapgetpage(scan, page);

          dp = (Page) BufferGetPage(scan->rs_cbuf);
          lines = scan->rs_ntuples;
          linesleft = lines;
!         if (backward)
              lineindex = lines - 1;
          else
              lineindex = 0;
***************
*** 1008,1022 ****

      HEAPDEBUG_1;                /* heap_getnext( info ) */

-     /*
-      * Note: we depend here on the -1/0/1 encoding of ScanDirection.
-      */
      if (scan->rs_pageatatime)
!         heapgettup_pagemode(scan, (int) direction,
                              scan->rs_nkeys, scan->rs_key);
      else
!         heapgettup(scan, (int) direction,
!                    scan->rs_nkeys, scan->rs_key);

      if (scan->rs_ctup.t_data == NULL)
      {
--- 994,1004 ----

      HEAPDEBUG_1;                /* heap_getnext( info ) */

      if (scan->rs_pageatatime)
!         heapgettup_pagemode(scan, direction,
                              scan->rs_nkeys, scan->rs_key);
      else
!         heapgettup(scan, direction, scan->rs_nkeys, scan->rs_key);

      if (scan->rs_ctup.t_data == NULL)
      {
***************
*** 2745,2757 ****
          {
              scan->rs_cindex = scan->rs_mindex;
              heapgettup_pagemode(scan,
!                                 0,            /* "no movement" */
                                  0,            /* needn't recheck scan keys */
                                  NULL);
          }
          else
              heapgettup(scan,
!                        0,                    /* "no movement" */
                         0,                    /* needn't recheck scan keys */
                         NULL);
      }
--- 2727,2739 ----
          {
              scan->rs_cindex = scan->rs_mindex;
              heapgettup_pagemode(scan,
!                                 NoMovementScanDirection,
                                  0,            /* needn't recheck scan keys */
                                  NULL);
          }
          else
              heapgettup(scan,
!                        NoMovementScanDirection,
                         0,                    /* needn't recheck scan keys */
                         NULL);
      }
Index: src/backend/executor/execMain.c
===================================================================
RCS file: /Users/neilc/postgres/cvs_root/pgsql/src/backend/executor/execMain.c,v
retrieving revision 1.266
diff -c -r1.266 execMain.c
*** src/backend/executor/execMain.c    19 Feb 2006 00:04:26 -0000    1.266
--- src/backend/executor/execMain.c    21 Feb 2006 22:29:16 -0000
***************
*** 218,224 ****
      /*
       * run plan
       */
!     if (direction == NoMovementScanDirection)
          result = NULL;
      else
          result = ExecutePlan(estate,
--- 218,224 ----
      /*
       * run plan
       */
!     if (ScanDirectionIsNoMovement(direction))
          result = NULL;
      else
          result = ExecutePlan(estate,
Index: src/backend/tcop/pquery.c
===================================================================
RCS file: /Users/neilc/postgres/cvs_root/pgsql/src/backend/tcop/pquery.c,v
retrieving revision 1.98
diff -c -r1.98 pquery.c
*** src/backend/tcop/pquery.c    22 Nov 2005 18:17:21 -0000    1.98
--- src/backend/tcop/pquery.c    21 Feb 2006 22:30:56 -0000
***************
*** 795,801 ****
              nprocessed = queryDesc->estate->es_processed;
          }

!         if (direction != NoMovementScanDirection)
          {
              long        oldPos;

--- 795,801 ----
              nprocessed = queryDesc->estate->es_processed;
          }

!         if (!ScanDirectionIsNoMovement(direction))
          {
              long        oldPos;

***************
*** 837,843 ****
              nprocessed = queryDesc->estate->es_processed;
          }

!         if (direction != NoMovementScanDirection)
          {
              if (nprocessed > 0 && portal->atEnd)
              {
--- 837,843 ----
              nprocessed = queryDesc->estate->es_processed;
          }

!         if (!ScanDirectionIsNoMovement(direction))
          {
              if (nprocessed > 0 && portal->atEnd)
              {
***************
*** 890,902 ****

      (*dest->rStartup) (dest, CMD_SELECT, portal->tupDesc);

!     if (direction == NoMovementScanDirection)
      {
          /* do nothing except start/stop the destination */
      }
      else
      {
!         bool        forward = (direction == ForwardScanDirection);

          for (;;)
          {
--- 890,902 ----

      (*dest->rStartup) (dest, CMD_SELECT, portal->tupDesc);

!     if (ScanDirectionIsNoMovement(direction))
      {
          /* do nothing except start/stop the destination */
      }
      else
      {
!         bool        forward = ScanDirectionIsForward(direction);

          for (;;)
          {

pgsql-patches by date:

Previous
From: Martijn van Oosterhout
Date:
Subject: [PATCH] Explicit shell types
Next
From: Andrew Dunstan
Date:
Subject: Re: [HACKERS] windows / initdb oddness