Skip to content

Commit

Permalink
feat: Use IDENTITY column for int primary keys instead of BIGSERIAL (#…
Browse files Browse the repository at this point in the history
…293)

`BIGSERIAL` is not recommended. See:
https://wiki.postgresql.org/wiki/Don%27t_Do_This#Don.27t_use_serial. The
recommendation is instead to use `IDENTITY` columns.

This PR updates the create `user` table migration (just the one using
bigint as the primary key) to create the pk as an IDENTITY column
instead of a BIGSERIAL. This PR also adds the `pk_bigint_identity`
method to enable creating an IDENTITY pk in any table. The
`pk_bigint_identity_options` method is added as well, which is the same
except it allows configuring the sequence for the identity generation.
See: https://www.postgresql.org/docs/current/sql-createsequence.html

Closes #290
  • Loading branch information
spencewenski authored Jul 25, 2024
1 parent b9fcac4 commit f0d8726
Show file tree
Hide file tree
Showing 9 changed files with 141 additions and 5 deletions.
108 changes: 107 additions & 1 deletion src/migration/schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
//! timezone, while SeaORM/Loco do not.
use sea_orm_migration::{prelude::*, schema::*};
use typed_builder::TypedBuilder;

/// Timestamp related fields.
#[derive(DeriveIden)]
Expand Down Expand Up @@ -46,13 +47,78 @@ pub fn timestamps(mut table: TableCreateStatement) -> TableCreateStatement {
.to_owned()
}

/// Create a `BIGINT` primary key column with no default -- the application would need to provide
/// the value. Not exposed publicly; this should only be used internally as a utility method.
fn pk_bigint<T>(name: T) -> ColumnDef
where
T: IntoIden,
{
big_integer(name).primary_key().to_owned()
}

/// Create an auto-incrementing primary key column using [BigInteger][sea_orm::sea_query::ColumnType::BigInteger]
/// as the column type.
#[deprecated(
since = "0.5.10",
note = "This creates a `BIGSERIAL` column, which is not recommended. Use `pk_bigint_identity` instead to create an IDENTITY column, as recommended [here](https://wiki.postgresql.org/wiki/Don%27t_Do_This#Don.27t_use_serial)."
)]
pub fn pk_bigint_auto<T>(name: T) -> ColumnDef
where
T: IntoIden,
{
big_integer(name).primary_key().auto_increment().to_owned()
pk_bigint(name).auto_increment().to_owned()
}

/// Create an auto-incrementing primary key column using [BigInteger][sea_orm::sea_query::ColumnType::BigInteger]
/// as the column type. This creates an `IDENTITY` column instead of a `BIGSERIAL` as recommended
/// [here](https://wiki.postgresql.org/wiki/Don%27t_Do_This#Don.27t_use_serial).
///
/// See also: <https://www.postgresql.org/docs/17/ddl-identity-columns.html>
pub fn pk_bigint_identity<T>(name: T) -> ColumnDef
where
T: IntoIden,
{
pk_bigint(name)
.extra("GENERATED ALWAYS AS IDENTITY")
.to_owned()
}

/// Configuration options for creating an `IDENTITY` column.
#[derive(TypedBuilder)]
pub struct IdentityOptions {
/// If `true`, will add `ALWAYS` to the column definition, which will prevent the application
/// from providing a value for the column. If the application needs to be able to set the
/// value of the column, set to `false`.
///
/// See: <https://www.postgresql.org/docs/17/ddl-identity-columns.html>
#[builder(default = true)]
always: bool,

/// The value for the `START WITH` option of the `IDENTITY` sequence.
///
/// See: <https://www.postgresql.org/docs/current/sql-createsequence.html>
#[builder(default = 1)]
start: i64,

/// The value for the `INCREMENT BY` option of the `IDENTITY` sequence.
///
/// See: <https://www.postgresql.org/docs/current/sql-createsequence.html>
#[builder(default = 1)]
increment: i64,
}

/// Same as [pk_bigint_identity], except allows configuring the IDENTITY column with the
/// given [IdentityOptions].
pub fn pk_bigint_identity_options<T>(name: T, options: IdentityOptions) -> ColumnDef
where
T: IntoIden,
{
let always = if options.always { " ALWAYS" } else { "" };
let generated = format!(
"GENERATED{} AS IDENTITY (START WITH {} INCREMENT BY {})",
always, options.start, options.increment
);
pk_bigint(name).extra(generated).to_owned()
}

/// Create a primary key column using [Uuid][sea_orm::sea_query::ColumnType::Uuid] as the column
Expand Down Expand Up @@ -141,14 +207,54 @@ mod tests {
assert_snapshot!(table_stmt.to_string(PostgresQueryBuilder));
}

#[rstest]
#[cfg_attr(coverage_nightly, coverage(off))]
fn pk_bigint(_case: TestCase, mut table_stmt: TableCreateStatement) {
table_stmt.col(super::pk_bigint(Foo::Bar));

assert_snapshot!(table_stmt.to_string(PostgresQueryBuilder));
}

#[rstest]
#[cfg_attr(coverage_nightly, coverage(off))]
fn pk_bigint_auto(_case: TestCase, mut table_stmt: TableCreateStatement) {
#[allow(deprecated)]
table_stmt.col(super::pk_bigint_auto(Foo::Bar));

assert_snapshot!(table_stmt.to_string(PostgresQueryBuilder));
}

