workload/schemachange: columnIsDependedOn not seeing FKs #127286

annrpom · 2024-07-16T20:50:37Z

***UNEXPECTED ERROR; Received an unexpected execution error.: ERROR: "table_w1_1_col1_w1_3_key" is referenced by foreign key from table "table_w1_1" (SQLSTATE 2BP01)

"ops": [
  "BEGIN",
  {
   "sql": "ALTER TABLE public.table_w1_1 DROP COLUMN \"col1_w1_4\"",
   "potentialExecErr": "42P10"
  }
 ],
 "expectedExecErrors": "",
 "expectedCommitErrors": "",
 "message": "***UNEXPECTED ERROR; Received an unexpected execution error.: ERROR: \"table_w1_1_col1_w1_3_key\" is referenced by foreign key from table \"table_w1_1\" (SQLSTATE 2BP01)",

"sql": "CREATE TABLE public.table_w1_1 (\"col\\\\x0a1_w1_2\" REGCLASS, col1_w1_3 \"char\" NOT NULL, col1_w1_4 INET, \"col)1_w1_5\" BIT(26) NOT NULL, col1_w1_6 DECIMAL NOT NULL, col1_w1_7 FLOAT4, \"c%qol1_w1_8\" INT8 NULL, col1_w1_9 BIT(21) NOT NULL, col1_w1_10 INT4 NOT NULL, \"\ncol1_w1_11\" BOOL NOT NULL, 😈col1_w1_12 INT8 NULL AS (col1_w1_10 + \"c%qol1_w1_8\") STORED, \"co_'\nl 1 _w1_13\" INT8 NULL AS (\"c%qol1_w1_8\" + col1_w1_10) VIRTUAL, \"co(\u000bl\u000b1_w1_14\" STRING NOT NULL AS (lower(col1_w1_3)) STORED, col1_w1_15 INT8 NULL AS (col1_w1_10 + \"c%qol1_w1_8\") VIRTUAL, PRIMARY KEY (col1_w1_6), INDEX (col1_w1_10 DESC, \"col)1_w1_5\", \"co(\u000bl\u000b1_w1_14\" DESC), INDEX (\"c%qol1_w1_8\"), UNIQUE (😈col1_w1_12 ASC, col1_w1_15 ASC, \"c%qol1_w1_8\" DESC), UNIQUE (col1_w1_3 ASC) STORING (col1_w1_4, \"col)1_w1_5\", \"c%qol1_w1_8\", \"\ncol1_w1_11\", \"co(\u000bl\u000b1_w1_14\"), INDEX (col1_w1_6 DESC) STORING (col1_w1_4, \"col)1_w1_5\", col1_w1_7, col1_w1_9, col1_w1_10, 😈col1_w1_12, \"co(\u000bl\u000b1_w1_14\") PARTITION BY LIST (col1_w1_6) (PARTITION \"tabl\\\\U0007A6E0e_1_part_0\" VALUES IN (((-39452947910937236.05):::DECIMAL,), (4.131211305159401117E+21:::DECIMAL,), ((-119702.2057144638860):::DECIMAL,)), PARTITION \"tabl\\\\U0007A6E0e_1_part_1\" VALUES IN (((-62.1408378357815539):::DECIMAL,), ((-1.234E+401):::DECIMAL,), ((-2.771145352935285194E+26):::DECIMAL,)), PARTITION \"tabl\\\\U0007A6E0e_1_part_2\" VALUES IN (((-1.452853155333343117E+32):::DECIMAL,), ((-1.039578189579554490E+23):::DECIMAL,), ((-5.962324792125169045E+30):::DECIMAL,)), PARTITION \"tabl\\\\U0007A6E0e_1_part_3\" VALUES IN (((-1.0):::DECIMAL,), (1.0:::DECIMAL,))), FAMILY (\"\ncol1_w1_11\", col1_w1_9), FAMILY (\"co(\u000bl\u000b1_w1_14\"), FAMILY (\"col)1_w1_5\", col1_w1_3, 😈col1_w1_12, col1_w1_10, col1_w1_7, col1_w1_4, \"col\\\\x0a1_w1_2\", \"c%qol1_w1_8\"), FAMILY (col1_w1_6))"

