A small tweak to some comments for PartitionKeyData
While doing a bit of work on a partitioning patch I noticed that it's
not really that obvious that there's meant to be exactly 1 item in the
partexprs List for each zero-valued partattrs element. Some incorrect
code using these fields was the cause of CVE-2018-1052, so I think
it's worthwhile to mention how they should be used in the comments.
Patch attached.
--
David Rowley http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
Attachments:
reword_comments_in_partitionkeydata_struct.patchapplication/octet-stream; name=reword_comments_in_partitionkeydata_struct.patchDownload
diff --git a/src/include/utils/partcache.h b/src/include/utils/partcache.h
index 873c60fafd..2939032f0a 100644
--- a/src/include/utils/partcache.h
+++ b/src/include/utils/partcache.h
@@ -26,9 +26,9 @@ typedef struct PartitionKeyData
char strategy; /* partitioning strategy */
int16 partnatts; /* number of columns in the partition key */
AttrNumber *partattrs; /* attribute numbers of columns in the
- * partition key */
+ * partition key or 0 if it's an expr */
List *partexprs; /* list of expressions in the partitioning
- * key, or NIL */
+ * key, one for each zero-valued partattrs */
Oid *partopfamily; /* OIDs of operator families */
Oid *partopcintype; /* OIDs of opclass declared input data types */
On 2018/10/25 12:43, David Rowley wrote:
While doing a bit of work on a partitioning patch I noticed that it's
not really that obvious that there's meant to be exactly 1 item in the
partexprs List for each zero-valued partattrs element. Some incorrect
code using these fields was the cause of CVE-2018-1052, so I think
it's worthwhile to mention how they should be used in the comments.Patch attached.
Good idea.
+ * key, one for each zero-valued partattrs */
How about: for each zero-valued member of partattrs?
Regards,
Amit
On 25 October 2018 at 16:46, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote:
+ * key, one for each zero-valued partattrs */
How about: for each zero-valued member of partattrs?
Aren't arrays made up of elements? I did have "element" on the end,
but I didn't think it was worth having the extra line caused by the 80
line length limit, so I deleted it.
Seems "element" is mentioned 73 times in [1]https://en.wikipedia.org/wiki/Array_data_structure, but "member" does not
get a mention.
[1]: https://en.wikipedia.org/wiki/Array_data_structure
--
David Rowley http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
On 2018/10/25 12:54, David Rowley wrote:
On 25 October 2018 at 16:46, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote:
+ * key, one for each zero-valued partattrs */
How about: for each zero-valued member of partattrs?
Aren't arrays made up of elements? I did have "element" on the end,
but I didn't think it was worth having the extra line caused by the 80
line length limit, so I deleted it.Seems "element" is mentioned 73 times in [1], but "member" does not
get a mention.
I thought they can be used interchangeably, but perhaps not.
Anyway, it's just that "each zero-valued partattrs" sounds a bit odd to
me, especially because it you seem to be referring to the previous array
field 'partattrs'. It would've sounded better with "each zero-valued
partition attribute", for example, but again that's too long for the line too.
Thanks,
Amit
On 25 October 2018 at 17:05, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote:
On 2018/10/25 12:54, David Rowley wrote:
On 25 October 2018 at 16:46, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote:
+ * key, one for each zero-valued partattrs */
How about: for each zero-valued member of partattrs?
Aren't arrays made up of elements? I did have "element" on the end,
but I didn't think it was worth having the extra line caused by the 80
line length limit, so I deleted it.Seems "element" is mentioned 73 times in [1], but "member" does not
get a mention.I thought they can be used interchangeably, but perhaps not.
A struct or a class has members.
--
David Rowley http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
On 2018/10/25 13:13, David Rowley wrote:
On 25 October 2018 at 17:05, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote:
On 2018/10/25 12:54, David Rowley wrote:
On 25 October 2018 at 16:46, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote:
+ * key, one for each zero-valued partattrs */
How about: for each zero-valued member of partattrs?
Aren't arrays made up of elements? I did have "element" on the end,
but I didn't think it was worth having the extra line caused by the 80
line length limit, so I deleted it.Seems "element" is mentioned 73 times in [1], but "member" does not
get a mention.I thought they can be used interchangeably, but perhaps not.
A struct or a class has members.
You're right.
Thanks,
Amit
On 25/10/2018 05:43, David Rowley wrote:
While doing a bit of work on a partitioning patch I noticed that it's
not really that obvious that there's meant to be exactly 1 item in the
partexprs List for each zero-valued partattrs element. Some incorrect
code using these fields was the cause of CVE-2018-1052, so I think
it's worthwhile to mention how they should be used in the comments.Patch attached.
committed
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Fri, 16 Nov 2018 at 11:23, Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:
committed
Thank you.
--
David Rowley http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services