From: | Andreas Karlsson <andreas(at)proxel(dot)se> |
---|---|
To: | Julien Tachoires <julmon(at)gmail(dot)com>, Alex Shulgin <ash(at)commandprompt(dot)com> |
Cc: | Alvaro Herrera <alvherre(at)commandprompt(dot)com>, Jaime Casanova <jaime(at)2ndquadrant(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: patch : Allow toast tables to be moved to a different tablespace |
Date: | 2015-03-09 23:26:27 |
Message-ID: | 54FE2C23.1080604@proxel.se |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 03/03/2015 04:14 PM, Julien Tachoires wrote:
> On 30/12/2014 03:48, Andreas Karlsson wrote:
>> - A test fails in create_view.out. I looked some into it and did not see
>> how this could happen.
>>
>>[...]
>
> I can't reproduce this issue.
Neither can I anymore.
>> - pg_dump is broken
>>
>> pg_dump: [archiver (db)] query failed: ERROR: syntax error at or
>> near "("
>> LINE 1: ...nest(tc.reloptions) x), ', ') AS toast_reloptions
>> (SELECT sp...
>
> Fixed.
I still get a failing pg_dump test though when running make check-world.
And it does not work when run manually either.
+ pg_dumpall -f
/home/andreas/dev/postgresql/contrib/pg_upgrade/tmp_check/dump1.sql
pg_dump: column number -1 is out of range 0..28
cannot duplicate null pointer (internal error)
pg_dumpall: pg_dump failed on database "postgres", exiting
+ pg_dumpall1_status=1
+ [ /home/andreas/dev/postgresql != /home/andreas/dev/postgresql ]
+
/home/andreas/dev/postgresql/contrib/pg_upgrade/tmp_check/install//home/andreas/dev/postgresql-inst/bin/pg_ctl
-m fast stop
waiting for server to shut down.... done
server stopped
+ [ -n ]
+ [ -n ]
+ [ -n 1 ]
+ echo pg_dumpall of pre-upgrade database cluster failed
pg_dumpall of pre-upgrade database cluster failed
+ exit 1
+ rm -rf /tmp/pg_upgrade_check-5A3wsI
>> - I do not like how \d handles the toast tablespace. Having TOAST in
>> pg_default and the table in another space looks the same as if there was
>> no TOAST table at all. I think we should always print both tablespaces
>> if either of them are not pg_default.
>
> If we do it that way, we should always show the tablespace even if it's
> pg_default in any case to be consistent. I'm pretty sure that we don't
> want that.
I think we will have to agree to disagree here. I think it should be
obvious when the toast table is in the default tablespace for tables
outside it.
>> - Would it be interesting to add syntax for moving the toast index to a
>> separate tablespace?
>
> -1, we just want to be able to move TOAST heap, which is supposed to
> contain a lot of data and we want to move on a different storage device
> / filesystem.
I think we should allow moving the indexes for consistency. With this
patch we can move everything except for TOAST indexes.
>> - There is no warning if you set the toast table space of a table
>> lacking toast. I think there should be one.
>
> A notice is raised now in that case.
Excellent, also add a test case for this.
>> - Missing periods on the ALTER TABLE manual page after "See also CREATE
>> TABLESPACE" (in two places).
>>
>> - Missing period last in the new paragraph added to the storage manual page.
>
> I don't understand those 2 last points ?
I mean that "TOAST table can be moved to a different tablespace with
<command>ALTER TABLE SET TOAST TABLESPACE</>" should be changed to
"TOAST table can be moved to a different tablespace with <command>ALTER
TABLE SET TOAST TABLESPACE</>." since a sentece should always end in ".".
= New comments
- The patch does not apply cleanly anymore, I had to solve to minor
conflicts.
- Rewrap the documentation for SET TABLESPACE.
- You have added a tab in alter_table.sgml.
- Merge "case AT_SetTableTableSpace:" and "case AT_SetToastTableSpace:"
where they both do the same thing, i.e. nothing.
- Should it not be foo_mv in the query from the test suite below?
SELECT spcname FROM pg_class c JOIN pg_class d ON
(c.reltoastrelid=d.oid) JOIN pg_tablespace ON (d.reltablespace =
pg_tablespace.oid) WHERE c.relname = 'foo2';
--
Andreas Karlsson
From | Date | Subject | |
---|---|---|---|
Next Message | Andreas Karlsson | 2015-03-09 23:31:55 | Re: patch : Allow toast tables to be moved to a different tablespace |
Previous Message | Michael Paquier | 2015-03-09 23:25:27 | Re: File based Incremental backup v8 |