#[derive(Serialize)] for Record

[rich-murphey]

It might be convenient for some who use actix-web, to derive serde::Serialize for a Record created by the query!() macro.

The idea here is that one could write an actix-web method such as:

#[post("/test")]
pub async fn test(pool: web::Data<PgPool>) -> HttpResponse {
    HttpResponse::Ok().content_type("application/json").json(
        sqlx::query!("SELECT * FROM widgets",)
            .fetch_all(pool.as_ref())
            .await
            .unwrap(),
    )
}

As it stands, this PR will fail CI tests because use of serde::Serialize is not behind a feature flag such as "json". I just could not find a way to make the feature flag work. Please feel free to make any changes to this that you see fit!

[mehcode]

The json feature needs to be tested outside of quote! so it can look for the feature in the sqlx-macros crate and not whatever crate invoked the derive.

[mehcode]

Additionally, you'll want to do serde::Serialize so the user doesn't need to have the derive imported.

[jplatte]

Nit: Should re-export serde in a #[doc(hidden)] exports module and use it as ::sqlx::export::serde::Deserialize so the user doesn't need to depend on serde directly either.

[rich-murphey]

when enabled, this breaks any query containing a column that is not Serialize-able, such as:

sqlx::query!("select * from (select 1 as id: MyInt) records")

So enabling it with an existing feature such as "json" would cause issues. I'm not sure this deserves a separate feature flag. Need some direction here.

An 'export' module seems like a good idea. I don't see one as it stands, and it sounds easy enough to add.

[rich-murphey]

With these changes, Record implements Serialize when the "serialize" feature flag is asserted. I added a unit test that confirms the json text of a query result.

[rich-murphey]

Here's an example use case for this feature:

#[post("/tracks")]
pub async fn tracks(pool: web::Data<Pool<Db>>) -> Result<HttpResponse> {
    Ok(HttpResponse::Ok().json(
        &sqlx::query!(" SELECT * FROM tracks ;")
            .fetch_all(pool.as_ref())
            .await
            .map_err(error::ErrorInternalServerError)?,
    ))
}

[rich-murphey]

Here's an example actix-web streaming response:

#[get("/tracks/{limit}/{offset}")]
pub async fn tracks(path: web::Path<(i64, i64)>, pool: web::Data<Pool<Db>>) -> HttpResponse {
    let (limit, offset) = path.into_inner();
    sqlx_actix_streaming::query_json!(
        "select * from tracks limit ?1 offset ?2",
        pool.as_ref().clone(),
        limit,
        offset
    )
}

The query_json!() macro is defined here:
https://github.com/rich-murphey/sqlx-actix-streaming/blob/77e518a920d40a7d0647b481554c9b1c84cafd2c/src/macros.rs#L29

[abonander]

I don't really like the idea of a Cargo feature enabling backwards-incompatible codegen. That sounds like adding to the nightmare we already have with the runtime features.

I think this could be worked into our proposal on configuring the macros in user code: #916

[abonander]

This isn't a sufficient test to ensure that the codegen works for external users. Maybe add a small project in examples/postgres/ to demonstrate (which will be covered whenever we get around to turning on CI for examples).

[rich-murphey]

OK, I'll do the equivalent of examples/postgres/json unless you have more preferences.

[rich-murphey]

Hey @abonander, thanks for clarifying the concern that the feature flag would cause compile time errors for users' existing code. And also thanks for the pointer to #916.

We might consider using a visually similar syntax for specifying the derives, such as:

sqlx::query!("SELECT * FROM foo", derive(Serialize))

Given that the feature flag appears to be unworkable, I'm ready to close this PR in favor of a configurable query!() macro.

[jplatte]

That old conflict with a call to a function named derive, though not sure whether that's a problem 😄

[rich-murphey]

Good point. Even though it could be disambiguated by matching derive(($id:ident),*), yea, that would still break any code that might pass derive(a,b,c...) as a bound SQL parameter.

[rich-murphey]

@abonander suggested considering this in #916 - a proposal on configuring the macros in user code.

I'll go ahead and close this in a few days unless someone has more to add.