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

Serialize QueryResult into serde_json::Value #4

Closed
tyt2y3 opened this issue May 21, 2021 · 18 comments · Fixed by #9
Closed

Serialize QueryResult into serde_json::Value #4

tyt2y3 opened this issue May 21, 2021 · 18 comments · Fixed by #9
Assignees

Comments

@tyt2y3
Copy link
Member

tyt2y3 commented May 21, 2021

  • This feature should be feature guarded
  • Cater for SelectTwo, which becomes Vec<Vec>
@tyt2y3 tyt2y3 mentioned this issue May 21, 2021
36 tasks
@billy1624
Copy link
Member

Proposed use cases

For Select

let select_one: Value = cake::Entity::find()  // Value::Object()
        .one_json(db)
        .await?;

let select_all: Value = cake::Entity::find()  // Value::Array( Vec<Value::Object()> )
        .all_json(db)
        .await?;

For SelectTwo

let tuple_one: Value = cake::Entity::find()  // Value::Array( Vec<Value::Object()> )
        .left_join_and_select(filling::Entity)
        .one_json(db)
        .await?;

let tuple_all: Value = cake::Entity::find()  // Value::Array( Vec<Value::Array( Value::Object() )> )
        .left_join_and_select(filling::Entity)
        .all_json(db)
        .await?;

https://docs.serde.rs/serde_json/value/enum.Value.html

@billy1624 billy1624 self-assigned this May 25, 2021
@tyt2y3
Copy link
Member Author

tyt2y3 commented May 25, 2021

We should not need to add new methods to selects.
Just impl Into<serde_json::Value> for QueryResult

@billy1624
Copy link
Member

From what I understand there is no way user can get the QueryResult directly is that correct?
Then, how can we call .into() on QueryResult

@billy1624
Copy link
Member

To construct the serde_json::Value, we can...

// inside ModelTrait
fn serialize_query_result(row: &QueryResult, pre: &str) -> Result<serde_json::Value, Err>;

This will be implemented inside DeriveModel macro

@tyt2y3
Copy link
Member Author

tyt2y3 commented May 25, 2021

No, we should not serialize via Model

@tyt2y3
Copy link
Member Author

tyt2y3 commented May 25, 2021

We can add an as_json method to Select<E> and transform into SelectJson (just like SelectModel)

@tyt2y3
Copy link
Member Author

tyt2y3 commented May 25, 2021

we should not serialize via Model

@tyt2y3
Copy link
Member Author

tyt2y3 commented May 25, 2021

We should directly convert from QueryResult into serde_json::Value

@billy1624
Copy link
Member

To avoid unnecessary conversion?

@billy1624
Copy link
Member

But still we need to know query result column type and its name.

To support both

  • Normal select which retrieve a Model
  • And custom select which retrieve a non-Model struct

Perhaps we can add a new method inside trait FromQueryResult?

// Serialize a row of query result
fn serialize_query_result(row: &QueryResult, pre: &str) -> Result<serde_json::Value, TypeErr>;

Example custom select

#[derive(Debug, FromQueryResult)]
struct SelectResult {
    name: String,
    num_of_fruits: i32,
}

print!("count fruits by cake: ");

let select = cake::Entity::find()
    .left_join(fruit::Entity)
    .select_only()
    .column(cake::Column::Name)
    .column_as(fruit::Column::Id.count(), "num_of_fruits")
    .group_by(cake::Column::Name);

let results = select.into_model::<SelectResult>().all(db).await?;

@tyt2y3
Copy link
Member Author

tyt2y3 commented May 26, 2021

I reject the above proposal. The column list and info can be queried from the Row trait via SQLx.

@billy1624
Copy link
Member

billy1624 commented May 26, 2021

How about column type? SQLx just tell us the &str column type. Parsing the column type will be very tricky.

@billy1624
Copy link
Member

billy1624 commented May 26, 2021

