Skip to content

Commit e675d57

Browse files
normanchennnormanchenn
committed
jsonpath: refactor string representation
This commit refactors the JSONPath string representation logic to more closely match Postgres' output format. This new implementation notably removes unnecessary parentheses around operations based on operation precedence. Release note: None
1 parent 70bb4fa commit e675d57

File tree

8 files changed

+185
-97
lines changed

8 files changed

+185
-97
lines changed

‎pkg/sql/logictest/testdata/logic_test/jsonpath‎

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -101,54 +101,54 @@ SELECT bpchar('$. abc [*]':::JSONPATH::JSONPATH)::BPCHAR FROM t ORDER BY 1
101101
$."abc"[*]
102102

103103
query T
104-
SELECT '$.a[*] ? (@.b == 1 && @.c != 1)'::JSONPATH
104+
SELECT '$.a[*] ? (@.b == 1 && @.c != 1)'::JSONPATH;
105105
----
106-
$."a"[*]?(((@."b" == 1) && (@."c" != 1)))
106+
$."a"[*]?(@."b" == 1 && @."c" != 1)
107107

108108
query T
109-
SELECT '$.a[*] ? (@.b != 1)'::JSONPATH
109+
SELECT '$.a[*] ? (@.b != 1)'::JSONPATH;
110110
----
111-
$."a"[*]?((@."b" != 1))
111+
$."a"[*]?(@."b" != 1)
112112

113113
query T
114-
SELECT '$.a[*] ? (@.b < 1)'::JSONPATH
114+
SELECT '$.a[*] ? (@.b < 1)'::JSONPATH;
115115
----
116-
$."a"[*]?((@."b" < 1))
116+
$."a"[*]?(@."b" < 1)
117117

118118
query T
119-
SELECT '$.a[*] ? (@.b <= 1)'::JSONPATH
119+
SELECT '$.a[*] ? (@.b <= 1)'::JSONPATH;
120120
----
121-
$."a"[*]?((@."b" <= 1))
121+
$."a"[*]?(@."b" <= 1)
122122

123123
query T
124-
SELECT '$.a[*] ? (@.b > 1)'::JSONPATH
124+
SELECT '$.a[*] ? (@.b > 1)'::JSONPATH;
125125
----
126-
$."a"[*]?((@."b" > 1))
126+
$."a"[*]?(@."b" > 1)
127127

128128
query T
129-
SELECT '$.a[*] ? (@.b >= 1)'::JSONPATH
129+
SELECT '$.a[*] ? (@.b >= 1)'::JSONPATH;
130130
----
131-
$."a"[*]?((@."b" >= 1))
131+
$."a"[*]?(@."b" >= 1)
132132

133133
query T
134-
SELECT '$.a ? ($.b == 1)'::JSONPATH
134+
SELECT '$.a ? ($.b == 1)'::JSONPATH;
135135
----
136-
$."a"?(($."b" == 1))
136+
$."a"?($."b" == 1)
137137

138138
query T
139-
SELECT '$.a ? (@.b == 1).c ? (@.d == 2)'::JSONPATH
139+
SELECT '$.a ? (@.b == 1).c ? (@.d == 2)'::JSONPATH;
140140
----
141-
$."a"?((@."b" == 1))."c"?((@."d" == 2))
141+
$."a"?(@."b" == 1)."c"?(@."d" == 2)
142142

143143
query T
144-
SELECT '$.a?(@.b==1).c?(@.d==2)'::JSONPATH
144+
SELECT '$.a?(@.b==1).c?(@.d==2)'::JSONPATH;
145145
----
146-
$."a"?((@."b" == 1))."c"?((@."d" == 2))
146+
$."a"?(@."b" == 1)."c"?(@."d" == 2)
147147

148148
query T
149-
SELECT '$ . a ? ( @ . b == 1 ) . c ? ( @ . d == 2 ) '::JSONPATH
149+
SELECT '$ . a ? ( @ . b == 1 ) . c ? ( @ . d == 2 ) '::JSONPATH;
150150
----
151-
$."a"?((@."b" == 1))."c"?((@."d" == 2))
151+
$."a"?(@."b" == 1)."c"?(@."d" == 2)
152152

153153
statement error pgcode 2201B pq: could not parse .* invalid regular expression: error parsing regexp: missing closing \)
154154
SELECT '$ ? (@ like_regex "(invalid pattern")'::JSONPATH

‎pkg/util/jsonpath/BUILD.bazel‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,5 +15,6 @@ go_library(
1515
"//pkg/sql/pgwire/pgcode",
1616
"//pkg/sql/pgwire/pgerror",
1717
"//pkg/util/json",
18+
"@com_github_cockroachdb_errors//:errors",
1819
],
1920
)

‎pkg/util/jsonpath/jsonpath.go‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,17 +5,20 @@
55

66
package jsonpath
77

8+
import "strings"
9+
810
type Jsonpath struct {
911
Strict bool
1012
Path Path
1113
}
1214

1315
func (j Jsonpath) String() string {
14-
var mode string
16+
var sb strings.Builder
1517
if j.Strict {
16-
mode = "strict "
18+
sb.WriteString("strict ")
1719
}
18-
return mode + j.Path.String()
20+
j.Path.ToString(&sb, false /* inKey */, true /* printBrackets */)
21+
return sb.String()
1922
}
2023

2124
// Validate walks the Jsonpath AST. It returns an error if the AST is invalid

‎pkg/util/jsonpath/method.go‎

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,12 @@
55

66
package jsonpath
77

8-
import "fmt"
8+
import (
9+
"fmt"
10+
"strings"
11+
12+
"github.com/cockroachdb/errors"
13+
)
914

