sql: add pg_class.relpartbound column #61162
Conversation
|
This change is |
DBeaver relies on this column existing. Release note (sql change): The pg_catalog.pg_class table now has a relpartbound column. This is only for compatibility, and the column value is always NULL. Release justification: Low-risk, high-benefit change to existing functionality.
| diff, ok := columns[columnName] | ||
| if !ok { | ||
| if !ok || diff == nil { |
There was a problem hiding this comment.
Wait, if the column is expected to be missing and is stored as nil, shouldn't we return false so it is ignored in pg_catalog_test.go line 204?
There was a problem hiding this comment.
well if we return false, then line 204 will skip over this, so the test will pass when it should not.
ah actually, the root issue is that in general, we do want to allow crdb to have extra columns that are not in postgres, since sometimes we add additional columns for other reasons. so this change here will make it so that if we do add a column that overlaps with a postgres column, then it will check that it's an expected OID.
There was a problem hiding this comment.
Ah I thought the intent was for the test to "pass" if it was an expected difference.
so this change here will make it so that if we do add a column that overlaps with a postgres column, then it will check that it's an expected OID.
Gotcha
There was a problem hiding this comment.
oh yeah i think this counts as an unexpected difference so it should fail
the reason i added this is that without this check, the test would fail with a nil pointer error.
|
thanks for the review! bors r=RichardJCai |
|
Build succeeded: |
DBeaver relies on this column existing.
Release note (sql change): The pg_catalog.pg_class table now has a
relpartbound column. This is only for compatibility, and the column
value is always NULL.
Release justification: Low-risk, high-benefit change to existing
functionality.