planner: if dynamic call fails, or deref of data fails, break to unde…

…fined (#4275)

Due to excessive nesting of blocks, the break statement wasn't breaking
out of enough of them: this would this add an element to the result set
when it should have turned into undefined.

This was the case in two conditions:

a. the dynamic lookup not finding a data function to call, and the resulting
   static data lookup failing to resolve the ref; or
b. the dynamically called function returns undefined

Signed-off-by: Stephan Renatus <stephan.renatus@gmail.com>

internal/compiler/wasm/wasm.go

@@ -1074,8 +1074,8 @@ func (c *Compiler) compileBlock(block *ir.Block) ([]instruction.Instruction, err
 				instrs = append(instrs, instruction.I32Eqz{})
 				instrs = append(instrs, instruction.BrIf{Index: 0})
 			} else {
-				// Booleans and strings would lead to the BrIf (since opa_value_get
-				// on them returns 0), so let's skip that.
+				// Booleans and string sources would lead to the BrIf (since opa_value_get
+				// on them returns 0), so let's skip trying that.
 				instrs = append(instrs, instruction.Br{Index: 0})
 				break
 			}
@@ -1484,7 +1484,7 @@ func (c *Compiler) compileCallDynamicStmt(stmt *ir.CallDynamicStmt, result *[]in
 		instruction.CallIndirect{Index: typeIndex}, // [arg0 arg1 tbl_idx] -> [res]
 		instruction.TeeLocal{Index: c.local(stmt.Result)},
 		instruction.I32Eqz{},
-		instruction.BrIf{Index: 2}, // mapping found, "undefined" result counts
+		instruction.BrIf{Index: 3}, // mapping found, "undefined" result counts
 	)
 
 	*result = append(*result, instrs...)

internal/planner/planner.go

@@ -1473,23 +1473,26 @@ func (p *Planner) planRefData(virtual *ruletrie, base *baseptr, ref ast.Ref, ind
 
 			// We're planning a structure like this:
 			//
-			// block a
-			//   block b
-			//     block c1
-			//       opa_mapping_lookup || br c1
-			//       call_indirect      || br a
-			//       br b
+			// block res
+			//   block a
+			//     block b
+			//       block c1
+			//         opa_mapping_lookup || br c1
+			//         call_indirect      || br res
+			//         br b
+			//       end
+			//       block c2
+			//         dot i   || br c2
+			//         dot i+1 || br c2
+			//         br b
+			//       end
+			//       br a
 			//     end
-			//     block c2
-			//       dot i   || br c2
-			//       dot i+1 || br c2
-			//       br b
-			//     end
-			//     br a
-			//   end
-			//   dot i+2 || br a
-			//   dot i+3 || br a
-			// end
+			//     dot i+2 || br res
+			//     dot i+3 || br res
+			//   end; a
+			//   [add_to_result_set]
+			// end; res
 			//
 			// We have to do it like this because the dot IR stmts
 			// are compiled to `br 0`, the innermost block, if they
@@ -1531,7 +1534,7 @@ func (p *Planner) planRefData(virtual *ruletrie, base *baseptr, ref ast.Ref, ind
 						{ // block "b" in the sketch above
 							Stmts: []ir.Stmt{
 								&ir.BlockStmt{Blocks: []*ir.Block{callDynBlock, dotBlock}},
-								&ir.BreakStmt{Index: 1}},
+								&ir.BreakStmt{Index: 2}},
 						},
 					}},
 				}}

internal/wasm/sdk/test/e2e/external_test.go

@@ -2,6 +2,7 @@
 // Use of this source code is governed by an Apache2
 // license that can be found in the LICENSE file.
 
+//go:build wasm_sdk_e2e
 // +build wasm_sdk_e2e
 
 package e2e

test/wasm/assets/019_call_indirect_optimization.yaml

@@ -120,4 +120,72 @@ cases:
         q:
           baz: 8
     want_result:
-      - x: 100
+      - x: 100
+  - note: func lookup unsuccessful, data deref unsuccessful
+    query: data.test.p = x
+    modules:
+      - |
+        package test
+
+        p {
+          x := "foo"
+          data.other[x].p
+        }
+      - |
+        package other.bar # not "foo", but existence triggers dynamic lookup
+
+        p = true
+    data: {}
+    want_defined: false
+  - note: func lookup successful, but call yields undefined
+    query: data.test.p = x
+    modules:
+      - |
+        package test
+
+        p {
+          x := "foo"
+          data.other[x].p
+        }
+      - package other.foo # empty
+      - |
+        package other.bar # not foo
+
+        p = true
+    data: {}
+    want_defined: false
+  - note: func lookup successful, but call yields undefined (in a loop)
+    query: data.test.p = x
+    modules:
+      - |
+        package test
+        xs := {"foo", "baz", "123"}
+        p {
+          xs[x]
+          data.other[x].p
+        }
+      - package other.foo # empty
+      - |
+        package other.bar # not foo
+
+        p = true
+    data: {}
+    want_defined: false
+  - note: func lookup in a loop, one call yields a result
+    query: data.test.p = x
+    modules:
+      - |
+        package test
+        xs := {"fox", "baz", "123", "xyz"}
+        p = y {
+          xs[x]
+          y := data.other[x].p
+        }
+      - package other.foo # empty
+      - |
+        package other.xyz
+
+        p = "yay"
+    data: {}
+    want_result:
+      - x: yay