sqlx
sqlx copied to clipboard
query_as! macros do not use FromRow
instead the query_as! macro's directly build the target struct, while it might be nice not to have to derive FromRow for the struct, this also means that you can't customize the behaviour with a custom FromRow implementation.
We unfortunately have no way to do this right now while keeping the guarantee that output columns and types match the field names and types in the struct. We could possibly do some type-level magic with FromRow to get this information in a way that the macros can use it but leaning too heavily on typechecking produces some really obtuse compiler errors.
I think with the const_if_match and const_panic features we can make this work, although we would also need to be able to loop through a list which isn't currently possible in a const context either, but I think we can work around that.
would also need to be able to loop through a list which isn't currently possible in a
constcontext either
const_loop also got stabilized for 1.46, same as const_if_match. So that shouldn't be a problem :)
Ah, thanks. const_panic is still a necessary feature for this, though, otherwise we can't control the error message. There's been some promising discussion on the tracking issue so I think I could push it through the stabilization process if I can find the time: https://github.com/rust-lang/rust/issues/51999
Could we at least document this currently for now? As It was not clear for me.
Could we at least document this currently for now? As It was not clear for me.
This is already stated in the documentation: https://github.com/launchbadge/sqlx/blob/master/src/macros.rs#L379
This is also stated in the documentation for 0.3.5, it's just missing the bold formatting: https://github.com/launchbadge/sqlx/blob/v0.3.5/src/macros.rs#L247
If that isn't clear enough then we'd love suggestions for better phrasing.
The beta version is okay as it is, but I think it would be better if it directly mentions the FromRow as it's currently vague by which "trait implementation" it refers to, perhaps update FromRow's docs since they indicate that it's required for query_as, and shouldn't it link to this issue as well?
The documentation also goes on to explain, pretty clearly I think, how the macro maps rows to the struct.
However, "no trait implementations are required" can be misleading because Decode is still required for the individual field types of the struct so the wording should be adjusted anyway.
FromRow is indeed required for the function version of query_as which is why it's mentioned in its docs, which also link to the function to make it clear what it's referring to. In general if the docs are referring to a macro they will include the bang (!) in the item name to disambiguate.
This is already stated in the documentation: https://github.com/launchbadge/sqlx/blob/master/src/macros.rs#L379
Can you pin the link to a commit? Otherwise the line number gets outdated.
It would be nice to have a note about this at the top of https://docs.rs/sqlx/0.5.5/sqlx/trait.FromRow.html . That's where I started looking first when using sqlx(rename) didn't work.
It would be nice to have a note about this at the top of docs.rs/sqlx/0.5.5/sqlx/trait.FromRow.html . That's where I started looking first when using
sqlx(rename)didn't work.
I'm sure adding more docs would be appreciated š
Ah, thanks.
const_panicis still a necessary feature for this, though, otherwise we can't control the error message.
Just run into this too - Also: Rust 1.57 has stabilized const_panic
Has any work been done on this? If not Iād be willing to try and help contribute this feature.
im also waiting on this
So, to use nested fields we need FromRow. But to use FromRow we can't use query_as! macro, which dynamically runs queries at compile time, only the query_as func, which doesn't. Is that it?
I came looking for how I could split some db fields into another struct, just trying to make sure I understand the current state of things. I love the compile-time query running, and don't want to lose it...
EDIT: Well, I just tried to use FromRow anyway, only to realize query_as func also doesn't accept arguments!! Humm, I'd have to manually format!() the query and handle SQL injections myself (or use the QueryBuilder which I've just found out about in the docs), and then send its &str to it? No way, it is too much of a compromise. Now I can see why this is important...
Thank you for your work, sqlx is awesome! I can't stand diesel š
that deserves its own issue and there probably already is one
EDIT: Well, I just tried to use FromRow anyway, only to realize query_as func also doesn't accept arguments!! Humm, I'd have to manually format!() the query and handle SQL injections myself (or use the QueryBuilder which I've just found out about in the docs), and then send its &str to it? No way, it is too much of a compromise. Now I can see why this is important...
@rsalmei I think what you are looking for is something like the bind function?
slqx::query_as("SELECT foo FROM bar WHERE name = ? AND age = ?")
.bind("Harrison Ford")
.bind(80);
It seems like the needs rust feature tag should be removed from this issue since the needed features have been stabilised for some now?
It can, but I'm not really happy about how the compiler currently handles const panics: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=33e7700ac709e6581dcf478fb3c3b3b4
I proposed a format that would be more useful to us during the feature's development, but it was brushed off: https://github.com/rust-lang/rust/issues/51999#issuecomment-651022782
We also can't use Type::compatible() since trait methods can't be invoked in const which would necessitate some more codegen hackery, like generating a #[doc(hidden)] pub const fn method in an inherent impl with #[derive(Type)].
There's also the issue of formatting the error messages. We'd like to be able to output something like:
error mapping column `<column name>` to field `<field name>`:
expected Rust type <field type>, got incompatible type <column type>
However, panic!() currently only accepts either a string literal or a trivial format-args invocation: panic!("{}", s) where s is a &str.
There's been some relevant groundwork laid, e.g.: https://github.com/rust-lang/rust/issues/78356
But I cannot find evidence of any movement for allowing non-trivial format args in const panic!(). All the issues I can find are either closed or not relevant.
The const_format crate is interesting but it requires some unsafe hackery to work: https://github.com/rodrimati1992/const_format_crates/blob/9a9e1e8d5f477b30a22ea5eeb857d73f8b72682b/const_format/src/macros/fmt_macros.rs#L101
It's basically the same approach as the const-concat crate which has existed unchanged for 5 years, but with a nice API on top.
I suppose knowing that the former exists and is actively maintained is enough, but it is still rather a dumb reason to reach for unsafe on the face of it.
I have managed to modify const-concat crate to run on stable:
pub const unsafe fn union_transmute<From, To>(from: From) -> To
where
From: Copy,
To: Copy,
{
union Transmute<From: Copy, To: Copy> {
from: From,
to: To,
}
Transmute { from }.to
}
pub const unsafe fn concat<First, Second, Out>(a: &[u8], b: &[u8]) -> Out
where
First: Copy,
Second: Copy,
Out: Copy,
{
#[derive(Copy, Clone)]
struct Both<A, B>(A, B);
let arr: Both<First, Second> = Both(
*union_transmute::<_, &First>(a),
*union_transmute::<_, &Second>(b),
);
union_transmute(arr)
}
#[macro_export]
macro_rules! const_concat {
($a:expr, $b:expr) => {{
let bytes: &'static [u8] = unsafe {
&$crate::concat::<
[u8; $a.len()],
[u8; $b.len()],
[u8; $a.len() + $b.len()],
>($a.as_bytes(), $b.as_bytes())
};
unsafe { $crate::union_transmute::<_, &'static str>(bytes) }
}};
($a:expr, $($rest:expr),*) => {{
const TAIL: &str = const_concat!($($rest),*);
const_concat!($a, TAIL)
}};
($a:expr, $($rest:expr),*,) => {
const_concat!($a, $($rest),*);
};
}
#[cfg(test)]
mod tests {
#[test]
fn top_level_constants() {
const HELLO: &str = "Hello";
const COMMA: &str = ", ";
const WORLD: &str = "world";
const GREETING2: &str = const_concat!(HELLO, WORLD);
const GREETING3: &str = const_concat!(HELLO, COMMA, WORLD);
const GREETING4: &str = const_concat!(HELLO, COMMA, WORLD, "!");
const GREETING6: &str = const_concat!(HELLO, COMMA, WORLD, "!", "1", "2");
// const GREETING_TRAILING_COMMA: &str = const_concat!(HELLO, COMMA, WORLD, "!",);
assert_eq!(GREETING2, "Helloworld");
assert_eq!(GREETING3, "Hello, world");
assert_eq!(GREETING4, "Hello, world!");
assert_eq!(GREETING6, "Hello, world!12");
// assert_eq!(GREETING_TRAILING_COMMA, "Hello, world!");
}
}
But it fails on GREETING4 with:
assertion left == right failed
left: ", world!Hello"
right: "Hello, world!"
And it does not matter if you use const_concat!($a, TAIL) or const_concat!(TAIL, $a) which is very weird.
I've made it just for fun, guess it's not usable.
Without macros (expanded) version:
fn main() {
const HELLO: &str = "Hello";
const COMMA: &str = ", ";
const WORLD: &str = "world";
const TAIL: &'static str = {
const TAIL: &'static str = {
let bytes: &'static [u8] = unsafe {
&crate::concat::<[u8; WORLD.len()], [u8; "!".len()], [u8; WORLD.len() + "!".len()]>(
WORLD.as_bytes(),
"!".as_bytes(),
)
};
unsafe { crate::union_transmute::<_, &'static str>(bytes) }
};
{
let bytes: &'static [u8] = unsafe {
&crate::concat::<[u8; COMMA.len()], [u8; TAIL.len()], [u8; COMMA.len() + TAIL.len()]>(
COMMA.as_bytes(),
TAIL.as_bytes(),
)
};
unsafe { crate::union_transmute::<_, &'static str>(bytes) }
}
};
let bytes: [u8; 13] = unsafe {
crate::concat::<[u8; HELLO.len()], [u8; TAIL.len()], [u8; HELLO.len() + TAIL.len()]>(
HELLO.as_bytes(),
TAIL.as_bytes(),
)
};
println!("{}", String::from_utf8(bytes.to_vec()).unwrap()); // ", world!Hello"
}
Looks like const_format::concatcp! works good.
a dumb reason to reach for unsafe on the face of it.
@abonander As far as I understood, you need to panic!() during compilation, and you are worried about unsafe. Is it something to worry about?
I mean the panic itself is like the final destination)
I guess you are worried about the message being correct?
I've just wrote my own implementation. Is it safe enough? And is it something you was looking for?
So, to use nested fields we need
FromRow. But to useFromRowwe can't usequery_as!macro, which dynamically runs queries at compile time, only thequery_asfunc, which doesn't. Is that it? I came looking for how I could split some db fields into another struct, just trying to make sure I understand the current state of things. I love the compile-time query running, and don't want to lose it...
Please, I am confused by this @rsalmei previous comment. Talking about "nested structs" - I have seen examples of query_as! macro with selects like (column1, column2, column3) as "address!: Address" posted elsewhere, that says this should work.
Well, do I need query_as to map to a nested custom structs or can I do it with query_as! macro easily too, without deriving FromRow at all?