]> git.ipfire.org Git - thirdparty/postgresql.git/commitdiff
Fix another longstanding problem in copy_relation_data: it was blithely
authorTom Lane <tgl@sss.pgh.pa.us>
Thu, 29 Jul 2010 19:24:05 +0000 (19:24 +0000)
committerTom Lane <tgl@sss.pgh.pa.us>
Thu, 29 Jul 2010 19:24:05 +0000 (19:24 +0000)
assuming that a local char[] array would be aligned on at least a word
boundary.  There are architectures on which that is pretty much guaranteed to
NOT be the case ... and those arches also don't like non-aligned memory
accesses, meaning that log_newpage() would crash if it ever got invoked.
Even on Intel-ish machines there's a potential for a large performance penalty
from doing I/O to an inadequately aligned buffer.  So palloc it instead.

Backpatch to 8.0 --- 7.4 doesn't have this code.

src/backend/commands/tablecmds.c

index f82109bab359e20c6571c51b8e50fe84eb5869ce..bce8750d293b7de093e9354b4e218071b9011b55 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/commands/tablecmds.c,v 1.142.4.13 2010/07/29 16:15:47 rhaas Exp $
+ *       $PostgreSQL: pgsql/src/backend/commands/tablecmds.c,v 1.142.4.14 2010/07/29 19:24:05 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -5729,11 +5729,11 @@ static void
 copy_relation_data(Relation rel, SMgrRelation dst)
 {
        SMgrRelation src;
+       char       *buf;
+       Page            page;
        bool            use_wal;
        BlockNumber nblocks;
        BlockNumber blkno;
-       char            buf[BLCKSZ];
-       Page            page = (Page) buf;
 
        /*
         * Since we copy the data directly without looking at the shared
@@ -5744,6 +5744,15 @@ copy_relation_data(Relation rel, SMgrRelation dst)
         */
        FlushRelationBuffers(rel, 0);
 
+       /*
+        * palloc the buffer so that it's MAXALIGN'd.  If it were just a local
+        * char[] array, the compiler might align it on any byte boundary, which
+        * can seriously hurt transfer speed to and from the kernel; not to
+        * mention possibly making PageSetLSN fail.
+        */
+       buf = (char *) palloc(BLCKSZ);
+       page = (Page) buf;
+
        /*
         * We need to log the copied data in WAL iff WAL archiving is enabled
         * AND it's not a temp rel.
@@ -5807,6 +5816,8 @@ copy_relation_data(Relation rel, SMgrRelation dst)
                smgrwrite(dst, blkno, buf, true);
        }
 
+       pfree(buf);
+
        /*
         * If the rel isn't temp, we must fsync it down to disk before it's
         * safe to commit the transaction.      (For a temp rel we don't care