"sql": "ALTER TABLE public.table_w1_1 ADD CONSTRAINT table_w1_1_col1_w1_3_table_w1_1_col1_w1_3_fk FOREIGN KEY (col1_w1_3) REFERENCES public.table_w1_1 (col1_w1_3) ON DELETE CASCADE ON UPDATE CASCADE",


   {
    "query": "SELECT EXISTS (SELECT source.column_id FROM (SELECT DISTINCT column_id FROM (SELECT unnest(string_to_array(rtrim(ltrim(fd.dependedonby_details, 'Columns: ['), ']'), ' ')::INT8[]) AS column_id FROM crdb_internal.forward_dependencies AS fd WHERE (fd.descriptor_id = $1::REGCLASS) AND (fd.dependedonby_type != 'sequence')) UNION (SELECT unnest(confkey) AS column_id FROM pg_catalog.pg_constraint WHERE confrelid = $1::REGCLASS)) AS cons INNER JOIN (SELECT ordinal_position AS column_id FROM information_schema.columns WHERE ((table_schema = $2) AND (table_name = $3)) AND (column_name = $4)) AS source ON source.column_id = cons.column_id)",
    "queryArgs": [
     "public.table_w1_1",
     "public",
     "table_w1_1",
     "col1_w1_4"
    ],
    "result": false
   },

A simple repro on master-ish shows that the forward_dependencies table looks like:

[email protected]:26257/demoapp/movr> select * from crdb_internal.forward_dependencies;                                
  descriptor_id | descriptor_name | index_id | dependedonby_id | dependedonby_type | dependedonby_index_id | dependedonby_name | dependedonby_details
----------------+-----------------+----------+-----------------+-------------------+-----------------------+-------------------+-----------------------
            106 | users           |     NULL |             107 | fk                |                  NULL | NULL              | NULL
            106 | users           |     NULL |             108 | fk                |                  NULL | NULL              | NULL
            106 | users           |     NULL |             111 | fk                |                  NULL | NULL              | NULL
            107 | vehicles        |     NULL |             108 | fk                |                  NULL | NULL              | NULL
            108 | rides           |     NULL |             109 | fk                |                  NULL | NULL              | NULL
            117 | example_table   |     NULL |             117 | fk                |                  NULL | NULL              | NULL
(6 rows)

during the call to dependedonby_details for crdb_internal.forward_dependencies. Let's use something else

Jira issue: CRDB-40354

Epic CRDB-19168

The text was updated successfully, but these errors were encountered:

annrpom · 2024-07-16T20:52:50Z

could we get it from the json representation?

example here

[email protected]:26257/demoapp/movr> select jsonb_pretty(crdb_internal.pb_to_json('cockroach.sql.sqlbase.Descriptor', 
                                -> descriptor))                                                                     
                                -> from system.descriptor where id = 117;                                           
                        jsonb_pretty
