[PATCH] ALTER TABLE ... SET STORAGE default

Started by Aleksander Alekseevover 3 years ago6 messageshackers
Jump to latest
#1Aleksander Alekseev
aleksander@timescale.com

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

Attachments:

v1-0001-ALTER-TABLE-.-SET-STORAGE-default.patchapplication/octet-stream; name=v1-0001-ALTER-TABLE-.-SET-STORAGE-default.patchDownload+17-12
#2Nikita Malakhov
hukutoc@gmail.com
In reply to: Aleksander Alekseev (#1)
Re: [PATCH] ALTER TABLE ... SET STORAGE default

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
https://postgrespro.ru/

#3Aleksander Alekseev
aleksander@timescale.com
In reply to: Nikita Malakhov (#2)
Re: [PATCH] ALTER TABLE ... SET STORAGE default

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

#4Nikita Malakhov
hukutoc@gmail.com
In reply to: Aleksander Alekseev (#3)
Re: [PATCH] ALTER TABLE ... SET STORAGE default

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
https://postgrespro.ru/

#5Tom Lane
tgl@sss.pgh.pa.us
In reply to: Aleksander Alekseev (#3)
Re: [PATCH] ALTER TABLE ... SET STORAGE default

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

#6Tom Lane
tgl@sss.pgh.pa.us
In reply to: Tom Lane (#5)
Re: [PATCH] ALTER TABLE ... SET STORAGE default

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