In our case there are only four possible serde_json::Value.

  • serde_json::Value::Null()
  • serde_json::Value::Bool()
  • serde_json::Value::Number()
  • serde_json::Value::String()
// Rather than getting the &str column type
assert_eq!(d.column(2).type_info().name(), "BOOLEAN");

// We can check for compatibility, starts from null, bool, number then finally string
assert!(<bool as Type<MySql>>::compatible(&d.column(2).type_info()));

Seems above plan will not work as expected...

  • basically, int is compatible with bool, so if we check compatibility starting from bool then int column will be treated as bool
// `int` column is compatible with `bool`
impl Type<MySql> for bool {
    fn type_info() -> MySqlTypeInfo {
        // MySQL has no actual `BOOLEAN` type, the type is an alias of `TINYINT(1)`
        MySqlTypeInfo {
            flags: ColumnFlags::BINARY | ColumnFlags::UNSIGNED,
            char_set: 63,
            max_size: Some(1),
            r#type: ColumnType::Tiny,
        }
    }

    fn compatible(ty: &MySqlTypeInfo) -> bool {
        matches!(
            ty.r#type,
            ColumnType::Tiny
                | ColumnType::Short
                | ColumnType::Long
                | ColumnType::Int24
                | ColumnType::LongLong
                | ColumnType::Bit
        )
    }
}

// `int` type info
fn int_compatible(ty: &MySqlTypeInfo) -> bool {
    matches!(
        ty.r#type,
        ColumnType::Tiny
            | ColumnType::Short
            | ColumnType::Long
            | ColumnType::Int24
            | ColumnType::LongLong
    ) && !ty.flags.contains(ColumnFlags::UNSIGNED)
}

impl Type<MySql> for i8 {
    fn type_info() -> MySqlTypeInfo {
        MySqlTypeInfo::binary(ColumnType::Tiny)
    }

    fn compatible(ty: &MySqlTypeInfo) -> bool {
        int_compatible(ty)
    }
}

@billy1624
Copy link
Member

Wait... I can explicitly check type equality.

#[async_trait]
impl Connection for &SqlxMySqlPoolConnection {
    async fn query_one(&self, stmt: Statement) -> Result<QueryResult, QueryErr> {
        debug_print!("{}", stmt);

        let query = bind_query(sqlx::query(&stmt.sql), &stmt.values);
        if let Ok(conn) = &mut self.pool.acquire().await {
            if let Ok(row) = query.fetch_one(conn).await {
                for col in row.columns() {
                    let col_type = col.type_info();
                    dbg!(col.name());
                    dbg!(<bool as Type<MySql>>::type_info().eq(col_type));
                    dbg!(<i8 as Type<MySql>>::type_info().eq(col_type));
                    dbg!(<i16 as Type<MySql>>::type_info().eq(col_type));
                    dbg!(<i32 as Type<MySql>>::type_info().eq(col_type));
                    dbg!(<i64 as Type<MySql>>::type_info().eq(col_type));
                    dbg!(<u8 as Type<MySql>>::type_info().eq(col_type));
                    dbg!(<u16 as Type<MySql>>::type_info().eq(col_type));
                    dbg!(<u32 as Type<MySql>>::type_info().eq(col_type));
                    dbg!(<u64 as Type<MySql>>::type_info().eq(col_type));
                    dbg!(<f32 as Type<MySql>>::type_info().eq(col_type));
                    dbg!(<f64 as Type<MySql>>::type_info().eq(col_type));
                    dbg!(<String as Type<MySql>>::type_info().eq(col_type));
                }
                return Ok(row.into());
            }
        }
        Err(QueryErr)
    }
    // ...
}

The example sqlx-mysql print...

