Merge #144259

144259: jsonpath: clean up JSONPath TODOs r=normanchenn a=normanchenn

This commit updates many JSONPath-related TODOs with links to their relevant issues, and removes some unnecessary ones.

Epic: None
Release note: None

Co-authored-by: Norman Chen <[email protected]>

Author: craig[bot]

pkg/sql/conn_executor_exec.go

@@ -58,8 +58,8 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/util/errorutil/unimplemented"
 	"github.com/cockroachdb/cockroach/pkg/util/fsm"
 	"github.com/cockroachdb/cockroach/pkg/util/hlc"
-	// TODO(normanchenn): temporarily import the parser here to ensure that
-	// init() is called.
+	// This import is needed here to properly inject tree.ValidateJSONPath from
+	// pkg/util/jsonpath/parser/parse.go.
 	_ "github.com/cockroachdb/cockroach/pkg/util/jsonpath/parser"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
 	"github.com/cockroachdb/cockroach/pkg/util/log/eventpb"

pkg/sql/distsql_physical_planner.go

@@ -358,7 +358,7 @@ func (v *distSQLExprCheckVisitor) VisitPre(expr tree.Expr) (recurse bool, newExp
 			return false, expr
 		}
 	case *tree.DJsonpath:
-		// TODO(normanchenn): We currently do not have an encoding for jsonpath
+		// TODO(#22513): We currently do not have an encoding for jsonpath
 		// thus do not support it within distsql
 		v.err = newQueryNotSupportedErrorf("jsonpath %s cannot be executed with distsql", t)
 		return false, expr

pkg/sql/opt/memo/interner.go

@@ -910,7 +910,7 @@ func (h *hasher) IsDatumEqual(l, r tree.Datum) bool {
 		}
 		return len(lt.Array) != 0 || h.IsTypeEqual(ltyp, rtyp)
 	case *tree.DJsonpath:
-		// TODO(normanchenn): Workaround until we allow jsonpath encoding.
+		// TODO(#22513): Workaround until we allow jsonpath encoding.
 		rt := r.(*tree.DJsonpath)
 		return h.IsStringEqual(string(*lt), string(*rt))
 	default:

pkg/sql/randgen/datum.go

@@ -582,7 +582,7 @@ func randJSONSimpleDepth(rng *rand.Rand, depth int) json.JSON {
 
 const charSet = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ"
 
-// TODO(normanchenn): Add support for more complex jsonpath queries.
+// TODO(#22513): Add support for more complex jsonpath queries.
 func randJsonpath(rng *rand.Rand) string {
 	var parts []string
 	depth := 1 + rng.Intn(20)

pkg/sql/randgen/type.go

@@ -47,7 +47,7 @@ func init() {
 			// Don't include FLOAT4 due to known bugs that cause test failures.
 			// See #73743 and #48613.
 		case oidext.T_jsonpath:
-			// TODO(normanchenn): Temporarily don't include Jsonpath
+			// TODO(#22513): Temporarily don't include Jsonpath
 		case oid.T_anyarray, oid.T_oidvector, oid.T_int2vector:
 			// Include these.
 			SeedTypes = append(SeedTypes, typ)

pkg/sql/randgen/types_test.go

@@ -34,7 +34,7 @@ loop:
 			// These are not included on purpose.
 			continue loop
 		case types.JsonpathFamily:
-			// TODO(normanchenn): Don't include jsonpath in randomized tests yet.
+			// TODO(#22513): Don't include jsonpath in randomized tests yet.
 			continue loop
 		}
 		noFamilyRepresentative[familyID] = struct{}{}

pkg/sql/scanner/jsonpath_scan.go

@@ -23,7 +23,7 @@ func (s *JSONPathScanner) Scan(lval ScanSymType) {
 		return
 	}
 
-	// TODO(normanchenn): We still need to handle $.Xe where X is any digit.
+	// TODO(#144258): We still need to handle $.Xc where X is any digit and c is any character.
 	switch ch {
 	case '$':
 		// Root path ($)
@@ -129,7 +129,7 @@ func isIdentMiddle(ch int) bool {
 
 // scanIdent is similar to Scanner.scanIdent, but uses Jsonpath tokens.
 func (s *JSONPathScanner) scanIdent(lval ScanSymType) {
-	// TODO(normanchenn): Allow any case for specific identifiers (strict, lax, to)
+	// TODO(#144255): Allow any case for specific identifiers (strict, lax, to)
 	s.normalizeIdent(lval, isIdentMiddle, false /* toLower */)
 	lval.SetID(lexbase.GetKeywordID(lval.Str()))
 }

pkg/sql/sem/eval/math.go

@@ -53,7 +53,6 @@ func DecimalPow(ctx *apd.Context, d, x, y *apd.Decimal) error {
 		d.Set(e)
 		return nil
 	}
-	// TODO(normanchenn): do something with the condition.
 	_, err = ctx.Pow(d, x, y)
 	if err != nil {
 		return err

pkg/sql/sem/tree/constant.go

@@ -552,7 +552,7 @@ var (
 		types.VarBitArray,
 		types.AnyTuple,
 		types.AnyTupleArray,
-		// TODO(normanchenn): Reevaluate conversions for jsonpath array.
+		// TODO(#22513): Reevaluate conversions for jsonpath array.
 	}
 	// StrValAvailBytes is the set of types convertible to byte array.
 	StrValAvailBytes = []*types.T{types.Bytes, types.Uuid, types.String, types.AnyEnum}

pkg/sql/sem/tree/constant_test.go

@@ -726,7 +726,7 @@ func TestStringConstantResolveAvailableTypes(t *testing.T) {
 					continue
 				}
 
-				// TODO(normanchenn): Skip JsonpathFamily for now, since it will resolve to a
+				// TODO(#22513): Skip JsonpathFamily for now, since it will resolve to a
 				// string but in the future we don't want to.
 				if availType.Family() == types.JsonpathFamily {
 					continue