Thread: [PATCH] ALTER TABLE ... SET STORAGE default

[PATCH] ALTER TABLE ... SET STORAGE default

From
Aleksander Alekseev
Date:
Hi hackers,

I noticed that we support 'ALTER TABLE ... SET COMPRESSION default'
syntax, but not 'SET STORAGE default' which seems to be a bit
inconsistent. When the user changes the storage mode for a column
there is no convenient way to revert the change.

The proposed patch fixes this.

-- 
Best regards,
Aleksander Alekseev

Attachment

Re: [PATCH] ALTER TABLE ... SET STORAGE default

From
Nikita Malakhov
Date:
Hi hackers!

This seems a little bit confusing and thus very unfriendly for the user, because the actual meaning
of the same 'DEFAULT' option will be different for each data type, and to check storage mode user
has to query full table (or column) description.
I'd rather add a paragraph in documentation describing each data type default storage mode.

On Mon, Aug 22, 2022 at 3:34 PM Aleksander Alekseev <aleksander@timescale.com> wrote:
Hi hackers,

I noticed that we support 'ALTER TABLE ... SET COMPRESSION default'
syntax, but not 'SET STORAGE default' which seems to be a bit
inconsistent. When the user changes the storage mode for a column
there is no convenient way to revert the change.

The proposed patch fixes this.

--
Best regards,
Aleksander Alekseev


--
Regards,
Nikita Malakhov

Re: [PATCH] ALTER TABLE ... SET STORAGE default

From
Aleksander Alekseev
Date:
Hi Nikita,

> This seems a little bit confusing and thus very unfriendly for the user, because the actual meaning
> of the same 'DEFAULT' option will be different for each data type, and to check storage mode user
> has to query full table (or column) description.
> I'd rather add a paragraph in documentation describing each data type default storage mode.

I agree that "SET STORAGE default" syntax leaves much to be desired.

Personally I would prefer "RESET STORAGE" and "RESET COMPRESSION". But
since we already have "SET COMPRESSION default" this going to be
either two commands that do the same thing, or a broken backward
compatibility. Simply removing "SET COMPRESSION default" will make the
syntax consistent too, but again, this would be a broken backward
compatibility. I would argue that a sub-optimal but consistent syntax
that does the job is better than inconsistent syntax and figuring out
the default storage strategy manually.

But let's see what is others people opinion.

-- 
Best regards,
Aleksander Alekseev



Re: [PATCH] ALTER TABLE ... SET STORAGE default

From
Nikita Malakhov
Date:
Hi!

Anyway, adding a paragraph with default storage mode for each standard data type seems 
like a good idea and I'd prepare a patch for it.
Thank you!

On Mon, Aug 22, 2022 at 4:28 PM Aleksander Alekseev <aleksander@timescale.com> wrote:
Hi Nikita,

> This seems a little bit confusing and thus very unfriendly for the user, because the actual meaning
> of the same 'DEFAULT' option will be different for each data type, and to check storage mode user
> has to query full table (or column) description.
> I'd rather add a paragraph in documentation describing each data type default storage mode.

I agree that "SET STORAGE default" syntax leaves much to be desired.

Personally I would prefer "RESET STORAGE" and "RESET COMPRESSION". But
since we already have "SET COMPRESSION default" this going to be
either two commands that do the same thing, or a broken backward
compatibility. Simply removing "SET COMPRESSION default" will make the
syntax consistent too, but again, this would be a broken backward
compatibility. I would argue that a sub-optimal but consistent syntax
that does the job is better than inconsistent syntax and figuring out
the default storage strategy manually.

But let's see what is others people opinion.

--
Best regards,
Aleksander Alekseev


--
Regards,
Nikita Malakhov

Re: [PATCH] ALTER TABLE ... SET STORAGE default

From
Tom Lane
Date:
Aleksander Alekseev <aleksander@timescale.com> writes:
> Hi Nikita,
>> This seems a little bit confusing and thus very unfriendly for the user, because the actual meaning
>> of the same 'DEFAULT' option will be different for each data type, and to check storage mode user
>> has to query full table (or column) description.
>> I'd rather add a paragraph in documentation describing each data type default storage mode.

> I agree that "SET STORAGE default" syntax leaves much to be desired.

FWIW, I don't buy that argument at all.  If you believe that then
you must also think that

    INSERT INTO mytab VALUES (..., DEFAULT, ...);

is a poorly-designed feature because you have to go consult the table
definition to find out what will be inserted.  (Well, maybe you do
think that, but the SQL committee won't agree with you ;-))  So I don't
see any problem with DEFAULT representing a data-type-specific default
in this situation.

> Personally I would prefer "RESET STORAGE" and "RESET COMPRESSION".

Perhaps, but what's done is done, and I agree that STORAGE had better
follow the existing precedent.

I've not read the patch in any detail, but I don't see a problem
with the design.

            regards, tom lane



Re: [PATCH] ALTER TABLE ... SET STORAGE default

From
Tom Lane
Date:
I wrote:
> I've not read the patch in any detail, but I don't see a problem
> with the design.

Hearing no push-back on that position, I reviewed and pushed the
patch.  You'd missed that it also affects CREATE TABLE, but
otherwise it was in pretty good shape.

            regards, tom lane