chore: ignore unnecessary char type cast in GetColumns #2278

flyingpigge · 2023-06-21T07:31:58Z

flyingpigge commented

2023-06-21 07:31:58 +00:00

PostgreSQL有两种char类型：character和"char": https://www.postgresql.org/docs/current/datatype-character.html.

pg_class里的relkind是"char"类型，所以GetColumns这里应该转成::"char"或者和本PR里请求的一样去掉转换。这样对于PostgreSQL以及其他兼容PostgreSQL的数据库容错性更好，在做比较时它们通常都会被隐式转换。

postgres=# select pg_typeof(relkind), pg_typeof('a'::char), pg_typeof('a'::"char") from pg_class limit 1;
 pg_typeof | pg_typeof | pg_typeof
-----------+-----------+-----------
 "char"    | character | "char"
(1 row)

PostgreSQL有两种char类型：`character`和`"char"`: https://www.postgresql.org/docs/current/datatype-character.html. `pg_class`里的`relkind`是`"char"`类型，所以`GetColumns`这里应该转成`::"char"`或者和本PR里请求的一样去掉转换。这样对于PostgreSQL以及其他兼容PostgreSQL的数据库容错性更好，在做比较时它们通常都会被隐式转换。 ```sql postgres=# select pg_typeof(relkind), pg_typeof('a'::char), pg_typeof('a'::"char") from pg_class limit 1; pg_typeof | pg_typeof | pg_typeof -----------+-----------+----------- "char" | character | "char" (1 row) ```

flyingpigge added 1 commit 2023-06-21 07:32:03 +00:00

chore: ignore unnecessary char type transform in GetColumns

test mariadb / test mariadb (pull_request) Successful in 5m4s

Details

test mssql / test mssql (pull_request) Successful in 5m55s

Details

test mysql / test mysql (pull_request) Successful in 4m39s

Details

test sqlite / unit test & test sqlite (pull_request) Successful in 8m50s

Details

test tidb / test tidb (pull_request) Successful in 5m26s

Details

test mysql8 / test mysql8 (pull_request) Successful in 7m24s

Details

test postgres / test postgres (pull_request) Successful in 5m31s

Details

test cockroach / test cockroach (pull_request) Successful in 16m15s

Details

e75511eff0

flyingpigge requested review from lunny 2023-06-26 06:13:50 +00:00

lunny commented

2023-06-27 09:32:09 +00:00

Could you write a test for that?

flyingpigge commented

2023-06-29 08:23:04 +00:00

Could you write a test for that?

Well, the type casting involved in this piece are basically some of the internal behavior of the db side, and I'm not sure what kind of test would be appropriate to add in this PR. Since the affected function GetColumns is called in almost all test cases in ci job test postgres, I think it's okay to leave this PR without any test here. Does it LGTY?

> Could you write a test for that? Well, the type casting involved in this piece are basically some of the internal behavior of the db side, and I'm not sure what kind of test would be appropriate to add in this PR. Since the affected function `GetColumns` is called in almost all test cases in ci job `test postgres`, I think it's okay to leave this PR without any test here. Does it LGTY?

👍 1

lunny approved these changes 2023-06-29 10:24:46 +00:00

lunny added 1 commit 2023-06-29 10:25:09 +00:00

Merge branch 'master' into chore/ignore-char-tranform

test mariadb / test mariadb (pull_request) Successful in 4m49s

Details

test mssql / test mssql (pull_request) Successful in 4m2s

Details

test mysql / test mysql (pull_request) Successful in 3m38s

Details

test mysql8 / test mysql8 (pull_request) Successful in 3m31s

Details