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

Defining custom type conversion for schema migrations #127

Closed
CptWesley opened this issue Apr 17, 2024 · 8 comments
Closed

Defining custom type conversion for schema migrations #127

CptWesley opened this issue Apr 17, 2024 · 8 comments

Comments

@CptWesley
Copy link
Contributor

We're currently running into a situation where we (using Marten 6.4.1) are trying to add a duplicated index to an existing table that contains data. We are using ApplyAllConfiguredChangesToDatabaseAsync to apply the schema changes to the database. This appears to be using functionality for schema upgrades that have been implemented in Weasel, hence why I am asking this question here. This seems to work well for any integer and string types, but it seems to fail for DateTime, Guid and custom type properties (which are converted to varchars in the JSON and when querying) when trying to fill the columns.

For the DateTime upgrade it fails with the following message:

      ----> Npgsql.PostgresException : 42804: column "document_time" is of type timestamp without time zone but expression is of type character varying

POSITION: 78
Data:
  Severity: ERROR
  InvariantSeverity: ERROR
  SqlState: 42804
  MessageText: column "document_time" is of type timestamp without time zone but expression is of type character varying
  Hint: You will need to rewrite or cast the expression.
  Position: 78
  File: parse_target.c
  Line: 594
  Routine: transformAssignedExpr

For the Guid upgrade it fails with the following message:

      ----> Npgsql.PostgresException : 42804: column "document_value" is of type uuid but expression is of type character varying

POSITION: 79
Data:
  Severity: ERROR
  InvariantSeverity: ERROR
  SqlState: 42804
  MessageText: column "document_value" is of type uuid but expression is of type character varying
  Hint: You will need to rewrite or cast the expression.
  Position: 79
  File: parse_target.c
  Line: 594
  Routine: transformAssignedExpr

It appears that the migration is not aware that it should cast/transform the string json field when trying to fill the column. Is there any way to configure weasel (or marten) in order to support these casts/transformations?

@RimshaYousafVolue
Copy link

Hi @CptWesley, I am also using ApplyAllDatabaseChangesOnStartup to apply migrations in marten v6.4.0. But if I add a new duplciated field even on int, it gives me error "Weasel.Core.SchemaMigrationException: Cannot derive schema migrations for TableDelta for schema.table AutoCreate.CreateOrUpdate". Is there something wrong with this setup?

services.AddMarten(options =>
{
options.Connection(connectionString);

            options.RegisterDocumentType<Table>();
            options.Schema.For<Table>().Duplicate(x => x.field, null, null, null, true);
        })
            .ApplyAllDatabaseChangesOnStartup();

@CptWesley
Copy link
Contributor Author

@RimshaYousafVolue I'm not sure, the only difference I can quickly spot is that I am not explicitly passing the optional arguments to the .Duplicate function, not sure if you are overriding some of the defaults with a different value.

@RimshaYousafVolue
Copy link

RimshaYousafVolue commented Apr 22, 2024

@CptWesley thanks for responding. I also tried with annotating the field but it didn't work. The specific issue is that if I remove this annotation, the migration will acknowledge it. But if I add the DuplicateField annotation to a field, the migration doesn't see it as a valid migration.
image

Can you please show how you are defining the duplicate fields that they get added in the migration?

@CptWesley
Copy link
Contributor Author

CptWesley commented Apr 22, 2024

@RimshaYousafVolue I personally avoid using the attribute, since it does not seem to work very well. I only use the .Duplicate method on the schema configuration.

@jeremydmiller
Copy link
Member

@RimshaYousafVolue I personally avoid using the attribute, since it does not seem to work very well. I only use the .Duplicate method on the schema configuration.

In what way does the attribute not work? That code hasn't changed in years

@jeremydmiller
Copy link
Member

@CptWesley Weasel is purposely not carrying out any schema changes where there might be lost data. There's very few cases where Weasel will allow you to change field types. I'm happy to take pull requests for whatever behavior you're trying to get out of this here.

@CptWesley
Copy link
Contributor Author

CptWesley commented Apr 23, 2024

@jeremydmiller In this particular instance there can be no data loss since it is adding a duplicated column that wasn't there previously, which weasel seems to be trying. The sql commands that are executed are non-valid. It seems to be trying to copy over the data from the fields in the jsonb column but fails because it lacks the type casting/transformation decoration that is there when new entries are inserted. This results in it working for simple strings and numbers, but not for guids, dates and other custom types.

Using weasel v7 (and marten v7) it did seem to no longer throw exceptions, but I haven't checked if it actually created the column or if it never attempted the upgrade because data was already present. I haven't been able to fully test it due to me spending the last few days sick in bed and because of some issues I am still encountering that are related to JasperFx/marten#3148

@CptWesley
Copy link
Contributor Author

Issue seems indeed to be resolved in weasel 7. So I'm closing this issue

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

No branches or pull requests

3 participants