Copilot commented on code in PR #6521:
URL: https://github.com/apache/hive/pull/6521#discussion_r3629517867
##########
standalone-metastore/metastore-server/src/main/sql/postgres/upgrade-4.2.0-to-4.3.0.postgres.sql:
##########
@@ -5,6 +5,60 @@ ALTER TABLE "MATERIALIZATION_REBUILD_LOCKS" ADD COLUMN
"MRL_CAT_NAME" varchar(12
CREATE INDEX "MIN_HISTORY_WRITE_ID_IDX" ON "MIN_HISTORY_WRITE_ID"
("MH_DATABASE", "MH_TABLE", "MH_WRITEID");
+-- Add surrogate primary keys for HA database replication. For large
transactional
+-- tables, add the column without a default first, backfill via sequence, then
set
+-- NOT NULL and add the PK. This avoids the long ACCESS EXCLUSIVE table
rewrite that
+-- ADD COLUMN ... bigserial PRIMARY KEY takes on populated tables. Counter
tables
+-- contain a single row and keep the one-step form. Plan a maintenance window
+-- for busy metastores as backfill duration scales with
TXN_COMPONENTS/WRITE_SET row counts.
+
+ALTER TABLE "TXN_COMPONENTS" ADD COLUMN "TC_ID" bigint;
+CREATE SEQUENCE "TXN_COMPONENTS_TC_ID_seq" OWNED BY "TXN_COMPONENTS"."TC_ID";
+UPDATE "TXN_COMPONENTS" SET "TC_ID" = nextval('"TXN_COMPONENTS_TC_ID_seq"')
WHERE "TC_ID" IS NULL;
+ALTER TABLE "TXN_COMPONENTS" ALTER COLUMN "TC_ID" SET DEFAULT
nextval('"TXN_COMPONENTS_TC_ID_seq"');
+ALTER TABLE "TXN_COMPONENTS" ALTER COLUMN "TC_ID" SET NOT NULL;
Review Comment:
The backfill order allows a window where concurrent inserts can create new
rows with `TC_ID IS NULL` (because the DEFAULT isn’t set yet at line 18). That
can cause `ALTER COLUMN ... SET NOT NULL` (line 19) to fail (or require another
backfill pass) if the upgrade is run against an active metastore. Consider
setting the DEFAULT immediately after sequence creation (before the UPDATE),
and then backfilling `WHERE ... IS NULL`; alternatively, explicitly `LOCK TABLE
"TXN_COMPONENTS"` for the duration of the backfill/constraint steps. (Same
pattern applies to `COMPLETED_TXN_COMPONENTS`, `COMPACTION_METRICS_CACHE`,
`WRITE_SET`, and `MIN_HISTORY_WRITE_ID` in this script.)
##########
standalone-metastore/metastore-server/src/main/sql/postgres/upgrade-4.2.0-to-4.3.0.postgres.sql:
##########
@@ -5,6 +5,60 @@ ALTER TABLE "MATERIALIZATION_REBUILD_LOCKS" ADD COLUMN
"MRL_CAT_NAME" varchar(12
CREATE INDEX "MIN_HISTORY_WRITE_ID_IDX" ON "MIN_HISTORY_WRITE_ID"
("MH_DATABASE", "MH_TABLE", "MH_WRITEID");
+-- Add surrogate primary keys for HA database replication. For large
transactional
+-- tables, add the column without a default first, backfill via sequence, then
set
+-- NOT NULL and add the PK. This avoids the long ACCESS EXCLUSIVE table
rewrite that
+-- ADD COLUMN ... bigserial PRIMARY KEY takes on populated tables. Counter
tables
+-- contain a single row and keep the one-step form. Plan a maintenance window
+-- for busy metastores as backfill duration scales with
TXN_COMPONENTS/WRITE_SET row counts.
+
+ALTER TABLE "TXN_COMPONENTS" ADD COLUMN "TC_ID" bigint;
+CREATE SEQUENCE "TXN_COMPONENTS_TC_ID_seq" OWNED BY "TXN_COMPONENTS"."TC_ID";
+UPDATE "TXN_COMPONENTS" SET "TC_ID" = nextval('"TXN_COMPONENTS_TC_ID_seq"')
WHERE "TC_ID" IS NULL;
+ALTER TABLE "TXN_COMPONENTS" ALTER COLUMN "TC_ID" SET DEFAULT
nextval('"TXN_COMPONENTS_TC_ID_seq"');
+ALTER TABLE "TXN_COMPONENTS" ALTER COLUMN "TC_ID" SET NOT NULL;
+ALTER TABLE "TXN_COMPONENTS" ADD CONSTRAINT "TXN_COMPONENTS_pkey" PRIMARY KEY
("TC_ID");
+
+ALTER TABLE "COMPLETED_TXN_COMPONENTS" ADD COLUMN "CTC_ID" bigint;
+CREATE SEQUENCE "COMPLETED_TXN_COMPONENTS_CTC_ID_seq" OWNED BY
"COMPLETED_TXN_COMPONENTS"."CTC_ID";
+UPDATE "COMPLETED_TXN_COMPONENTS" SET "CTC_ID" =
nextval('"COMPLETED_TXN_COMPONENTS_CTC_ID_seq"') WHERE "CTC_ID" IS NULL;
+ALTER TABLE "COMPLETED_TXN_COMPONENTS" ALTER COLUMN "CTC_ID" SET DEFAULT
nextval('"COMPLETED_TXN_COMPONENTS_CTC_ID_seq"');
+ALTER TABLE "COMPLETED_TXN_COMPONENTS" ALTER COLUMN "CTC_ID" SET NOT NULL;
+ALTER TABLE "COMPLETED_TXN_COMPONENTS" ADD CONSTRAINT
"COMPLETED_TXN_COMPONENTS_pkey" PRIMARY KEY ("CTC_ID");
+
+ALTER TABLE "COMPACTION_METRICS_CACHE" ADD COLUMN "CMC_ID" bigint;
+CREATE SEQUENCE "COMPACTION_METRICS_CACHE_CMC_ID_seq" OWNED BY
"COMPACTION_METRICS_CACHE"."CMC_ID";
+UPDATE "COMPACTION_METRICS_CACHE" SET "CMC_ID" =
nextval('"COMPACTION_METRICS_CACHE_CMC_ID_seq"') WHERE "CMC_ID" IS NULL;
+ALTER TABLE "COMPACTION_METRICS_CACHE" ALTER COLUMN "CMC_ID" SET DEFAULT
nextval('"COMPACTION_METRICS_CACHE_CMC_ID_seq"');
+ALTER TABLE "COMPACTION_METRICS_CACHE" ALTER COLUMN "CMC_ID" SET NOT NULL;
+ALTER TABLE "COMPACTION_METRICS_CACHE" ADD CONSTRAINT
"COMPACTION_METRICS_CACHE_pkey" PRIMARY KEY ("CMC_ID");
+
+ALTER TABLE "WRITE_SET" ADD COLUMN "WS_ID" bigint;
+CREATE SEQUENCE "WRITE_SET_WS_ID_seq" OWNED BY "WRITE_SET"."WS_ID";
+UPDATE "WRITE_SET" SET "WS_ID" = nextval('"WRITE_SET_WS_ID_seq"') WHERE
"WS_ID" IS NULL;
+ALTER TABLE "WRITE_SET" ALTER COLUMN "WS_ID" SET DEFAULT
nextval('"WRITE_SET_WS_ID_seq"');
+ALTER TABLE "WRITE_SET" ALTER COLUMN "WS_ID" SET NOT NULL;
+ALTER TABLE "WRITE_SET" ADD CONSTRAINT "WRITE_SET_pkey" PRIMARY KEY ("WS_ID");
+
+DROP INDEX IF EXISTS tbl_to_txn_id_idx;
+DROP INDEX IF EXISTS "TBL_TO_TXN_ID_IDX";
+ALTER TABLE "TXN_TO_WRITE_ID" ADD PRIMARY KEY ("T2W_DATABASE", "T2W_TABLE",
"T2W_TXNID");
+
+DROP INDEX IF EXISTS next_write_id_idx;
+DROP INDEX IF EXISTS "NEXT_WRITE_ID_IDX";
+ALTER TABLE "NEXT_WRITE_ID" ADD PRIMARY KEY ("NWI_DATABASE", "NWI_TABLE");
Review Comment:
Dropping the unique indexes before adding the new primary keys removes
uniqueness enforcement during the upgrade window; if the metastore is active,
concurrent writes could introduce duplicates and make the subsequent `ADD
PRIMARY KEY` fail. Safer options are: (1) add the PK “using” the existing
unique index (so enforcement is continuous), or (2) take an explicit table lock
/ run these DDL steps in a sequence that preserves uniqueness until the PK is
in place.
##########
standalone-metastore/metastore-server/src/main/sql/oracle/upgrade-4.2.0-to-4.3.0.oracle.sql:
##########
@@ -3,8 +3,123 @@ SELECT 'Upgrading MetaStore schema from 4.2.0 to 4.3.0' AS
Status from dual;
ALTER TABLE HIVE_LOCKS ADD (HL_CATALOG VARCHAR2(128) DEFAULT 'hive' NOT NULL);
ALTER TABLE MATERIALIZATION_REBUILD_LOCKS ADD (MRL_CAT_NAME VARCHAR2(128)
DEFAULT 'hive' NOT NULL);
+-- Add surrogate primary keys for HA database replication. Oracle IDENTITY
columns
+-- do not backfill existing rows when added via ALTER TABLE, so rebuild each
+-- populated table via a TMP_* copy/swap (same pattern as counter tables
below).
+-- Plan a maintenance window: each swap locks the table and scales with row
count.
+
+CREATE TABLE TMP_TXN_COMPONENTS (
+ TC_ID NUMBER(19) GENERATED BY DEFAULT AS IDENTITY NOT NULL,
+ TC_TXNID NUMBER(19) NOT NULL,
+ TC_DATABASE VARCHAR2(128) NOT NULL,
+ TC_TABLE VARCHAR2(256),
+ TC_PARTITION VARCHAR2(767) NULL,
+ TC_OPERATION_TYPE char(1) NOT NULL,
+ TC_WRITEID NUMBER(19),
+ CONSTRAINT TMP_TXN_COMPONENTS_PK PRIMARY KEY (TC_ID)
+) ROWDEPENDENCIES;
Review Comment:
After the TMP table is renamed to `TXN_COMPONENTS`, the primary key
constraint name remains `TMP_TXN_COMPONENTS_PK`, which becomes misleading in
the final schema (same issue for `TMP_*_PK` constraints throughout this
script). Prefer defining constraints with their final names up front (if they
won’t collide), or explicitly renaming constraints after the table swap so the
resulting schema matches the non-TMP table names.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
