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

cannot cast type bigint to uuid #455

Open
mathiasag7 opened this issue Nov 11, 2023 · 12 comments · May be fixed by #478
Open

cannot cast type bigint to uuid #455

mathiasag7 opened this issue Nov 11, 2023 · 12 comments · May be fixed by #478

Comments

@mathiasag7
Copy link
Contributor

mathiasag7 commented Nov 11, 2023

Hello, I put version 1.5.0 on the project from which all this started and I noticed something that is due to the migration file 0010. In fact, we can't go from a BigAutoField to a UUIDField directly, nor can we do the reverse: to do so, we'd have to go through a CharField, which leads us to the fact that migration 0010 has to be a migration that transforms the id field into a Charfield rather than a BigAutoField. Otherwise, we won't be able to use the UUIDField option for ids in this version without using the step of conveying the pk into a CharField and then now into a uuid, whereas we'd like a project without data to be able to choose what type it wants for its basic pk. Here's the error

django.db.utils.ProgrammingError: cannot cast type bigint to uuid
LINE 1: ..."eav_attribute" ALTER COLUMN "id" TYPE uuid USING "id"::uuid

@mathiasag7
Copy link
Contributor Author

`# Generated by Django 4.2.6 on 2023-11-11 09:19

from django.db import migrations, models
import eav.fields

class Migration(migrations.Migration):

dependencies = [
    ('eav', '0009_enchance_naming'),
]

operations = [
    migrations.AlterField(
        model_name='attribute',
        name='datatype',
        field=eav.fields.EavDatatypeField(
            choices=[
                ('text', 'Texte'),
                ('date', 'Date'),
                ('float', 'Nombre décimal'),
                ('int', 'Entier'),
                ('bool', 'Vrai / Faux'),
                ('object', 'Django Object'),
                ('enum', 'Multiple Choice'),
                ('json', 'JSON Object'),
                ('csv', 'Comma-Separated-Value'),
            ],
            max_length=6,
            verbose_name='Data Type',
        ),
    ),
    migrations.AlterField(
        model_name='attribute',
        name='id',
        field=models.CharField(
            editable=False, max_length=40, primary_key=True, serialize=False
        ),
    ),
    migrations.AlterField(
        model_name='enumgroup',
        name='id',
        field=models.CharField(
            editable=False, max_length=40, primary_key=True, serialize=False
        ),
    ),
    migrations.AlterField(
        model_name='enumvalue',
        name='id',
        field=models.CharField(
            editable=False, max_length=40, primary_key=True, serialize=False
        ),
    ),
    migrations.AlterField(
        model_name='value',
        name='id',
        field=models.CharField(
            editable=False, max_length=40, primary_key=True, serialize=False
        ),
    ),
]

0010 migration file that successfully work

@Dresdn
Copy link
Contributor

Dresdn commented Nov 13, 2023

Since we're not going to modify the package to convert all the models PK's to CharField(), you'll need to come up with an alternative solution if you want to change the datatype for the model IDs.

This means you'll need to look at using MIGRATION_MODULES to create your own migrations.

Honestly, this has me thinking the changes from #428 may not have been a great idea. Allowing customization of a 3rd party package model ID datatypes is going to create issues with the migration management. I think the package should just use BigAutoField as I really don't see a use case for needing a different PK datatype.

@mathiasag7
Copy link
Contributor Author

mathiasag7 commented Nov 13, 2023

Okay, no problem if you think so. But just to clarify, it's not like we're converting all PK models to CharField() permanently. When you retrieve the package the pk's are in CharField but as soon as you define the EAV2_PRIMARY_KEY_FIELD parameter, the pk's will be either a BigAutoField or UUIDField. Now, if it's the Charfield that's causing the problem, you can just remove it from the possible values of EAV2_PRIMARY_KEY_FIELD; that way you'll never have pk that are CharFields. As soon as you take the package and run the migrations, it will create a migration for the pk you've defined (BigAuto or UUIDField) and the CharField will just be a transition in case the user decides to use UUIDFields instead of BigAutoFields. Just as a consequence, once you've set the project, you'll never be able to change the type of the pk (for example, if you want to switch from BigAutoField to UUIDField or vice versa, for whatever reason). Now, if that's not adequate or if it's a bit too much, I don't know what to say.

@Dresdn
Copy link
Contributor

Dresdn commented Nov 22, 2023

I spent a little time on this, and I'm not sure I understand what you mean. What I did was replicate the Mozilla Django Local Library project as a starting point and just try to install v1.5.0.

When I set EAV2_PRIMARY_KEY_FIELD to anything other than django.db.models.BigAutoField, running manage.py makemigrations --dry-run attempts to create migration files in the ...site-packages/eav/migrations/ directory, which isn't going to work.

What do you think and what are you seeing? Starting a new project from scratch is one thing, but we'll need to support existing projects too.

@mathiasag7
Copy link
Contributor Author

Hello, let me simplify my point for better clarity. We both understand that it's impossible to directly change the primary key (pk) or any instance field type from BigAutoField or AutoField to a UUIDField, and vice versa. The only viable approach is to first convert it into a CharField, enabling a smooth transition.

Now, in our migrations directory, there are existing migration files that will be applied to the user's database, regardless of the EAV2_PRIMARY_KEY_FIELD value. These files default to a BigAutoField for the pk field. However, this will creates an issue because a BigAutoField or AutoField cannot be directly converted to a UUIDField or vice versa.

To address this problem and allow users to choose between BigAutoField or UUIDField at the beginning of their project, we need to ensure that the initial pk from the migration files is a CharField. However, it's important to note that the pk won't remain a CharField in the long run. While not ideal, having the initial pk as a CharField is a workaround because CharField can be subsequently changed to either BigAutoField or UUIDField without errors.

In summary, if the last migration file sets the pk to a CharField, the user can then run the next migration with their preferred EAV2_PRIMARY_KEY_FIELD setting without encountering errors.

@mathiasag7
Copy link
Contributor Author

mathiasag7 commented Nov 23, 2023

To provide further clarification, I invite you to create a new migration file in the EAV v1.5.0 package project (maybe replicate it and use the replicate one for the test) that transforms the pk from BigAutoField to CharField. Once you've done this, install this specific version of the package into a fresh project, such as the Mozilla Django Local Library project. Subsequently, select either UUIDField or BigAutoField as the pk field type and execute the migration.

Upon doing so, you'll observe that the migration process proceeds without encountering any errors. This practical demonstration in the Mozilla Django Local Library project should provide a clear illustration of the smooth transition from a CharField pk to either UUIDField or BigAutoField, supporting the point made earlier.

@Dresdn
Copy link
Contributor

Dresdn commented Nov 25, 2023

Ahhh - I see what you're saying now. I'll try that out and see if I can replicate your results.

@jacobjove

This comment was marked as off-topic.

@Dresdn

This comment was marked as off-topic.

@Dresdn
Copy link
Contributor

Dresdn commented Dec 2, 2023

@mathiasag7 - Can you open an MR with the changes you're proposing? Might be the best way to get things tested and ensure we're on the same page.

@mathiasag7 mathiasag7 mentioned this issue Dec 4, 2023
@mathiasag7 mathiasag7 linked a pull request Dec 20, 2023 that will close this issue
11 tasks
@mathiasag7
Copy link
Contributor Author

Hello, @Dresdn, it's been almost 2 months since I submitted an MR with no response from you.

@Dresdn
Copy link
Contributor

Dresdn commented Mar 5, 2024

Hi @mathiasag7. I've had a lot going on lately. Can you update the PR? Let's take a look at it once the tests pass.

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