sql: Improve AS OF SYSTEM TIME error message

[arulajmani]

Improve the error message to hint the SET TRANSACTION syntax instead
of saying AOST can't be generally used inside a transaction.

Fixes #40204

Release note: None

Release justification: Minor change

[cockroach-teamcity]

This change is 

[rohany]

LGTM, should leave a note about why the test case was changed too.

Needs @jordanlewis to sign off for landing though.

[solongordon]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @arulajmani and @solongordon)

---

pkg/sql/as_of_test.go, line 230 at r1 (raw file):

	// Can't use in a transaction.
	_, err = db.Query(
		fmt.Sprintf("BEGIN; SELECT a FROM d.t AS OF SYSTEM TIME %s; COMMIT;", tsVal1))

I don't see any reason to delete this test.

---

pkg/sql/as_of_test.go, line 233 at r1 (raw file):

			"SET TRANSACTION AS OF SYSTEM TIME %s; "+
			"SELECT a FROM d.t; "+
			"COMMIT;", tsVal1)).Scan(&i); err != nil {

Nit: You can use backticks to include newlines in your string:

	if err := db.QueryRow(fmt.Sprintf(`
		BEGIN;
		SET TRANSACTION AS OF SYSTEM TIME %s;
		SELECT a FROM d.t;
		COMMIT;
	`, tsVal1)).Scan(&i); err != nil {

---

pkg/sql/conn_executor_exec.go, line 392 at r1 (raw file):

					"inconsistent AS OF SYSTEM TIME timestamp; expected: %s", origTs)
				err = errors.WithHint(err, fmt.Sprintf("See %s for help with AS OF SYSTEM TIME",
					base.DocsURL("as-of-system-time.html#using-as-of-system-time-in-transactions")))

I inquired about including docs URLs in errors recently, and the current guidance was to avoid this until we have better guardrails around it. See #40799. Maybe for now we could just change the hint to "try SET TRANSACTION AS OF SYSTEM TIME".

[arulajmani]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @solongordon)

---

pkg/sql/as_of_test.go, line 230 at r1 (raw file):

Previously, solongordon (Solon) wrote…

I don't see any reason to delete this test.

Done.

---

pkg/sql/conn_executor_exec.go, line 392 at r1 (raw file):

Previously, solongordon (Solon) wrote…

I inquired about including docs URLs in errors recently, and the current guidance was to avoid this until we have better guardrails around it. See #40799. Maybe for now we could just change the hint to "try SET TRANSACTION AS OF SYSTEM TIME".

Done.

[solongordon]

Reviewable status: complete! 0 of 0 LGTMs obtained

[arulajmani]

bors r+

[craig]

Build failed (retrying...)

• GitHub CI (Cockroach)

[craig]

Build succeeded

• GitHub CI (Cockroach)

[awoods187]

@nstewart this is merged now FYI

[nstewart]

awesome. nice usability win. Thank you @arulajmani!