sql: "char" is supposed to truncate long values

[alxfv]

Resolves #65631

Release note (sql change, backward-incompatible change): The "char"
column type will truncate long values in line with Postgres.

[cockroach-teamcity]

All committers have signed the CLA.

[blathers-crl]

Thank you for contributing to CockroachDB. Please ensure you have followed the guidelines for creating a PR.

Before a member of our team reviews your PR, I have some potential action items for you:

• Please ensure your git commit message contains a release note.
• When CI has completed, please ensure no errors have appeared.

I have added a few people who may be able to assist in reviewing:

• @rafiss (author of sql: "char" is supposed to truncate long values #65631)

_{🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan.}

[cockroach-teamcity]

This change is 

[otan]

thanks! one small improvement

[otan]

do we have to TruncateString again here?
i think we can omit the second usage of TrimRight above as well - oops.

[alxfv]

thank you, good catch

[otan]

this logic is very similar to above and the code in AdjustValueToType.

Do you mind making a routine

func adjustStringToType(typ *types.T, str string) string {
   switch typ.Oid() {
      case oid.T_char:
         // ...
      case oid.T_bpchar:
         // ...
   }
   return str
}

and using it in all three places, replacing the TrimRight && TruncateString code?

[alxfv]

done

[blathers-crl]

Thank you for updating your pull request.

My owl senses detect your PR is good for review. Please keep an eye out for any test failures in CI.

_{🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan.}

[otan]

thanks!

bors r+

[craig]

Build failed (retrying...):

• GitHub CI (Cockroach)

[craig]

Build failed:

• GitHub CI (Cockroach)

[rafiss]

hmm seems like this is causing issues because of how we type CASE expressions (pkg/sql/opt/memo/typing.go):

// typeCase returns the type of a CASE expression, which is
// of the form:
//   CASE [ <cond> ]
//       WHEN <condval1> THEN <expr1>
//     [ WHEN <condval2> THEN <expr2> ] ...
//     [ ELSE <expr> ]
//   END
// The type is equal to the type of the WHEN <condval> THEN <expr> clauses, or
// the type of the ELSE <expr> value if all the previous types are unknown.
func typeCase(e opt.ScalarExpr) *types.T {
	caseExpr := e.(*CaseExpr)
	return InferWhensType(caseExpr.Whens, caseExpr.OrElse)
}

so a CASE where the inputs are all "char" is automatically assumed to output "char". But that's not really what we want here, for one example:

cockroach/pkg/sql/delegate/show_table.go

Lines 196 to 207 in 12eaa93

	func (d *delegator) delegateShowConstraints(n *tree.ShowConstraints) (tree.Statement, error) {
	sqltelemetry.IncrementShowCounter(sqltelemetry.Constraints)
	const getConstraintsQuery = `
	SELECT
	t.relname AS table_name,
	c.conname AS constraint_name,
	CASE c.contype
	WHEN 'p' THEN 'PRIMARY KEY'
	WHEN 'u' THEN 'UNIQUE'
	WHEN 'c' THEN 'CHECK'
	WHEN 'f' THEN 'FOREIGN KEY'
	ELSE c.contype

@otan any idea if it's safe to change that case typing rule?

[otan]

ah that's super gross :(

we really should be asserting that all the THEN clauses have the same type.

i think for now the safest thing to do is change the CASE to CASE c.contype::text

[alxfv]

ah that's super gross :(

we really should be asserting that all the THEN clauses have the same type.

i think for now the safest thing to do is change the CASE to CASE c.contype::text

CASE c.contetype::text didn't work, so I checked:

cockroach/pkg/sql/opt/memo/typing.go

Lines 72 to 81 in 12eaa93

	func InferWhensType(whens ScalarListExpr, orElse opt.ScalarExpr) *types.T {
	result := orElse.DataType()
	// Sanity check.
	for _, when := range whens {
	if !result.Equivalent(when.DataType()) {
	panic(errors.AssertionFailedf("inconsistent Case return types %s %s", when.DataType(), result))
	}
	}
	return result
	}

casting ELSE is enough:

cockroach/pkg/sql/delegate/show_table.go

Lines 202 to 208 in 3309914

	CASE c.contype
	WHEN 'p' THEN 'PRIMARY KEY'
	WHEN 'u' THEN 'UNIQUE'
	WHEN 'c' THEN 'CHECK'
	WHEN 'f' THEN 'FOREIGN KEY'
	ELSE c.contype::TEXT
	END AS constraint_type,

[otan]

nice. lgtm!

bors r+

[craig]

Build succeeded:

• GitHub CI (Cockroach)