Carry over hash value for all literal types in remote caching (flyteo…

…rg#378)

Previously, the hash set on an input literal would be ignored if
the literal was of either the `Collection` or `Map` type. This is
fixed here by moving the hash-checking branch up in front of the
special type logic, so that an empty literal with hash is constructed
in either case if a hash is found.

Adds a regression test to verify the hashes are carried over into the
representer literals for both map and collection types.

Signed-off-by: Nicholas Junge <[email protected]>

go/tasks/pluginmachinery/catalog/hashing.go

@@ -13,10 +13,17 @@ var emptyLiteralMap = core.LiteralMap{Literals: map[string]*core.Literal{}}
 // Hashify a literal, in other words, produce a new literal where the corresponding value is removed in case
 // the literal hash is set.
 func hashify(literal *core.Literal) *core.Literal {
+	// If the hash is set, return an empty literal with the same hash,
+	// regardless of type (scalar/collection/map).
+	if literal.GetHash() != "" {
+		return &core.Literal{
+			Hash: literal.GetHash(),
+		}
+	}
+
 	// Two recursive cases:
 	//   1. A collection of literals or
 	//   2. A map of literals
-
 	if literal.GetCollection() != nil {
 		literals := literal.GetCollection().Literals
 		literalsHash := make([]*core.Literal, 0)
@@ -45,12 +52,6 @@ func hashify(literal *core.Literal) *core.Literal {
 		}
 	}
 
-	// And a base case that consists of a scalar, where the hash might be set
-	if literal.GetHash() != "" {
-		return &core.Literal{
-			Hash: literal.GetHash(),
-		}
-	}
 	return literal
 }
 

go/tasks/pluginmachinery/catalog/hashing_test.go

@@ -550,6 +550,64 @@ func TestHashLiteralMap_LiteralsWithHashSet(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "literal map containing hash",
+			literal: &core.Literal{
+				Value: &core.Literal_Map{
+					Map: &core.LiteralMap{
+						Literals: map[string]*core.Literal{
+							"hello": {
+								Value: &core.Literal_Scalar{
+									Scalar: &core.Scalar{
+										Value: &core.Scalar_Primitive{
+											Primitive: &core.Primitive{
+												Value: &core.Primitive_StringValue{
+													StringValue: "world",
+												},
+											},
+										},
+									},
+								},
+							},
+						},
+					},
+				},
+				Hash: "0xffff",
+			},
+			expectedLiteral: &core.Literal{
+				Value: nil,
+				Hash:  "0xffff",
+			},
+		},
+		{
+			name: "literal collection containing hash",
+			literal: &core.Literal{
+				Value: &core.Literal_Collection{
+					Collection: &core.LiteralCollection{
+						Literals: []*core.Literal{
+							{
+								Value: &core.Literal_Scalar{
+									Scalar: &core.Scalar{
+										Value: &core.Scalar_Primitive{
+											Primitive: &core.Primitive{
+												Value: &core.Primitive_Integer{
+													Integer: 42,
+												},
+											},
+										},
+									},
+								},
+							},
+						},
+					},
+				},
+				Hash: "0xabcdef",
+			},
+			expectedLiteral: &core.Literal{
+				Value: nil,
+				Hash:  "0xabcdef",
+			},
+		},
 	}
 
 	for _, tt := range tests {