#[rstest]
#[cfg_attr(coverage_nightly, coverage(off))]
fn pk_bigint_identity(_case: TestCase, mut table_stmt: TableCreateStatement) {
table_stmt.col(super::pk_bigint_identity(Foo::Bar));

assert_snapshot!(table_stmt.to_string(PostgresQueryBuilder));
}

#[rstest]
#[case(true, 1, 1)]
#[case(false, 1, 1)]
#[case(true, -100, 1)]
#[case(true, 0, 1)]
#[cfg_attr(coverage_nightly, coverage(off))]
fn pk_bigint_identity_options(
_case: TestCase,
mut table_stmt: TableCreateStatement,
#[case] always: bool,
#[case] start: i64,
#[case] increment: i64,
) {
let options = IdentityOptions::builder()
.always(always)
.start(start)
.increment(increment)
.build();
table_stmt.col(super::pk_bigint_identity_options(Foo::Bar, options));

assert_snapshot!(table_stmt.to_string(PostgresQueryBuilder));
}

#[rstest]
#[cfg_attr(coverage_nightly, coverage(off))]
fn pk_uuid(_case: TestCase, mut table_stmt: TableCreateStatement) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
source: src/migration/schema.rs
expression: table_stmt.to_string(PostgresQueryBuilder)
---
CREATE TABLE "foo" ( "bar" bigint NOT NULL PRIMARY KEY )
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
source: src/migration/schema.rs
expression: table_stmt.to_string(PostgresQueryBuilder)
---
CREATE TABLE "foo" ( "bar" bigint NOT NULL PRIMARY KEY GENERATED ALWAYS AS IDENTITY )
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
source: src/migration/schema.rs
expression: table_stmt.to_string(PostgresQueryBuilder)
---
CREATE TABLE "foo" ( "bar" bigint NOT NULL PRIMARY KEY GENERATED ALWAYS AS IDENTITY (START WITH 1 INCREMENT BY 1) )
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
source: src/migration/schema.rs
expression: table_stmt.to_string(PostgresQueryBuilder)
---
CREATE TABLE "foo" ( "bar" bigint NOT NULL PRIMARY KEY GENERATED AS IDENTITY (START WITH 1 INCREMENT BY 1) )
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
source: src/migration/schema.rs
expression: table_stmt.to_string(PostgresQueryBuilder)
---
CREATE TABLE "foo" ( "bar" bigint NOT NULL PRIMARY KEY GENERATED ALWAYS AS IDENTITY (START WITH -100 INCREMENT BY 1) )
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
source: src/migration/schema.rs
expression: table_stmt.to_string(PostgresQueryBuilder)
---
CREATE TABLE "foo" ( "bar" bigint NOT NULL PRIMARY KEY GENERATED ALWAYS AS IDENTITY (START WITH 0 INCREMENT BY 1) )
4 changes: 2 additions & 2 deletions src/migration/user/create_and_drop_table.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::migration::check::str_not_empty;
use crate::migration::schema::{pk_bigint_auto, pk_uuid, table};
use crate::migration::schema::{pk_bigint_identity, pk_uuid, table};
use crate::migration::user::User;
use sea_orm_migration::{prelude::*, schema::*};

Expand All @@ -8,7 +8,7 @@ pub(crate) fn create_table_uuid_pk() -> TableCreateStatement {
}

pub(crate) fn create_table_int_pk() -> TableCreateStatement {
create_table(pk_bigint_auto(User::Id))
create_table(pk_bigint_identity(User::Id))
}

pub(crate) fn create_table(pk_col: ColumnDef) -> TableCreateStatement {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
---
source: src/migration/user/create_table.rs
source: src/migration/user/create_and_drop_table.rs
expression: query.to_string(PostgresQueryBuilder)
---
CREATE TABLE IF NOT EXISTS "user" ( "created_at" timestamp with time zone NOT NULL DEFAULT CURRENT_TIMESTAMP, "updated_at" timestamp with time zone NOT NULL DEFAULT CURRENT_TIMESTAMP, "id" bigserial NOT NULL PRIMARY KEY, "name" varchar NOT NULL CHECK (CHAR_LENGTH("name") > 0), "username" varchar NOT NULL UNIQUE CHECK (CHAR_LENGTH("username") > 0), "email" varchar NOT NULL UNIQUE CHECK (CHAR_LENGTH("email") > 0), "password" varchar NOT NULL )
CREATE TABLE IF NOT EXISTS "user" ( "created_at" timestamp with time zone NOT NULL DEFAULT CURRENT_TIMESTAMP, "updated_at" timestamp with time zone NOT NULL DEFAULT CURRENT_TIMESTAMP, "id" bigint NOT NULL PRIMARY KEY GENERATED ALWAYS AS IDENTITY, "name" varchar NOT NULL CHECK (CHAR_LENGTH("name") > 0), "username" varchar NOT NULL UNIQUE CHECK (CHAR_LENGTH("username") > 0), "email" varchar NOT NULL UNIQUE CHECK (CHAR_LENGTH("email") > 0), "password" varchar NOT NULL )

0 comments on commit f0d8726

Please sign in to comment.