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: column padding not preserved like in postgres #49639

Closed
knz opened this issue May 28, 2020 · 4 comments · Fixed by #49886
Closed

sql: column padding not preserved like in postgres #49639

knz opened this issue May 28, 2020 · 4 comments · Fixed by #49886
Assignees
Labels
A-sql-pgcompat Semantic compatibility with PostgreSQL A-sql-pgwire pgwire protocol issues. C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.

Comments

@knz
Copy link
Contributor

knz commented May 28, 2020

tldr: postgres preserves the right-padding of padded columns in some cases, CockroachDB does not.

Start with the following SQL

> CREATE TABLE t(x CHAR(8));
> INSERT INTO t(x) VALUES ('hello');

The run SELECT x FROM t in crdb and pg.

PostgreSQL:

{"Type":"DataRow","Values":[{"text":"hello   "}]}

CockroachDB:

{"Type":"DataRow","Values":[{"text":"hello"}]}

(I found this discrepancy when running the row_description pgtest in both crdb and pg)

It's hard to see in a SQL shell because the whitespace does not show on the screen, but still.

Interestingly, the following query behaves differently:

SELECT ':' || x || ':' FROM t

This returns both a short :hello: in both pg and crdb.

@knz knz 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 A-sql-pgwire pgwire protocol issues. labels May 28, 2020
knz added a commit to knz/cockroach that referenced this issue May 28, 2020
There was some accumulated incompatibilities in the test files.
This patch fixes it, and in the process of doing so discovers
two new bugs (cockroachdb#49639 and cockroachdb#49640).

Summary of changes to the DSL:

- the new `only` directive skips an entire test file if the db
  does not match (used for the crdb-specific portal bug test file)

- the new flags `crdb_only` and `noncrdb_only` skip over a
  test directive if the db does not match.

- the new flags `ignore_table_oids` and `ignore_type_oids` replace
  the corresponding OIDs in the RowDescription message by 0
  prior to comparing with the expected value.

Release note: None
knz added a commit to knz/cockroach that referenced this issue May 28, 2020
There was some accumulated incompatibilities in the test files.
This patch fixes it, and in the process of doing so discovers
two new bugs (cockroachdb#49639 and cockroachdb#49640).

Summary of changes to the DSL:

- the new `only` directive skips an entire test file if the db
  does not match (used for the crdb-specific portal bug test file)

- the new flags `crdb_only` and `noncrdb_only` skip over a
  test directive if the db does not match.

- the new flags `ignore_table_oids` and `ignore_type_oids` replace
  the corresponding OIDs in the RowDescription message by 0
  prior to comparing with the expected value.

Release note: None
@jordanlewis
Copy link
Member

Postgres CHAR types have fixed-width display: https://www.postgresql.org/docs/9.1/datatype-character.html

This could be a good welcome-back issue for @arulajmani.

craig bot pushed a commit that referenced this issue Jun 1, 2020
49641: pgwire/pgtest: make the tests work both on pg and crdb r=rafiss,rohany a=knz

There was some accumulated incompatibilities in the test files.
This patch fixes it, and in the process of doing so discovers
two new bugs (#49639 and #49640).

Summary of changes to the DSL:

- the new `only` directive skips an entire test file if the db
  does not match (used for the crdb-specific portal bug test file)

- the new flags `crdb_only` and `noncrdb_only` skip over a
  test directive if the db does not match.

- the new flags `ignore_table_oids` and `ignore_type_oids` replace
  the corresponding OIDs in the RowDescription message by 0
  prior to comparing with the expected value.

Release note: None

Co-authored-by: Raphael 'kena' Poss <knz@thaumogen.net>
@solongordon solongordon assigned arulajmani and unassigned rohany Jun 2, 2020
arulajmani added a commit to arulajmani/cockroach that referenced this issue Jun 4, 2020
Previously, when returning row values of a column which has type char,
the value wouldn't be padded with spaces on the right if its length
was less than the column width.

This is in contrast to Postgres, which pads values with spaces on the
right to match the column width. This change fixes this by checking
the Oid and width of the value being written, and padding it
appropriately if required.

Fixes cockroachdb#49639

Release note (bug fix): Previously, when streaming values from a column
declared of type char(n), the length of the value could be <= n. After
this change, all values streamed have length exactly n by padding
spaces to the right if necessary.
arulajmani added a commit to arulajmani/cockroach that referenced this issue Jun 4, 2020
Previously, when returning row values of a column which has type char,
the value wouldn't be padded with spaces on the right if its length
was less than the column width.

This is in contrast to Postgres, which pads values with spaces on the
right to match the column width. This change fixes this by checking
the Oid and width of the value being written, and padding it
appropriately if required.

Fixes cockroachdb#49639

Release note (bug fix): Previously, when streaming values from a column
declared of type char(n), the length of the value could be <= n. After
this change, all values streamed have length exactly n by padding
spaces to the right if necessary.
arulajmani added a commit to arulajmani/cockroach that referenced this issue Jun 8, 2020
Previously, when returning row values of a column which has type char,
the value wouldn't be padded with spaces on the right if its length
was less than the column width.

This is in contrast to Postgres, which pads values with spaces on the
right to match the column width. This change fixes this by checking
the Oid and width of the value being written, and padding it
appropriately if required.

Fixes cockroachdb#49639

Release note (bug fix): Previously, when streaming values from a column
declared of type char(n), the length of the value could be <= n. After
this change, all values streamed have length exactly n by padding
spaces to the right if necessary.
arulajmani added a commit to arulajmani/cockroach that referenced this issue Jun 9, 2020
Previously, when returning row values of a column which has type char,
the value wouldn't be padded with spaces on the right if its length
was less than the column width.

This is in contrast to Postgres, which pads values with spaces on the
right to match the column width. This change fixes this by checking
the Oid and width of the value being written, and padding it
appropriately if required.

Fixes cockroachdb#49639

Release note (bug fix): Previously, when streaming values from a column
declared of type char(n), the length of the value could be <= n. After
this change, all values streamed have length exactly n by padding
spaces to the right if necessary.
craig bot pushed a commit that referenced this issue Jun 11, 2020
49886: pgwire: pad char values with spaces if length is less than column width r=solongordon a=arulajmani

Previously, when returning row values of a column which has type char,
the value wouldn't be padded with spaces on the right if its length
was less than the column width.

This is in contrast to Postgres, which pads values with spaces on the
right to match the column width. This change fixes this by checking
the Oid and width of the value being written, and padding it
appropriately if required.

Fixes #49639

Release note (bug fix): Previously, when streaming values from a column
declared of type char(n), the length of the value could be <= n. After
this change, all values streamed have length exactly n by padding
spaces to the right if necessary.

50071: builtins: fix bug in extract epoch from timestamptz in non-UTC timezone r=RaduBerinde a=otan

Release note (bug fix): `extract(epoch from timestamptz)` from a session
time zone which is not in UTC would previously return a value
which has been incorrect offset by the session time zone. This has now
been rectified.

Co-authored-by: arulajmani <arulajmani@gmail.com>
Co-authored-by: Oliver Tan <otan@cockroachlabs.com>
@craig craig bot closed this as completed in 8be299a Jun 11, 2020
@smahable
Copy link

smahable commented Jan 9, 2022

postgres 13.4 on aws aurora is having this problem.

column is 30 char. using length shows actual length
also '['||c||']' shows no right padded string ...

@knz
Copy link
Contributor Author

knz commented Jan 10, 2022

@smahable can you explain more? What problem still exists?

@smahable
Copy link

smahable commented Jan 10, 2022

@knz - this is issue in postgres, not in CockroachDB
as mentioned char(30) - with data less than 30- length function and even select shows issue

create table test(id int, c char(10) not null);
insert into test values(1,'x');
select length(c),'['||c||']' from test;

1 [x]

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 A-sql-pgwire pgwire protocol issues. C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants