From: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> |
---|---|
To: | PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Andrew Dunstan <andrew(dot)dunstan(at)2ndquadrant(dot)com> |
Subject: | Few cosmetic suggestions for commit 16828d5c (Fast Alter Table Add Column...) |
Date: | 2018-06-02 23:38:23 |
Message-ID: | CAA4eK1K2znsFpC+NQ9A4vxT4uDxADN4RmvHX0L6Y=aHVo9gB4Q@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Some assorted comments:
1.
- When a column is added with <literal>ADD COLUMN</literal>, all existing
- rows in the table are initialized with the column's default value
- (NULL if no <literal>DEFAULT</literal> clause is specified).
- If there is no <literal>DEFAULT</literal> clause, this is merely a
metadata
- change and does not require any immediate update of the table's data;
- the added NULL values are supplied on readout, instead.
+ When a column is added with <literal>ADD COLUMN</literal> and a
+ non-volatile <literal>DEFAULT</literal> is specified, the default is
+ evaluated at the time of the statement and the result stored in the
+ table's metadata. That value will be used for the column for all
existing
+ rows. If no <literal>DEFAULT</literal> is specified, NULL is used. In
+ neither case is a rewrite of the table required.
/the result stored/the result is stored
2.
+/*
+ * Structure used to represent value to be used when the attribute is not
+ * present at all in a tuple, i.e. when the column was created after the
tuple
+ */
+
+typedef struct attrMissing
+{
+ bool ammissingPresent; /* true if non-NULL missing value
exists */
+ Datum ammissing; /* value when attribute is missing */
+} AttrMissing;
+
a. Extra space (empty line) between structure and comments seems
unnecessary.
b. The names of structure members seem little difficult to understand if
you and or others also think so, can we rename them to something like
(missingPresent, missingVal) or (attmissingPresent, attmissingVal) or
something similar.
Patch to address 1 and 2a is attached. What do you think about 2b?
--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
Attachment | Content-Type | Size |
---|---|---|
cosmetic_changes_fast_addcol_v1.patch | application/octet-stream | 1.3 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Justin Pryzby | 2018-06-03 00:42:51 | \d t: ERROR: XX000: cache lookup failed for relation |
Previous Message | Tom Lane | 2018-06-02 22:06:10 | Re: why partition pruning doesn't work? |