Fluentmigrator: Columns are always NotNullable (as a default)

Created on 30 Mar 2020 · 5Comments · Source: fluentmigrator/fluentmigrator

I have the following migration (one of many, and it is a typical ASP.NET Identity table):

Create.Table("aspnet_users")
    .WithColumn("id").AsGuid().PrimaryKey().NotNullable()
    .WithColumn("username").AsString()
    .WithColumn("normalized_username").AsString()
    .WithColumn("email").AsString()
    .WithColumn("normalized_email").AsString()
    .WithColumn("email_confirmed").AsBoolean().NotNullable()
    .WithColumn("password_hash").AsString()
    .WithColumn("security_stamp").AsString()
    .WithColumn("concurrency_stamp").AsString()
    .WithColumn("phone_number").AsString()
    .WithColumn("phone_number_confirmed").AsBoolean().NotNullable()
    .WithColumn("two_factor_enabled").AsBoolean().NotNullable()
    .WithColumn("lockout_end").AsDateTime()
    .WithColumn("lockout_enabled").AsBoolean().NotNullable()
    .WithColumn("access_failed_count").AsInt32().NotNullable()
    .WithColumn("display_name").AsString();

My assumption was that unless NotNullable is specified the column would default to Nullable. However after the migration has ran, all the columns are NotNullable.

I'm using Postgresql.
Am I doing something wrong?

question question-answered

Source

n1ghtmare

All 5 comments

The FormatNullable override per-generator determines the default behavior. You can create your own Generator for Postgres if you like.

I tested on SqlServer and verified the behavior is exactly as you describe. TBH, I always explicitly specify nullability so this has never tripped me up. I can see rational for allowing null as a default. I can also see rational for making things not-nullable as a default. But probably the missing thing here is the xmldocs IntelliSense for Nullable() and NotNullable() dont mention which is the default. - Happy to accept a PR to fix that.

As you can see below, the various database generators are fairly consistent, aside from some obscure edge cases that I can't even explain off the top of my head.

Postgres

https://github.com/fluentmigrator/fluentmigrator/blob/5b44713448145572c70fcd283feb95ba7bcf0675/src/FluentMigrator.Runner.Postgres/Generators/Postgres/PostgresColumn.cs#L91-L99

Firebird

Uses ColumnBase definition

https://github.com/fluentmigrator/fluentmigrator/blob/e82aafa20e6dbe3cefa221303fe23cf8bf59fffd/src/FluentMigrator.Runner.Core/Generators/Base/ColumnBase.cs#L124-L132

jzabroski on 3 Apr 2020

Thanks for the clarifications. I ended up being explicit about the nullability of each column. My assumption of the defaults was stemming from defining columns by writing SQL, if you don’t specify NOT NULL explicitly the column would be nullable. Also it’s a more intuitive default (IMHO). Anyway I would be happy to help with the IntelliSense docs, this project is amazing, I’ll take a look at the codebase once I get some free time.

n1ghtmare on 3 Apr 2020

Thanks for offering to help, and it's wecome. To be quite honest, just answering questions takes a lot of my and @fubar-coder 's time. I don't think we get enough development time to actually improve things. So picking off some easy tasks would definitely help towards adding features. I feel this will be a good year roadmap wise, as I have finally come up with some decisions around how to support multiple target framework monikers.

jzabroski on 6 Apr 2020

👍1

One thought did occur to me just now. Probably won't ever get done, but:

The FluentMigrator namespace includes the Syntax required to build expressions. On some level, rather than replacing the Postgres generator (which would then cause issues if you later wanted to target a different database), it would make more sense to replace the Syntax entirely and have Nullable be the default value. Related to #1206 and allowing people to set their own Opinions on how things should work. However, I'm not sure it's a good idea. I can only see pros and cons at the moment:

Pros

If people don't like default behavior for "implicit syntax", they can replace it.
Would be generator agnostic and so can easily target multiple database ProcessorId values.

Cons
You would need to look at which syntax they've imported to know what the behavior is - so the code isn't prima facie obvious
Extensibility Problem with supporting database-specific syntax
Test case explosion / code coverage challenges

jzabroski on 6 Apr 2020

It also seems as though SQL Server lets you change the default behavior of implicit syntax, so FluentMigrator should explicitly specify null or not null. See: SET ANSI_NULL_DFLT_ON:

This setting only affects the nullability of new columns when the nullability of the column is not specified in the CREATE TABLE and ALTER TABLE statements. When SET ANSI_NULL_DFLT_ON is ON, new columns created by using the ALTER TABLE and CREATE TABLE statements allow null values if the nullability status of the column is not explicitly specified. SET ANSI_NULL_DFLT_ON does not affect columns created with an explicit NULL or NOT NULL.