Few cosmetic suggestions for commit 16828d5c (Fast Alter Table Add Column...)

Поиск
Список
Период
Сортировка
От Amit Kapila
Тема Few cosmetic suggestions for commit 16828d5c (Fast Alter Table Add Column...)
Дата
Msg-id CAA4eK1K2znsFpC+NQ9A4vxT4uDxADN4RmvHX0L6Y=aHVo9gB4Q@mail.gmail.com
обсуждение исходный текст
Ответы Re: Few cosmetic suggestions for commit 16828d5c (Fast Alter TableAdd Column...)  (Amit Kapila <amit.kapila16@gmail.com>)
Список 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
Вложения

В списке pgsql-hackers по дате отправления:

Предыдущее
От: Tom Lane
Дата:
Сообщение: Re: why partition pruning doesn't work?
Следующее
От: Justin Pryzby
Дата:
Сообщение: \d t: ERROR: XX000: cache lookup failed for relation