diff --git a/netmap/policy.go b/netmap/policy.go index 88265128..52cee8f7 100644 --- a/netmap/policy.go +++ b/netmap/policy.go @@ -455,7 +455,7 @@ func (p PlacementPolicy) WriteStringTo(w io.StringWriter) (err error) { return err } - err = writeFilterStringTo(w, p.filters[i]) + err = writeFilterStringTo(w, p.filters[i], false) if err != nil { return err } @@ -464,7 +464,7 @@ func (p PlacementPolicy) WriteStringTo(w io.StringWriter) (err error) { return nil } -func writeFilterStringTo(w io.StringWriter, f netmap.Filter) error { +func writeFilterStringTo(w io.StringWriter, f netmap.Filter, mayNeedOuterBrackets bool) error { var err error var s string op := f.GetOp() @@ -489,7 +489,7 @@ func writeFilterStringTo(w io.StringWriter, f netmap.Filter) error { if err != nil { return err } - err = writeFilterStringTo(w, inner[0]) + err = writeFilterStringTo(w, inner[0], false) if err != nil { return err } @@ -498,6 +498,13 @@ func writeFilterStringTo(w io.StringWriter, f netmap.Filter) error { return err } } else { + useBrackets := mayNeedOuterBrackets && op == netmap.OR && len(inner) > 1 + if useBrackets { + _, err = w.WriteString("(") + if err != nil { + return err + } + } for i := range inner { if i != 0 { _, err = w.WriteString(" " + op.String() + " ") @@ -505,7 +512,13 @@ func writeFilterStringTo(w io.StringWriter, f netmap.Filter) error { return err } } - err = writeFilterStringTo(w, inner[i]) + err = writeFilterStringTo(w, inner[i], true) + if err != nil { + return err + } + } + if useBrackets { + _, err = w.WriteString(")") if err != nil { return err } diff --git a/netmap/policy_test.go b/netmap/policy_test.go index 05b0c98d..81b7b484 100644 --- a/netmap/policy_test.go +++ b/netmap/policy_test.go @@ -1,6 +1,7 @@ package netmap_test import ( + "strings" "testing" . "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/netmap" @@ -29,6 +30,79 @@ func TestPlacementPolicyEncoding(t *testing.T) { }) } +func TestPlacementPolicyWriteString(t *testing.T) { + var testCases = []struct { + name string + input string + output string // If the output is empty, make it equal to input. + }{ + { + name: "no compound operators", + input: `REP 1 +CBF 1 +SELECT 1 FROM Color +FILTER Color EQ Red AS Color`, + }, + { + name: "no brackets in single level same-operator chain", + input: `REP 1 +CBF 1 +SELECT 1 FROM Color +FILTER Color EQ Red OR Color EQ Blue OR Color EQ Green AS Color`, + }, + { + name: "no brackets aroung higher precedence op", + input: `REP 1 +CBF 1 +SELECT 1 FROM Color +FILTER Color EQ Red OR Color EQ Blue AND Color NE Green AS Color`, + }, + { + name: "no brackets aroung higher precedence op, even if present in the input", + input: `REP 1 +CBF 1 +SELECT 1 FROM Color +FILTER Color EQ Red OR (Color EQ Blue AND Color NE Green) AS Color`, + output: `REP 1 +CBF 1 +SELECT 1 FROM Color +FILTER Color EQ Red OR Color EQ Blue AND Color NE Green AS Color`, + }, + { + name: "brackets aroung lower precedence op", + input: `REP 1 +CBF 1 +SELECT 1 FROM Color +FILTER (Color EQ Red OR Color EQ Blue) AND Color NE Green AS Color`, + }, + { + name: "no extra brackets for bracketed same-operator chain", + input: `REP 1 +CBF 1 +SELECT 1 FROM Color +FILTER (Color EQ Red OR Color EQ Blue OR Color EQ Yellow) AND Color NE Green AS Color`, + }, + } + + for _, tc := range testCases { + var p PlacementPolicy + require.NoError(t, p.DecodeString(tc.input)) + + var sb strings.Builder + require.NoError(t, p.WriteStringTo(&sb)) + + if tc.output == "" { + require.Equal(t, tc.input, sb.String()) + } else { + require.Equal(t, tc.output, sb.String()) + + var p1 PlacementPolicy + require.NoError(t, p1.DecodeString(tc.output)) + require.Equal(t, p, p1) + } + } +} + func TestDecodeSelectFilterExpr(t *testing.T) { for _, s := range []string{ "SELECT 1 FROM *",