Laurenz Albe <[email protected]> writes:
> On Tue, 2025-12-16 at 10:14 +0800, Chao Li wrote:
>> Overall the change looks good to me. I have only one comment about the
>> naming of "oversize_storage". Why not just "storage_parameters" or similar
>> that
>> sounds more straightforward?
> "Storage parameter" is not a good idea, because we use that term for
> something else: CREATE TABLE ... WITH (storage_parameter = value)
> I think "oversize_storage" expresses well what is regulated here.
> I'm open to "toast_options" or similar as an alternative, but I think
> it might be better to avoid jargon - not everybody reading that page
> will be familiar with the term.
I didn't like that name, and also didn't like cramming both the
STORAGE and COMPRESSION clauses into it. Not least because as
written, the patch made it look like you have to write STORAGE
if you intend to write COMPRESSION. I think a reasonable
compromise is to keep them separate, like so:
+ { <replaceable class="parameter">column_name</replaceable> <replaceable
class="parameter">data_type</replaceable> [
<replaceable>column_storage</replaceable> ] [
<replaceable>column_compression</replaceable> ] [ COLLATE <replaceable
class="parameter">collation</replaceable> ] [
<replaceable>column_constraint</replaceable> [ ... ] ]
...
+<phrase>and <replaceable>column_storage</replaceable> is:</phrase>
+
+STORAGE { PLAIN | EXTERNAL | EXTENDED | MAIN | DEFAULT }
+
+<phrase>and <replaceable>column_compression</replaceable> is:</phrase>
+
+COMPRESSION <replaceable>compression_method</replaceable>
This does make the first-quoted synopsis line a little longer than
David had it, but it's still not any longer than several other
lines in the CREATE TABLE synopsis. So let's call it good and
not let line length minimization push us into a confusing grammar.
Pushed like that.
BTW, I see that persistence_mode corresponds to OptTemp in gram.y,
and we use OptTemp in several other CREATE statements: CREATE TABLE
AS, CREATE SEQUENCE, CREATE VIEW, and now also CREATE PROPERTY GRAPH.
I fixed up CREATE TABLE AS to match, but left the others alone
because they don't currently admit to accepting GLOBAL/LOCAL at all.
regards, tom lane