find one by primary key: SELECT `cake`.`id`, `cake`.`name` FROM `cake` WHERE `cake`.`id` = 1 LIMIT 1
[src/driver/sqlx_mysql.rs:42] col.name() = "id"
[src/driver/sqlx_mysql.rs:43] <bool as Type<MySql>>::type_info().eq(col_type) = false
[src/driver/sqlx_mysql.rs:44] <i8 as Type<MySql>>::type_info().eq(col_type) = false
[src/driver/sqlx_mysql.rs:45] <i16 as Type<MySql>>::type_info().eq(col_type) = false
[src/driver/sqlx_mysql.rs:46] <i32 as Type<MySql>>::type_info().eq(col_type) = true  // is i32
[src/driver/sqlx_mysql.rs:47] <i64 as Type<MySql>>::type_info().eq(col_type) = false
[src/driver/sqlx_mysql.rs:48] <u8 as Type<MySql>>::type_info().eq(col_type) = false
[src/driver/sqlx_mysql.rs:49] <u16 as Type<MySql>>::type_info().eq(col_type) = false
[src/driver/sqlx_mysql.rs:50] <u32 as Type<MySql>>::type_info().eq(col_type) = false
[src/driver/sqlx_mysql.rs:51] <u64 as Type<MySql>>::type_info().eq(col_type) = false
[src/driver/sqlx_mysql.rs:52] <f32 as Type<MySql>>::type_info().eq(col_type) = false
[src/driver/sqlx_mysql.rs:53] <f64 as Type<MySql>>::type_info().eq(col_type) = false
[src/driver/sqlx_mysql.rs:54] <String as Type<MySql>>::type_info().eq(col_type) = false
[src/driver/sqlx_mysql.rs:42] col.name() = "name"
[src/driver/sqlx_mysql.rs:43] <bool as Type<MySql>>::type_info().eq(col_type) = false
[src/driver/sqlx_mysql.rs:44] <i8 as Type<MySql>>::type_info().eq(col_type) = false
[src/driver/sqlx_mysql.rs:45] <i16 as Type<MySql>>::type_info().eq(col_type) = false
[src/driver/sqlx_mysql.rs:46] <i32 as Type<MySql>>::type_info().eq(col_type) = false
[src/driver/sqlx_mysql.rs:47] <i64 as Type<MySql>>::type_info().eq(col_type) = false
[src/driver/sqlx_mysql.rs:48] <u8 as Type<MySql>>::type_info().eq(col_type) = false
[src/driver/sqlx_mysql.rs:49] <u16 as Type<MySql>>::type_info().eq(col_type) = false
[src/driver/sqlx_mysql.rs:50] <u32 as Type<MySql>>::type_info().eq(col_type) = false
[src/driver/sqlx_mysql.rs:51] <u64 as Type<MySql>>::type_info().eq(col_type) = false
[src/driver/sqlx_mysql.rs:52] <f32 as Type<MySql>>::type_info().eq(col_type) = false
[src/driver/sqlx_mysql.rs:53] <f64 as Type<MySql>>::type_info().eq(col_type) = false
[src/driver/sqlx_mysql.rs:54] <String as Type<MySql>>::type_info().eq(col_type) = true  // is String

Model { id: 1, name: "New York Cheese" }

@billy1624
Copy link
Member

To check for null value, simply...

let col_type = col.type_info();
dbg!(col_type.is_null());

@tyt2y3
Copy link
Member Author

tyt2y3 commented May 26, 2021

Sounds like a good plan. But it's better to try distinguish between i64 and f64

@billy1624 billy1624 linked a pull request May 26, 2021 that will close this issue
@tyt2y3 tyt2y3 closed this as completed in #9 May 26, 2021
@tyt2y3 tyt2y3 reopened this May 26, 2021
@tyt2y3
Copy link
Member Author

tyt2y3 commented May 26, 2021

JSON is not a matter of Connector, and deserialization is not a matter of QueryResult.

@tyt2y3 tyt2y3 closed this as completed May 27, 2021
@billy1624
Copy link
Member

Cool refactor!!

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