1015
type MethodType int
1116

@@ -26,11 +31,13 @@ type Method struct {
2631

2732
var _ Path = Method{}
2833

29-
func (m Method) String() string {
30-
if int(m.Type) < 0 || int(m.Type) >= len(methodTypeStrings) || m.Type == InvalidMethod {
31-
panic(fmt.Sprintf("invalid method type: %d", m.Type))
34+
func (m Method) ToString(sb *strings.Builder, _, _ bool) {
35+
switch m.Type {
36+
case SizeMethod, TypeMethod:
37+
sb.WriteString(fmt.Sprintf(".%s()", methodTypeStrings[m.Type]))
38+
default:
39+
panic(errors.AssertionFailedf("unhandled method type: %d", m.Type))
3240
}
33-
return fmt.Sprintf(".%s()", methodTypeStrings[m.Type])
3441
}
3542

3643
func (m Method) Validate(nestingLevel int, insideArraySubscript bool) error {

‎pkg/util/jsonpath/operation.go‎

Lines changed: 70 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,11 @@
55

66
package jsonpath
77

8-
import "fmt"
8+
import (
9+
"strings"
10+
11+
"github.com/cockroachdb/errors"
12+
)
913

1014
type OperationType int
1115

@@ -64,26 +68,47 @@ type Operation struct {
6468

6569
var _ Path = Operation{}
6670

67-
func (o Operation) String() string {
68-
if int(o.Type) < 0 || int(o.Type) >= len(OperationTypeStrings) || o.Type == OpInvalid {
69-
panic(fmt.Sprintf("invalid operation type: %d", o.Type))
70-
}
71-
// TODO(normanchenn): Fix recursive brackets. When there is a operation like
72-
// 1 == 1 && 1 != 1, postgres will output (1 == 1 && 1 != 1), but we output
73-
// ((1 == 1) && (1 != 1)).
74-
if o.Type == OpLogicalNot {
75-
return fmt.Sprintf("%s(%s)", OperationTypeStrings[o.Type], o.Left)
76-
}
77-
if o.Type == OpPlus || o.Type == OpMinus {
78-
return fmt.Sprintf("(%s%s)", OperationTypeStrings[o.Type], o.Left)
79-
}
80-
if o.Type == OpExists {
81-
return fmt.Sprintf("%s (%s)", OperationTypeStrings[o.Type], o.Left)
82-
}
83-
if o.Type == OpIsUnknown {
84-
return fmt.Sprintf("(%s) %s", o.Left, OperationTypeStrings[o.Type])
71+
func (o Operation) ToString(sb *strings.Builder, _, printBrackets bool) {
72+
switch o.Type {
73+
case OpCompEqual, OpCompNotEqual, OpCompLess, OpCompLessEqual,
74+
OpCompGreater, OpCompGreaterEqual, OpLogicalAnd, OpLogicalOr, OpAdd,
75+
OpSub, OpMult, OpDiv, OpMod, OpStartsWith, OpLikeRegex:
76+
if printBrackets {
77+
sb.WriteString("(")
78+
defer sb.WriteString(")")
79+
}
80+
o.Left.ToString(sb, false /* inKey */, opPriority(o.Left) <= opPriority(o))
81+
sb.WriteString(" ")
82+
sb.WriteString(OperationTypeStrings[o.Type])
83+
sb.WriteString(" ")
84+
o.Right.ToString(sb, false /* inKey */, opPriority(o.Right) <= opPriority(o))
85+
return
86+
case OpLogicalNot:
87+
sb.WriteString("!(")
88+
o.Left.ToString(sb, false /* inKey */, false /* printBrackets */)
89+
sb.WriteString(")")
90+
return
91+
case OpPlus, OpMinus:
92+
if printBrackets {
93+
sb.WriteString("(")
94+
defer sb.WriteString(")")
95+
}
96+
sb.WriteString(OperationTypeStrings[o.Type])
97+
o.Left.ToString(sb, false /* inKey */, opPriority(o.Left) <= opPriority(o))
98+
return
99+
case OpExists:
100+
sb.WriteString("exists (")
101+
o.Left.ToString(sb, false /* inKey */, false /* printBrackets */)
102+
sb.WriteString(")")
103+
return
104+
case OpIsUnknown:
105+
sb.WriteString("(")
106+
o.Left.ToString(sb, false /* inKey */, false /* printBrackets */)
107+
sb.WriteString(") is unknown")
108+
return
109+
default:
110+
panic(errors.AssertionFailedf("unhandled operation type: %d", o.Type))
85111
}
86-
return fmt.Sprintf("(%s %s %s)", o.Left, OperationTypeStrings[o.Type], o.Right)
87112
}
88113

89114
func (o Operation) Validate(nestingLevel int, insideArraySubscript bool) error {
@@ -97,3 +122,28 @@ func (o Operation) Validate(nestingLevel int, insideArraySubscript bool) error {
97122
}
98123
return nil
99124
}
125+
126+
func opPriority(path Path) int {
127+
switch path := path.(type) {
128+
case Operation:
129+
switch path.Type {
130+
case OpLogicalOr:
131+
return 0
132+
case OpLogicalAnd:
133+
return 1
134+
case OpCompEqual, OpCompNotEqual, OpCompLess, OpCompLessEqual,
135+
OpCompGreater, OpCompGreaterEqual, OpStartsWith:
136+
return 2
137+
case OpAdd, OpSub:
138+
return 3
139+
case OpMult, OpDiv, OpMod:
140+
return 4
141+
case OpPlus, OpMinus:
142+
return 5
143+
default:
144+
return 6
145+
}
146+
default:
147+
return 6
148+
}
149+
}

0 commit comments

Comments
 (0)