Provide a better "raw sql" escape hatch for full queries that we don't support in the query builder #650
Labels
Milestone
Comments
|
I'm new to Rust and Diesel. but what I love is type safety and all great things LoadDsl does. That's why I'd prefer "better I'm playing with Diesel and see a lot of limitations (like inability to make multiple joins or multiple belongs_to) and that's ok. Even with raw SQL Diesel is still great. I created couple workarounds to solve issues with raw sql queries. |
sgrif
added a commit
that referenced
this issue
Sep 29, 2017
`sql` has always been a bit clunky to use if you wanted to write a whole query with it. You have to specify the SQL type of the query, which also means you have to list every column out explicitly to ensure they're in the expected order. This adds a replacement API that more geared towards writing complete queries. There are two notable differences. The first is that the SQL type of each field is specified during deserialization, not when the query is constructed. The types are never checked anyway (other than to make sure the deserialization is valid), so it makes sense to defer this to a place where we can derive it. The second difference is that columns are deserialized by name, not by index. This has all of the gotchas that are associated with doing that with other libraries (name conflicts between tables, etc) I had originally planned on having `NamedRow::get` optionally also take a table name. However, PG gives you the OID not the name, so we'd need to do another query to get the table name. SQLite only gives you the table/column name if compiled with a flag to enable it, which the system sqlite3 on mac does not. For those two reasons, I've opted to use the column name alone. This means that the derived `QueryableByName` is unlikely to be usable with joins, but that's a price we'll have to pay for now. I'd like to expand the number of cases that our derive impl can handle (e.g. allow providing a sql type in addition to a column name), but this is a good bare minimum place to start. Fixes #650.
sgrif
added a commit
that referenced
this issue
Sep 29, 2017
`sql` has always been a bit clunky to use if you wanted to write a whole query with it. You have to specify the SQL type of the query, which also means you have to list every column out explicitly to ensure they're in the expected order. This adds a replacement API that more geared towards writing complete queries. There are two notable differences. The first is that the SQL type of each field is specified during deserialization, not when the query is constructed. The types are never checked anyway (other than to make sure the deserialization is valid), so it makes sense to defer this to a place where we can derive it. The second difference is that columns are deserialized by name, not by index. This has all of the gotchas that are associated with doing that with other libraries (name conflicts between tables, etc) I had originally planned on having `NamedRow::get` optionally also take a table name. However, PG gives you the OID not the name, so we'd need to do another query to get the table name. SQLite only gives you the table/column name if compiled with a flag to enable it, which the system sqlite3 on mac does not. For those two reasons, I've opted to use the column name alone. This means that the derived `QueryableByName` is unlikely to be usable with joins, but that's a price we'll have to pay for now. I'd like to expand the number of cases that our derive impl can handle (e.g. allow providing a sql type in addition to a column name), but this is a good bare minimum place to start. Fixes #650.
sgrif
added a commit
that referenced
this issue
Sep 30, 2017
`sql` has always been a bit clunky to use if you wanted to write a whole query with it. You have to specify the SQL type of the query, which also means you have to list every column out explicitly to ensure they're in the expected order. This adds a replacement API that more geared towards writing complete queries. There are two notable differences. The first is that the SQL type of each field is specified during deserialization, not when the query is constructed. The types are never checked anyway (other than to make sure the deserialization is valid), so it makes sense to defer this to a place where we can derive it. The second difference is that columns are deserialized by name, not by index. This has all of the gotchas that are associated with doing that with other libraries (name conflicts between tables, etc) I had originally planned on having `NamedRow::get` optionally also take a table name. However, PG gives you the OID not the name, so we'd need to do another query to get the table name. SQLite only gives you the table/column name if compiled with a flag to enable it, which the system sqlite3 on mac does not. For those two reasons, I've opted to use the column name alone. This means that the derived `QueryableByName` is unlikely to be usable with joins, but that's a price we'll have to pay for now. I'd like to expand the number of cases that our derive impl can handle (e.g. allow providing a sql type in addition to a column name), but this is a good bare minimum place to start. Fixes #650.
sgrif
added a commit
that referenced
this issue
Sep 30, 2017
`sql` has always been a bit clunky to use if you wanted to write a whole query with it. You have to specify the SQL type of the query, which also means you have to list every column out explicitly to ensure they're in the expected order. This adds a replacement API that more geared towards writing complete queries. There are two notable differences. The first is that the SQL type of each field is specified during deserialization, not when the query is constructed. The types are never checked anyway (other than to make sure the deserialization is valid), so it makes sense to defer this to a place where we can derive it. The second difference is that columns are deserialized by name, not by index. This has all of the gotchas that are associated with doing that with other libraries (name conflicts between tables, etc) I had originally planned on having `NamedRow::get` optionally also take a table name. However, PG gives you the OID not the name, so we'd need to do another query to get the table name. SQLite only gives you the table/column name if compiled with a flag to enable it, which the system sqlite3 on mac does not. For those two reasons, I've opted to use the column name alone. This means that the derived `QueryableByName` is unlikely to be usable with joins, but that's a price we'll have to pay for now. I'd like to expand the number of cases that our derive impl can handle (e.g. allow providing a sql type in addition to a column name), but this is a good bare minimum place to start. Fixes #650.
sgrif
added a commit
that referenced
this issue
Oct 16, 2017
`sql` has always been a bit clunky to use if you wanted to write a whole query with it. You have to specify the SQL type of the query, which also means you have to list every column out explicitly to ensure they're in the expected order. This adds a replacement API that more geared towards writing complete queries. There are two notable differences. The first is that the SQL type of each field is specified during deserialization, not when the query is constructed. The types are never checked anyway (other than to make sure the deserialization is valid), so it makes sense to defer this to a place where we can derive it. The second difference is that columns are deserialized by name, not by index. This has all of the gotchas that are associated with doing that with other libraries (name conflicts between tables, etc) I had originally planned on having `NamedRow::get` optionally also take a table name. However, PG gives you the OID not the name, so we'd need to do another query to get the table name. SQLite only gives you the table/column name if compiled with a flag to enable it, which the system sqlite3 on mac does not. For those two reasons, I've opted to use the column name alone. This means that the derived `QueryableByName` is unlikely to be usable with joins, but that's a price we'll have to pay for now. I'd like to expand the number of cases that our derive impl can handle (e.g. allow providing a sql type in addition to a column name), but this is a good bare minimum place to start. Fixes #650.
sgrif
added a commit
that referenced
this issue
Oct 16, 2017
`sql` has always been a bit clunky to use if you wanted to write a whole query with it. You have to specify the SQL type of the query, which also means you have to list every column out explicitly to ensure they're in the expected order. This adds a replacement API that more geared towards writing complete queries. There are two notable differences. The first is that the SQL type of each field is specified during deserialization, not when the query is constructed. The types are never checked anyway (other than to make sure the deserialization is valid), so it makes sense to defer this to a place where we can derive it. The second difference is that columns are deserialized by name, not by index. This has all of the gotchas that are associated with doing that with other libraries (name conflicts between tables, etc) I had originally planned on having `NamedRow::get` optionally also take a table name. However, PG gives you the OID not the name, so we'd need to do another query to get the table name. SQLite only gives you the table/column name if compiled with a flag to enable it, which the system sqlite3 on mac does not. For those two reasons, I've opted to use the column name alone. This means that the derived `QueryableByName` is unlikely to be usable with joins, but that's a price we'll have to pay for now. I'd like to expand the number of cases that our derive impl can handle (e.g. allow providing a sql type in addition to a column name), but this is a good bare minimum place to start. Fixes #650.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Today we have some form of an escape hatch in the form of the
sqlfunction. However, it's meant for use with small fragments, not full queries. It's limited because it requires specifying the full SQL type of the output, and cannot handle bind parameters. I'd like to introduce a new function that is a more ergonomic API for raw SQL as an escape hatch for queries we don't support in the query builder yet.Ultimately I've resisted introducing this, as we want to support all possible safe queries in the query builder eventually, but I think we need a proper escape hatch for us to go 1.0.
I'd like this API to be fully untyped, and named to make it clear that no verification of the query is occurring. For deserialization, it should not go through
Queryable, as I do not think enforcing column order is ergonomic when writing raw SQL (andQueryableneeds a SQL type to work). Instead I'd like to introduce anUntypedQueryable, which takes a row directly, and is able to fetch fields by name. I'm fine with the API requiring specifying the SQL type on this call if type inference can't pick it up. e.g.let updated_at: NaiveDateTime = row.get::<Timestamp>("updated_at")?. I suspect for most types which only implementFromSqlfor a single DB type type inference will handle it, but we should explore our options here and see what feels best.I'm less clear what we should do with bind parameters. I do know that I don't want to go the route that the postgres crate went where we take
&[&UntypedToSql], as constructing that value is painful, it forces dynamic dispatch (which then usually also forces boxing), and requires&[]for queries with no bind parameters. We could potentially still pass tuples here as long as type inference does what we want. I also thinkconn.raw_query("SELECT * FROM users WHERE id = $1").bind(id)could work (as an aside, I think we will likely need to pass the connection in before the bind parameters for this to work, which will lead to a semi-weird api ofconn.raw_query().bind().execute()). I'm fine with also sometimes requiring giving a SQL type for the bind here as long as type inference is able to handle the most common cases.The text was updated successfully, but these errors were encountered: