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/pg_catalog: Fix padding on pg_catalog dropped column attname #120855

Closed
Jeremyyang920 opened this issue Mar 21, 2024 · 0 comments · Fixed by #120861
Closed

sql/pg_catalog: Fix padding on pg_catalog dropped column attname #120855

Jeremyyang920 opened this issue Mar 21, 2024 · 0 comments · Fixed by #120861
Assignees
Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)

Comments

@Jeremyyang920
Copy link
Collaborator

Jeremyyang920 commented Mar 21, 2024

Describe the problem

There is a discrepancy between PG and CRDB in the attname atrribute for dropped columns. In the CRDB implementation now we use %8d to create the padding but that results in 1 . too few as PG always has 8. https://github.com/postgres/postgres/blob/57184c3b5d89763c882a15adfcdb00990a09d382/src/backend/catalog/heap.c#L1708

To Reproduce

Set up a PG and CRDB instance and create the same table definition. Drop a column and query pg_attributes for the dropped column attname.

SELECT attname, atttypid, attnotnull, collname FROM pg_attribute LEFT OUTER JOIN pg_collation ON (pg_collation.oid = pg_attribute.attcollation) WHERE attrelid = <table_oid> AND attnum > 0 ORDER BY attnum;

Expected behavior
Both the PG and CRDB query should return the same attname.

Jira issue: CRDB-36904

@Jeremyyang920 Jeremyyang920 added C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) labels Mar 21, 2024
@Jeremyyang920 Jeremyyang920 self-assigned this Mar 21, 2024
craig bot pushed a commit that referenced this issue Mar 26, 2024
120861: sql/pg_catalog: fix attname for dropped columns r=rafiss a=Jeremyyang920

Previously, the attname columns for dropped columns was padded used a %8d string format. This led to incorrect behavior as it meant a total of 8 '.' characters inclusive of the column ordinal as compared to the PG code base which always pads with 8 '.'. This affected migration tooling since it attempts to verify column names using the returned attname and led to mismatching column reports.

This commit fixes the issue by explicitly setting the padding to 8 '.' to match the PG implementation.

Fixes: #120855

Release note (bug fix): attname for dropped columns is now properly padded with 8 '.' to match PG.

120941: roachprod: add faster restarts, prettier multi-node sql printing r=dt a=dt

A couple minor quality-of-life improvements motivated by working on long-lived, multi-node DRT clusters.

121119: server: delete TestIntentResolution r=arulajmani a=arulajmani

This is an old test which is tightly coupled with how locks are tracked on the client. In particular, it expects all ranged locking requests to use `ResolveIntentRange` requests to resolve their locks; this is expected to change immminently (see #121086). It's also cumbersome to change given the randomization it uses to construct batches.

We've got coverage for things being tested here elsewhere. There's some integration tests in `intent_resolver_integration_test.go` that test things deterministically. There's also tests for the txn pipeliner and txn committer that test lock tracking on the client. I'm probably missing some other examples as well, but deleting this should be safe.

Epic: none

Release note: None

Co-authored-by: Jeremy Yang <[email protected]>
Co-authored-by: David Taylor <[email protected]>
Co-authored-by: Arul Ajmani <[email protected]>
@craig craig bot closed this as completed in a8aec2b Mar 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)
Projects
No open projects
Status: Done
Development

Successfully merging a pull request may close this issue.

1 participant