------------------------------------------------------------
  {
      "table": {
          "columns": [
              {
                  "defaultExpr": "unique_rowid()",
                  "id": 1,
                  "name": "id",
                  "type": {
                      "family": "IntFamily",
                      "oid": 20,
                      "width": 64
                  }
              },
              {
                  "id": 2,
                  "name": "parent_id",
                  "nullable": true,
                  "type": {
                      "family": "IntFamily",
                      "oid": 20,
                      "width": 64
                  }
              },
              {
                  "id": 3,
                  "name": "ref_col",
                  "nullable": true,
                  "type": {
                      "family": "IntFamily",
                      "oid": 20,
                      "width": 64
                  }
              },
              {
                  "id": 4,
                  "name": "data",
                  "nullable": true,
                  "type": {
                      "family": "StringFamily",
                      "oid": 1043,
                      "visibleType": 7,
                      "width": 255
                  }
              }
          ],
          "createAsOfTime": {},
          "families": [
              {
                  "columnIds": [
                      1,
                      2,
                      3,
                      4
                  ],
                  "columnNames": [
                      "id",
                      "parent_id",
                      "ref_col",
                      "data"
                  ],
                  "name": "primary"
              }
          ],
          "formatVersion": 3,
          "id": 117,
          "inboundFks": [
              {
                  "constraintId": 3,
                  "name": "fk_parent",
                  "originColumnIds": [
                      2
                  ],
                  "originTableId": 117,
                  "referencedColumnIds": [
                      3
                  ],
                  "referencedTableId": 117
              }
          ],
          "indexes": [
              {
                  "constraintId": 1,
                  "createdAtNanos": "1721162340569709000",
                  "foreignKey": {},
                  "geoConfig": {},
                  "id": 2,
                  "interleave": {},
                  "keyColumnDirections": [
                      "ASC"
                  ],
                  "keyColumnIds": [
                      3
                  ],
                  "keyColumnNames": [
                      "ref_col"
                  ],
                  "keySuffixColumnIds": [
                      1
                  ],
                  "name": "example_table_ref_col_key",
                  "partitioning": {},
                  "sharded": {},
                  "unique": true,
                  "version": 3
              }
          ],
          "modificationTime": {},
          "name": "example_table",
          "nextColumnId": 5,
          "nextConstraintId": 4,
          "nextFamilyId": 1,
          "nextIndexId": 3,
          "nextMutationId": 1,
          "outboundFks": [
              {
                  "constraintId": 3,
                  "name": "fk_parent",
                  "originColumnIds": [
                      2
                  ],
                  "originTableId": 117,
                  "referencedColumnIds": [
                      3
                  ],
                  "referencedTableId": 117
              }
          ],
          "parentId": 104,
          "primaryIndex": {
              "constraintId": 2,
              "createdAtNanos": "1721162340569709000",
              "encodingType": 1,
              "foreignKey": {},
              "geoConfig": {},
              "id": 1,
              "interleave": {},
              "keyColumnDirections": [
                  "ASC"
              ],
              "keyColumnIds": [
                  1
              ],
              "keyColumnNames": [
                  "id"
              ],
              "name": "example_table_pkey",
              "partitioning": {},
              "sharded": {},
              "storeColumnIds": [
                  2,
                  3,
                  4
              ],
              "storeColumnNames": [
                  "parent_id",
                  "ref_col",
                  "data"
              ],
              "unique": true,
              "version": 4
          },
          "privileges": {
              "ownerProto": "demo",
              "users": [
                  {
                      "privileges": "2",
                      "userProto": "admin",
                      "withGrantOption": "2"
                  },
                  {
                      "privileges": "2",
                      "userProto": "root",
                      "withGrantOption": "2"
                  }
              ],
              "version": 3
          },
          "replacementOf": {
              "time": {}
          },
          "unexposedParentSchemaId": 105,
          "version": "1"
      }
  }
(1 row)

annrpom · 2024-07-16T21:13:51Z

during the call to dependedonby_details for crdb_internal.forward_dependencies

ah actually -- this might be a case of me jumping the gun

// Then, this position is used to see if that column has view dependencies
// or foreign key dependencies which would be stored in crdb_internal.forward_dependencies and
// pg_catalog.pg_constraint respectively.

there could just be something wrong with our query -- checking rn

annrpom · 2024-07-18T16:25:05Z

Is the issue that our query doesn't look at if the column we are trying to drop is stored? in this case, we create a unique index

