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

chore: replace datalayer ids with uuids #1630

Merged
merged 14 commits into from
Mar 5, 2024
Merged

Conversation

davidbgk
Copy link
Contributor

@davidbgk davidbgk commented Feb 19, 2024

  • Use UUIDs rather than default IDs
  • Create migrations
  • Update the URLs
  • Update the tests
  • Do the uuid4 generation with SQL directly
  • Add a test to ensure old layers are still available trough the API
  • Ensure datalayer versioning still works when migrating an already existing map with layers
  • Get the constraint name (relevant stack overflow)
  • Migrate existing datalayers when using a local remoteURL
  • Make the migration reversible
  • Rename datalayer.id to datalayer.old_id

Resources

@almet almet marked this pull request as ready for review February 22, 2024 14:31
@almet
Copy link
Member

almet commented Feb 22, 2024

I've successfully migrated my local database with UUIDs, with the updated branch here.

(edit: ruff formated everything)

),
migrations.RunPython(gen_uuid, reverse_code=migrations.RunPython.noop),
migrations.RunSQL(
"ALTER TABLE umap_datalayer DROP CONSTRAINT umap_datalayer_pkey"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if this will work on other systems, as the constraints seems to be named uniquely on different installs.

umap/urls.py Show resolved Hide resolved
@almet almet self-assigned this Feb 22, 2024
@almet almet force-pushed the datalayer-uuids branch 2 times, most recently from 9ca5800 to 176d03d Compare February 26, 2024 19:25
Copy link
Member

@yohanboniface yohanboniface left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

umap/fields.py Outdated Show resolved Hide resolved
umap/migrations/0018_datalayer_uuid.py Show resolved Hide resolved
umap/migrations/0018_datalayer_uuid.py Show resolved Hide resolved
umap/models.py Show resolved Hide resolved
umap/migrations/0018_datalayer_uuid.py Show resolved Hide resolved
umap/templatetags/umap_tags.py Outdated Show resolved Hide resolved
@yohanboniface yohanboniface added this to the 2.x milestone Feb 27, 2024
@almet
Copy link
Member

almet commented Feb 29, 2024

Here is the generated SQL when running the migrations

ALTER TABLE "umap_datalayer" ALTER COLUMN "uuid" DROP DEFAULT
UPDATE umap_datalayer SET uuid = gen_random_uuid()
DO $$
BEGIN
    EXECUTE 'ALTER TABLE umap_datalayer DROP CONSTRAINT ' || (
        SELECT indexname
        FROM pg_indexes
        WHERE tablename = 'umap_datalayer' AND indexname LIKE '%pk%'
    );
END $$;

ALTER TABLE "umap_datalayer" ALTER COLUMN "id" DROP IDENTITY IF EXISTS
ALTER TABLE "umap_datalayer" ALTER COLUMN "id" TYPE integer, ALTER COLUMN "id" DROP NOT NULL
DROP INDEX IF EXISTS "umap_datalayer_id_5f429f28_like"
ALTER TABLE "umap_datalayer" RENAME COLUMN "id" TO "old_id"
ALTER TABLE "umap_datalayer" ALTER COLUMN "uuid" SET DEFAULT '135add95-f480-4467-9eba-84c7e1827785'::uuid
UPDATE "umap_datalayer" SET "uuid" = '7563076e-f4ed-4f91-886e-f20e73165c1c'::uuid WHERE "uuid" IS NULL; SET CONSTRAINTS ALL IMMEDIATE
ALTER TABLE "umap_datalayer" ALTER COLUMN "uuid" SET NOT NULL
ALTER TABLE "umap_datalayer" ADD CONSTRAINT "umap_datalayer_uuid_5cb47aed_pk" PRIMARY KEY ("uuid")
ALTER TABLE "umap_datalayer" ALTER COLUMN "uuid" DROP DEFAULT

@yohanboniface yohanboniface merged commit 6ed5ebc into master Mar 5, 2024
4 checks passed
@yohanboniface yohanboniface deleted the datalayer-uuids branch March 5, 2024 16:26
@yohanboniface
Copy link
Member

🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants