A small tweak to some comments for PartitionKeyData

Started by David Rowleyover 7 years ago8 messageshackers
Jump to latest
#1David Rowley
dgrowleyml@gmail.com

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+2-2
#2Amit Langote
Langote_Amit_f8@lab.ntt.co.jp
In reply to: David Rowley (#1)
Re: A small tweak to some comments for PartitionKeyData

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

#3David Rowley
dgrowleyml@gmail.com
In reply to: Amit Langote (#2)
Re: A small tweak to some comments for PartitionKeyData

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

#4Amit Langote
Langote_Amit_f8@lab.ntt.co.jp
In reply to: David Rowley (#3)
Re: A small tweak to some comments for PartitionKeyData

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.

[1] https://en.wikipedia.org/wiki/Array_data_structure

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

#5David Rowley
dgrowleyml@gmail.com
In reply to: Amit Langote (#4)
Re: A small tweak to some comments for PartitionKeyData

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.

[1] https://en.wikipedia.org/wiki/Array_data_structure

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

#6Amit Langote
Langote_Amit_f8@lab.ntt.co.jp
In reply to: David Rowley (#5)
Re: A small tweak to some comments for PartitionKeyData

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.

[1] https://en.wikipedia.org/wiki/Array_data_structure

I thought they can be used interchangeably, but perhaps not.

A struct or a class has members.

You're right.

Thanks,
Amit

#7Peter Eisentraut
peter_e@gmx.net
In reply to: David Rowley (#1)
Re: A small tweak to some comments for PartitionKeyData

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

#8David Rowley
dgrowleyml@gmail.com
In reply to: Peter Eisentraut (#7)
Re: A small tweak to some comments for PartitionKeyData

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