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

Determine SQLite type by following affinity rule #102

Open
RipleyTom opened this issue May 31, 2022 · 9 comments
Open

Determine SQLite type by following affinity rule #102

RipleyTom opened this issue May 31, 2022 · 9 comments

Comments

@RipleyTom
Copy link

RipleyTom commented May 31, 2022

I'm currently in the process of migrating a program from sqlite to sea-orm and used sea-orm-codegen to generate the entities from the existing database.

It seems it converted an UNSIGNED SMALLINT NOT NULL to a Vec<u8>

The database table used to be created like this:

		conn.execute(
            "CREATE TABLE IF NOT EXISTS users ( userId INTEGER PRIMARY KEY NOT NULL, username TEXT NOT NULL COLLATE NOCASE, hash BLOB NOT NULL, salt BLOB NOT NULL, online_name TEXT NOT NULL, avatar_url TEXT NOT NULL, email TEXT NOT NULL, email_check TEXT NOT NULL, token TEXT NOT NULL, rst_token TEXT, flags UNSIGNED SMALLINT NOT NULL)",
            [],
		)
		.expect("Failed to create users table!");

resulting users.rs:

//! SeaORM Entity. Generated by sea-orm-codegen 0.8.0

use sea_orm::entity::prelude::*;

#[derive(Clone, Debug, PartialEq, DeriveEntityModel)]
#[sea_orm(table_name = "users")]
pub struct Model {
	#[sea_orm(column_name = "userId", primary_key, auto_increment = false)]
	pub user_id: i32,
	pub username: String,
	pub hash: Vec<u8>,
	pub salt: Vec<u8>,
	pub online_name: String,
	pub avatar_url: String,
	pub email: String,
	pub email_check: String,
	pub token: String,
	pub rst_token: Option<String>,
	pub flags: Vec<u8>,
}

#[derive(Copy, Clone, Debug, EnumIter, DeriveRelation)]
pub enum Relation {
	#[sea_orm(has_many = "super::dates::Entity")]
	Dates,
}

impl Related<super::dates::Entity> for Entity {
	fn to() -> RelationDef {
		Relation::Dates.def()
	}
}

impl ActiveModelBehavior for ActiveModel {}

I'd expect pub flags: u16

@billy1624
Copy link
Member

Hey @RipleyTom, thanks for the questions! From what I understand SQLite does not support unsigned integer. It will be treated as a normal integer column in SQLite.
https://www.sqlite.org/datatype3.html#affinity_name_examples

@RipleyTom
Copy link
Author

RipleyTom commented Jun 1, 2022

I'm aware of this, every integer type in sqlite is really an INTEGER behind the hood(note that it's still a valid type in sqlite) but that still doesn't explained how a UNSIGNED SMALLINT(ie an INTEGER really) ended up as Vec<u8> in codegen(at worst it should be an i32).

@billy1624
Copy link
Member

It end up being Vec<u8> because UNSIGNED SMALLINT isn't included in the list:

  • let type_result = match split_type[0] {
    "INT" => Type::Int,
    "INTEGER" => Type::Integer,
    "TINY INT" => Type::TinyInt,
    "SMALL INT" => Type::SmallInt,
    "MEDIUM INT" => Type::MediumInt,
    "BIG INT" => Type::BigInt,
    "UNSIGNED INT" => Type::UnsignedBigInt,
    "INT2" => Type::Int2,
    "INT8" => Type::Int8,
    "TEXT" => Type::Text,
    "CLOB" => Type::Clob,
    "BLOB" => Type::Blob,
    "REAL" => Type::Real,
    "DOUBLE" => Type::Double,
    "DOUBLE PRECISION" => Type::DoublePrecision,
    "FLOAT" => Type::Float,
    "NUMERIC" => Type::Numeric,
    "DECIMAL" => {
    let decimals = split_type[1].chars().collect::<Vec<_>>();
    let integral = decimals[0].to_string().parse::<u8>()?;
    let fractional = decimals[2].to_string().parse::<u8>()?;
    Type::Decimal {
    integral,
    fractional,
    }
    }
    "BOOLEAN" => Type::Boolean,
    "DATE" => Type::Date,
    "DATETIME" => Type::DateTime,
    "TIMESTAMP" => Type::Timestamp,
    _ => Type::variable_types(&split_type)?,
    };

@RipleyTom
Copy link
Author

RipleyTom commented Jun 1, 2022

Hmm I'm even more confused now, why is there an UnsignedBigInt, Int2, Int8, etc types in there?
Those are not sqlite types since there is only INTEGER.

@billy1624
Copy link
Member

Hmm I'm even more confused now, why is there an UnsignedBigInt, Int2, Int8, etc types in there? Those are not sqlite types since there is only INTEGER.

All these types are on the list because they are mentioned on the page https://www.sqlite.org/datatype3.html#affinity_name_examples.

@billy1624
Copy link
Member

I don't think there is a good way to parse arbitrary types, e.g. UNSIGNED SMALLINT, in SQLite. I would suggest you to define it as INTEGER datatype

@RipleyTom
Copy link
Author

Yeah but like the table says, those are examples: This table shows only a small subset of the datatype names that SQLite will accept..
The actual types are, anything with INT in it => INTEGER(which in sqlite type is a variable integer type from 0 to 8 bytes).
https://www.sqlite.org/datatype3.html#storage_classes_and_datatypes actually describes the only 5 actual types sqlite handles internally.
The way it is done atm seems very inconsistent to me.

@billy1624
Copy link
Member

Okay, I agree we can update the logic and search if INT exists in the datatype string: it's INTEGER column if we found INT in it.

@tyt2y3 tyt2y3 transferred this issue from SeaQL/sea-orm Feb 21, 2023
@tyt2y3 tyt2y3 changed the title sea-orm-codegen wrong generation from sqlite db(UNSIGNED SMALLINT NOT NULL => Vec<u8>) Determine SQLite type following affinity rule Feb 21, 2023
@tyt2y3 tyt2y3 changed the title Determine SQLite type following affinity rule Determine SQLite type by following affinity rule Feb 21, 2023
@tyt2y3
Copy link
Member

tyt2y3 commented Feb 21, 2023

Summary: right now the database to type mapping is word-by-word.
Ideally we should follow SQLite's rules in resolving types.
This is a big task, because the implementation should have plenty of test cases verified against SQLite.
Contribution is welcomed.

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

No branches or pull requests

3 participants