public inbox for [email protected]  

help / color / mirror / Atom feed

From: Peter Eisentraut <[email protected]>
To: Aleksander Alekseev <[email protected]>
To: pgsql-hackers <[email protected]>
Cc: Matthias van de Meent <[email protected]>
Cc: Teodor Sigaev <[email protected]>
Cc: wenjing zeng <[email protected]>
Subject: Re: CREATE TABLE ( .. STORAGE ..)
Date: Wed, 13 Jul 2022 12:28:30 +0200
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAJ7c6TPsg6iUGZGaEGKi5vdG6VpEcLyZMbXTUQFnCNdi+Kd00Q@mail.gmail.com>
References: <[email protected]>
	<[email protected]>
	<[email protected]>
	<CAEze2WhWef-vemQzZJxQ3SuYANKT32Y1wyNB=NhjP_vfVgY2jQ@mail.gmail.com>
	<[email protected]>
	<CAJ7c6TP5sdpRLbzEvu5yDAw2LRB8OXZFo=ENjhq=bouBzSxh9A@mail.gmail.com>
	<CAJ7c6TM-O3=duY8Zk3S4Joi6s=wuLfePFTFuBgNu65bVZY+PdQ@mail.gmail.com>
	<CAJ7c6TOn1BrFAxWxK7zMfsGyGNqg3YWsou9DpvMp1=JX80909A@mail.gmail.com>
	<[email protected]>
	<CAJ7c6TPsg6iUGZGaEGKi5vdG6VpEcLyZMbXTUQFnCNdi+Kd00Q@mail.gmail.com>

On 12.07.22 12:10, Aleksander Alekseev wrote:
> Hi Peter,
> 
>> The "safety check: do not allow toasted storage modes unless column
>> datatype is TOAST-aware" could be moved into GetAttributeStorage(), so
>> it doesn't have to be repeated.  (Note that GetAttributeCompression()
>> does similar checking.)
> 
> Good point. Fixed.
> 
>> ATExecSetStorage() currently doesn't do any such check, and your patch
>> isn't adding one.  Is there a reason for that?
> 
> ATExecSetStorage() does this, but the check is a bit below [1]. In v7
> I moved the check to GetAttributeStorage() as well.
> 
> [1] https://git.postgresql.org/gitweb/?p=postgresql.git;a=blob;f=src/backend/commands/tablecmds.c#l8312

Committed.

I thought the removal of the documentation details of SET COMPRESSION 
and SET STORAGE from the ALTER TABLE ref page was a bit excessive, since 
that material actually contained useful information about what happens 
when you change compression or storage on a table with existing data. 
So I left that in.  Maybe there is room to deduplicate that material a 
bit, but it would need to be more fine-grained than just removing one 
side of it.

view thread (17+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: CREATE TABLE ( .. STORAGE ..)
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox