sql/tests: TestRandomSyntaxGeneration failed [index visibility precision]

[cockroach-teamcity]

sql/tests.TestRandomSyntaxGeneration failed with artifacts on master @ 4fe2a80d81c6fc5a3da3c7c44c5fc38da67e0367:

    rsg_test.go:832: 1m15s of 5m0s: 159112 executions, 9071 successful
    rsg_test.go:832: 1m20s of 5m0s: 170957 executions, 9699 successful
    rsg_test.go:832: 1m25s of 5m0s: 182776 executions, 10297 successful
    rsg_test.go:832: 1m30s of 5m0s: 194657 executions, 10882 successful
    rsg_test.go:832: 1m35s of 5m0s: 206728 executions, 11480 successful
    rsg_test.go:832: 1m40s of 5m0s: 218400 executions, 12040 successful
    rsg_test.go:832: 1m45s of 5m0s: 229942 executions, 12604 successful
    rsg_test.go:832: 1m50s of 5m0s: 241597 executions, 13158 successful
    rsg_test.go:832: 1m55s of 5m0s: 253943 executions, 13761 successful
    rsg_test.go:832: 2m0s of 5m0s: 266059 executions, 14317 successful
    rsg_test.go:832: 2m5s of 5m0s: 276465 executions, 14805 successful
    rsg_test.go:832: 2m10s of 5m0s: 288190 executions, 15370 successful
    rsg_test.go:832: 2m15s of 5m0s: 300051 executions, 15934 successful
    rsg_test.go:832: 2m20s of 5m0s: 311384 executions, 16457 successful
    rsg_test.go:832: 2m25s of 5m0s: 322533 executions, 16966 successful
    rsg_test.go:832: 2m30s of 5m0s: 331957 executions, 17442 successful
    rsg_test.go:832: 2m35s of 5m0s: 343858 executions, 17963 successful
    rsg_test.go:832: 2m40s of 5m0s: 356058 executions, 18530 successful
    rsg_test.go:832: 2m45s of 5m0s: 368528 executions, 19092 successful
    rsg_test.go:832: 2m50s of 5m0s: 379969 executions, 19621 successful
    rsg_test.go:832: 2m55s of 5m0s: 390832 executions, 20162 successful
    rsg_test.go:832: 3m0s of 5m0s: 401776 executions, 20646 successful
    rsg_test.go:832: 3m5s of 5m0s: 412687 executions, 21138 successful
    rsg_test.go:832: 3m10s of 5m0s: 424595 executions, 21717 successful
    rsg_test.go:832: 3m15s of 5m0s: 436181 executions, 22288 successful
    rsg_test.go:832: 3m20s of 5m0s: 447687 executions, 22797 successful
    rsg_test.go:832: 3m25s of 5m0s: 459526 executions, 23357 successful
    rsg_test.go:832: 3m30s of 5m0s: 470709 executions, 23854 successful
    rsg_test.go:832: 3m35s of 5m0s: 481609 executions, 24407 successful
    rsg_test.go:832: 3m40s of 5m0s: 486925 executions, 24647 successful
    rsg_test.go:832: 3m45s of 5m0s: 491161 executions, 24853 successful
    rsg_test.go:832: 3m50s of 5m0s: 495304 executions, 25048 successful
    rsg_test.go:832: 3m55s of 5m0s: 499025 executions, 25228 successful
    rsg_test.go:832: 4m0s of 5m0s: 503294 executions, 25446 successful
    rsg_test.go:832: 4m5s of 5m0s: 508213 executions, 25671 successful
    rsg_test.go:832: 4m10s of 5m0s: 513178 executions, 25906 successful
    rsg_test.go:832: 4m15s of 5m0s: 518304 executions, 26148 successful
    rsg_test.go:832: 4m20s of 5m0s: 523432 executions, 26391 successful
    rsg_test.go:832: 4m25s of 5m0s: 528217 executions, 26594 successful
    rsg_test.go:832: 4m30s of 5m0s: 533237 executions, 26800 successful
    rsg_test.go:832: 4m35s of 5m0s: 538015 executions, 27020 successful
    rsg_test.go:832: 4m40s of 5m0s: 543098 executions, 27234 successful
    rsg_test.go:832: 4m45s of 5m0s: 548108 executions, 27466 successful
    rsg_test.go:832: 4m50s of 5m0s: 553372 executions, 27686 successful
    rsg_test.go:832: 4m55s of 5m0s: 558231 executions, 27907 successful
    rsg_test.go:868: 562914 executions, 28098 successful
    rsg_test.go:876: Parse followed by Format is not idempotent: "ALTER INDEX IF EXISTS FAMILY . FAMILY . ident VISIBILITY 7.379652426127388e-12" -> "ALTER INDEX IF EXISTS \"family\".\"family\".ident VISIBILITY 0.00" != "ALTER INDEX IF EXISTS \"family\".\"family\".ident NOT VISIBLE"
    rsg_test.go:302: -- test log scope end --
test logs left over in: /artifacts/tmp/_tmp/d437d2c847dfedbc4972f231c3331c8e/logTestRandomSyntaxGeneration135218831
--- FAIL: TestRandomSyntaxGeneration (302.11s)

Help

See also: How To Investigate a Go Test Failure (internal)

/cc @cockroachdb/sql-foundations

This test on roachdash | Improve this report!

Jira issue: CRDB-30306

[rafiss]

rsg_test.go:876: Parse followed by Format is not idempotent:
"ALTER INDEX IF EXISTS FAMILY . FAMILY . ident VISIBILITY 7.379652426127388e-12"
->
"ALTER INDEX IF EXISTS \"family\".\"family\".ident VISIBILITY 0.00" != "ALTER INDEX IF EXISTS \"family\".\"family\".ident NOT VISIBLE"

This syntax was added by SQL Queries, so moving there.

[rafiss]

oh @annrpom just saw you picked it up! you are welcome to work on this if you'd like to get familiar with this

[Xiang-Gu]

The issue is about formatting VISIBILITY value in an index. In code, this is represented as a float64 but the issue arises when we attempt to format it into a string, which presently uses %.2f. It becomes problematic if the value is extremely small but non-zero (e.g. 1e-12), and the format gives back a string "VISIBILITY 0.00", which, if parsed and re-formatted, becomes "NOT VISIBLE".

[michae2]

Maybe we should be using a decimal instead of a float for this value?

[rafiss]

a decimal could have made sense, but i don't think it will be backwards-compatible to change now, since the field is already in the proto:

cockroach/pkg/sql/catalog/descpb/structured.proto

Line 511 in 142b971

optional double invisibility = 29 [(gogoproto.nullable) = false];

i think the real issue in this ticket is not just that the float gets rounded. more specifically, the problem is that if it gets rounded to zero, then the statement formatting logic has a special case:

cockroach/pkg/sql/sem/tree/alter_index.go

Lines 88 to 94 in 142b971

	if invisibility == 0.0 {
	ctx.WriteString(" VISIBLE")
	} else if invisibility == 1.0 {
	ctx.WriteString(" NOT VISIBLE")
	} else {
	ctx.WriteString(" VISIBILITY " + fmt.Sprintf("%.2f", 1-invisibility))
	}

But I do agree that if we just remove the %.2f founding everywhere, that might fix this.

[mgartner]

If VISIBILITY 0 is given, won't that fail to round-trip as well? Do we need to add a boolean to the AST to denote whether or not VISIBILITY or [NOT] VISIBLE was provided?

[mgartner]

Removing the %.2f rounding didn't work super well because values like 0.2 would change into 0.19999..6, so I went with the approach I described above. See #109697.