Jira (PUP-7930) Array delete does not work according to spec when right hand side is hash

9 views
Skip to first unread message

Thomas Hallgren (JIRA)

unread,
Sep 11, 2017, 12:11:03 AM9/11/17
to puppe...@googlegroups.com
Thomas Hallgren created an issue
 
Puppet / Bug PUP-7930
Array delete does not work according to spec when right hand side is hash
Issue Type: Bug Bug
Assignee: Unassigned
Created: 2017/09/10 9:10 PM
Priority: Minor Minor
Reporter: Thomas Hallgren

The following sample can be found in the the Puppet Specifications chapter on Expression, section on "- operator" / Delete:

[1,2,b] - {a => 1, b => 20}     # => [2]

but when evaluating the expression in puppet, the result is [1, 2, b]. Internally, puppet does roughly this:

array - hash.to_a

and since hash.to_a returns an array of 2-element arrays (key pairs) one of two things should change. Either the example should be changed into:

[[1,2],[b,20]] - {a => 1, b => 20} # => [[1,2]]

or the code should change to:

array - hash.to_a.flatten

Add Comment Add Comment
 
This message was sent by Atlassian JIRA (v6.4.14#64029-sha1:ae256fe)
Atlassian logo

Henrik Lindberg (JIRA)

unread,
Sep 13, 2017, 1:46:04 PM9/13/17
to puppe...@googlegroups.com
Henrik Lindberg commented on Bug PUP-7930
 
Re: Array delete does not work according to spec when right hand side is hash

The specification is vague. The example shown for "an array - a hash" is wrong given how the operation is specified (RHS if not an Array is turned into one before deleting). That means that the example should be updated.

It is however questionable if it is at all meaningful to treat a hash as a set of key-value tuples that are deleted from the LHS array. It is good if the LHS is the result of a hash to array conversion, but for all other purposes it just seems wrong. What if you have an array of hashes and want to delete a particular hash - that simply cannot be done.

I therefore think that an "an array - a hash" should really be "an array - [a hash]" to delete it as a value. Other use cases are then supported by first converting the Hash to an Array. If agreeing to this, the spec needs to specify what "RHS is converted to an array" means (i.e. that it is wrapped in an array).

Josh Cooper (JIRA)

unread,
Mar 29, 2019, 1:03:02 AM3/29/19
to puppe...@googlegroups.com
Josh Cooper updated an issue
 
Change By: Josh Cooper
Team: Coremunity Server
This message was sent by Atlassian JIRA (v7.7.1#77002-sha1:e75ca93)
Atlassian logo

Josh Cooper (JIRA)

unread,
Mar 29, 2019, 1:03:03 AM3/29/19
to puppe...@googlegroups.com

Henrik Lindberg (JIRA)

unread,
Mar 29, 2019, 8:33:03 AM3/29/19
to puppe...@googlegroups.com

Henrik Lindberg (JIRA)

unread,
Mar 29, 2019, 8:35:03 AM3/29/19
to puppe...@googlegroups.com
 
Re: Array delete does not work according to spec when right hand side is hash

For PUP 7.0.0 I think we should implement what I proposed above - delete of a hash deletes the entire hash as a value - other types is up to the user by first transforming the hash value (use the keys, use tuples, use values, or something else).

Josh Cooper (Jira)

unread,
Jul 22, 2020, 7:38:04 PM7/22/20
to puppe...@googlegroups.com
Josh Cooper updated an issue
 
Change By: Josh Cooper
Epic Link: PUP-10591
This message was sent by Atlassian Jira (v8.5.2#805002-sha1:a66f935)
Atlassian logo

Josh Cooper (Jira)

unread,
Oct 19, 2020, 2:52:02 PM10/19/20
to puppe...@googlegroups.com

Josh Cooper (Jira)

unread,
Oct 23, 2020, 3:29:03 PM10/23/20
to puppe...@googlegroups.com

It is however questionable if it is at all meaningful to treat a hash as a set of key-value tuples that are deleted from the LHS array.

I agree that it is questionable and the change is not hard

diff --git a/lib/puppet/pops/evaluator/evaluator_impl.rb b/lib/puppet/pops/evaluator/evaluator_impl.rb
index 8bf13d22e8..9c03c069d4 100644
--- a/lib/puppet/pops/evaluator/evaluator_impl.rb
+++ b/lib/puppet/pops/evaluator/evaluator_impl.rb
@@ -1264,7 +1264,6 @@ class EvaluatorImpl
     when Array
       y = case y
       when Array then y
-      when Hash then y.to_a
       else
         [y]
       end

But we have tests that exercise the behavior of deleting hash key tuples from the LHS array, see
https://github.com/puppetlabs/puppet/blob/c2b87527385516b64fbe3ef991ea4417df346e9e/spec/unit/pops/evaluator/collections_ops_spec.rb#L64
https://github.com/puppetlabs/puppet/blob/c2b87527385516b64fbe3ef991ea4417df346e9e/spec/unit/pops/evaluator/evaluating_parser_spec.rb#L102

Given that, I'm inclined to update the specification to match the current behavior and add a note warning about the hash to array conversion. Note deleting hashes from the array works provided the RHS is given as an array of hashes for 1 and 2 level hashes:

$ bx puppet apply -e "notice([{1 => 2}, {3 => 4}] - [{1 => 2}])"
Notice: Scope(Class[main]): [{3 => 4}]
..
$ bx puppet apply -e "notice([{1 => {2 => 3}}, {3 => 4}] - [{1 => {2 => 3}}])"
Notice: Scope(Class[main]): [{3 => 4}]

Josh Cooper (Jira)

unread,
Oct 23, 2020, 7:26:03 PM10/23/20
to puppe...@googlegroups.com
Josh Cooper commented on Bug PUP-7930

Talked this over with Nick Lewis and we concluded that puppet consistently treats a hash as an array of key-value pairs for set addition and subtraction:

[1,2] + {1 => 2} # => [1,2,[1,2]]
[1,2,[3,4]] - {3 => 4} # => [1,2]
[1,2,3,4] - {3 => 4} # => [1,2,3,4]

So we should update the specification to match the current behavior.

Josh Cooper (Jira)

unread,
Oct 26, 2020, 11:34:03 PM10/26/20
to puppe...@googlegroups.com

Josh Cooper (Jira)

unread,
Oct 26, 2020, 11:34:03 PM10/26/20
to puppe...@googlegroups.com

Josh Cooper (Jira)

unread,
Oct 27, 2020, 12:24:03 PM10/27/20
to puppe...@googlegroups.com

Josh Cooper (Jira)

unread,
Oct 27, 2020, 12:25:03 PM10/27/20
to puppe...@googlegroups.com
Reply all
Reply to author
Forward
0 new messages