UNIQUE (col1_w1_3 ASC) STORING (col1_w1_4, \"col)1_w1_5\", \"c%qol1_w1_8\", \"\ncol1_w1_11\", \"co(\u000bl\u000b1_w1_14\")

that is on col1_w1_3 -- which has a FK ref back onto itself:

FOREIGN KEY (col1_w1_3) REFERENCES table_w1_1(col1_w1_3) ON DELETE CASCADE ON UPDATE CASCADE

annrpom · 2024-07-30T17:18:52Z

This was suggested; we might have to tweak it:

SELECT (select  r.relname from pg_class r where r.oid = c.confrelid) as base_table,
       a.attname as base_col,
       (select r.relname from pg_class r where r.oid = c.conrelid) as referencing_table,
       UNNEST((select array_agg(attname) from pg_attribute where attrelid = c.conrelid and array[attnum] <@ c.conkey)) as referencing_col,
       pg_get_constraintdef(c.oid) contraint_sql
  FROM pg_constraint c join pg_attribute a on c.confrelid=a.attrelid and a.attnum = ANY(confkey)
 WHERE c.confrelid = (select oid from pg_class where relname = 'rides')
   AND c.confrelid!=c.conrelid;

Previously, drop column support was disabled because we did not properly detect if a column was referenced by foreign keys. This patch adds logic to detect if a column is in use by a foreign key. Fixes: cockroachdb#127286 Release note: None

Previously, drop column support was disabled because we did not properly detect if a column was referenced by foreign keys. We also did not properly detect if removing a column would remove indexes needed to enforce other foreign keys. This patch adds logic if a column or any indexes referencing this column are used by foreign keys. Additionally, other operations are tweaked to allow tables without columns (i.e. all columns dropped only leaving rowid). Fixes: cockroachdb#127286 Release note: None <pkg>: <short description - lowercase, no final period> <what was there before: Previously, ...> <why it needed to change: This was inadequate because ...> <what you did about it: To address this, this patch ...>

Previously, drop column support was disabled because we did not properly detect if a column was referenced by foreign keys. We also did not properly detect if removing a column would remove indexes needed to enforce other foreign keys. This patch adds logic if a column or any indexes referencing this column are used by foreign keys. Additionally, other operations are tweaked to allow tables without columns (i.e. all columns dropped only leaving rowid). Fixes: cockroachdb#127286

128137: workload/schemachange: enable drop column support r=fqazi a=fqazi Previously, drop column support was disabled because we did not properly detect if a column was referenced by foreign keys. We also did not properly detect if removing a column would remove indexes needed to enforce other foreign keys. This patch adds logic if a column or any indexes referencing this column are used by foreign keys. Fixes: #127286 Release note: None Co-authored-by: Faizan Qazi <[email protected]>

annrpom added C-test-failure Broken test (automatically or manually discovered). T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) labels Jul 16, 2024

annrpom self-assigned this Jul 16, 2024

rafiss assigned fqazi and unassigned annrpom Jul 30, 2024

exalate-issue-sync bot added the P-2 Issues/test failures with a fix SLA of 3 months label Jul 30, 2024

This was referenced Jul 30, 2024

roachtest: schemachange/mixed-versions failed [did not see error during DROP DEFAULT] #127320

Closed

roachtest: backup-restore/small-ranges failed #127416

Closed

fqazi mentioned this issue Aug 1, 2024

workload/schemachange: enable drop column support #128121

Closed

fqazi mentioned this issue Aug 2, 2024

workload/schemachange: enable drop column support #128137

Merged

rafiss mentioned this issue Aug 6, 2024

workload/schemachange: handle crdb_internal computed cols in colIsRefByComputed #126967

Closed

craig bot closed this as completed in da31b9a Aug 8, 2024

github-project-automation bot added this to SQL Foundations Aug 28, 2024

github-project-automation bot moved this to Done in SQL Foundations Aug 28, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

workload/schemachange: columnIsDependedOn not seeing FKs #127286

workload/schemachange: columnIsDependedOn not seeing FKs #127286

annrpom commented Jul 16, 2024 •

edited by exalate-issue-sync bot

Loading

annrpom commented Jul 16, 2024 •

edited

Loading

annrpom commented Jul 16, 2024

annrpom commented Jul 18, 2024

annrpom commented Jul 30, 2024

workload/schemachange: columnIsDependedOn not seeing FKs #127286

workload/schemachange: columnIsDependedOn not seeing FKs #127286

Comments

annrpom commented Jul 16, 2024 • edited by exalate-issue-sync bot Loading

annrpom commented Jul 16, 2024 • edited Loading

annrpom commented Jul 16, 2024

annrpom commented Jul 18, 2024

annrpom commented Jul 30, 2024

annrpom commented Jul 16, 2024 •

edited by exalate-issue-sync bot

Loading

annrpom commented Jul 16, 2024 •

edited

Loading