Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sql: "char" is supposed to truncate long values #65631

Closed
rafiss opened this issue May 24, 2021 · 2 comments · Fixed by #66422
Closed

sql: "char" is supposed to truncate long values #65631

rafiss opened this issue May 24, 2021 · 2 comments · Fixed by #66422
Labels
A-sql-pgcompat Semantic compatibility with PostgreSQL C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. E-starter Might be suitable for a starter project for new employees or team members. good first issue T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)

Comments

@rafiss
Copy link
Collaborator

rafiss commented May 24, 2021

In postgres:

rafiss@127:postgres> create table example (a "char");
CREATE TABLE

rafiss@127:postgres> insert into example values ('abc');
INSERT 0 1

rafiss@127:postgres> select a from example;
+-----+
| a   |
|-----|
| a   |
+-----+
SELECT 1

In cockroachdb

root@:26257/defaultdb> create table example (a "char");
CREATE TABLE

root@:26257/defaultdb>  insert into example values ('abc');
INSERT 1

root@:26257/defaultdb>  select a from example;
   a
-------
  abc
(1 row)

Note that this is DIFFERENT than CHAR -- CHAR will only truncate long values if the extra characters are whitespace, and cockroachdb already implements this correctly.

Epic CRDB-7217

@rafiss rafiss added C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. A-sql-pgcompat Semantic compatibility with PostgreSQL good first issue E-starter Might be suitable for a starter project for new employees or team members. T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) labels May 24, 2021
@Polokghosh53
Copy link

I am a beginner to open source contribution. How should I get started with this issue?

@rafiss
Copy link
Collaborator Author

rafiss commented Jun 1, 2021

The change here will be fairly similar to #50492 -- that PR truncates whitespace for CHAR types.

The steps you'll need are:

  1. update casting logic in pkg/sql/sem/tree/casts.go -- the AdjustValueToType function should have a new case for if typ.Oid() == oid.T_char and perform truncation.
  2. update constant resolution logic in pkg/sql/sem/tree/constant.go -- the ResolveAsType should have two cases for if typ.Oid() == oid.T_char
  3. Add tests (you could also start with this step first and work backwards to the code change) -- add tests in pkg/sql/logictest/testdata/logic_test/collatedstring and pkg/sql/sem/tree/testdata/eval/cast similar to the ones in the PR i linked.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-sql-pgcompat Semantic compatibility with PostgreSQL C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. E-starter Might be suitable for a starter project for new employees or team members. good first issue T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants