Issues with codegen for PG text[]

[anp]

When trying to set up insertable_into a table which has a text[] datatype in postgres:

$ multirust run nightly cargo build --verbose
multirust: checking metadata version
multirust: got metadata version 2
       Fresh libc v0.2.10
       Fresh byteorder v0.3.13
       Fresh pq-sys v0.2.1
       Fresh diesel v0.6.1
       Fresh diesel_codegen v0.6.1
   Compiling diesel_repro v0.1.0 (file:///home/adam/rust-projects/diesel_repro)
     Running `rustc src/main.rs --crate-name diesel_repro --crate-type bin -g --out-dir /home/adam/rust-projects/diesel_repro/target/debug --emit=dep-info,link -L dependency=/home/adam/rust-projects/diesel_repro/target/debug -L dependency=/home/adam/rust-projects/diesel_repro/target/debug/deps --extern diesel_codegen=/home/adam/rust-projects/diesel_repro/target/debug/deps/libdiesel_codegen-f1388be2227ebc98.so --extern diesel=/home/adam/rust-projects/diesel_repro/target/debug/deps/libdiesel-85b76bcc9835dd7c.rlib -L native=/usr/lib`
src/main.rs:9:1: 9:28 error: the trait bound `std::vec::Vec<std::string::String>: diesel::Expression` is not satisfied [E0277]
src/main.rs:9 #[insertable_into(testing)]
              ^~~~~~~~~~~~~~~~~~~~~~~~~~~
src/main.rs:9:1: 9:28 note: in this expansion of #[insertable_into] (defined in src/main.rs)
src/main.rs:9:1: 9:28 help: run `rustc --explain E0277` to see a detailed explanation
src/main.rs:9:1: 9:28 note: required by `diesel::expression::AsExpression::as_expression`
src/main.rs:9:1: 9:28 error: mismatched types:
 expected `diesel::expression::bound::Bound<diesel::types::Nullable<diesel::types::Array<diesel::types::Text>>, &std::vec::Vec<std::string::String>>`,
    found `&std::vec::Vec<std::string::String>`
(expected struct `diesel::expression::bound::Bound`,
    found &-ptr) [E0308]
src/main.rs:9 #[insertable_into(testing)]
              ^~~~~~~~~~~~~~~~~~~~~~~~~~~
src/main.rs:9:1: 9:28 note: in this expansion of #[insertable_into] (defined in src/main.rs)
src/main.rs:9:1: 9:28 help: run `rustc --explain E0308` to see a detailed explanation
error: aborting due to 2 previous errors
error: Could not compile `diesel_repro`.

Caused by:
  Process didn't exit successfully: `rustc src/main.rs --crate-name diesel_repro --crate-type bin -g --out-dir /home/adam/rust-projects/diesel_repro/target/debug --emit=dep-info,link -L dependency=/home/adam/rust-projects/diesel_repro/target/debug -L dependency=/home/adam/rust-projects/diesel_repro/target/debug/deps --extern diesel_codegen=/home/adam/rust-projects/diesel_repro/target/debug/deps/libdiesel_codegen-f1388be2227ebc98.so --extern diesel=/home/adam/rust-projects/diesel_repro/target/debug/deps/libdiesel-85b76bcc9835dd7c.rlib -L native=/usr/lib` (exit code: 101)

Rust version:

$ multirust run nightly rustc --version
rustc 1.10.0-nightly (c2aaad4e2 2016-04-19)

Cargo.toml:

[package]
name = "diesel_repro"
version = "0.1.0"
authors = ["Adam Perry <adam.n.perry@gmail.com>"]

[dependencies.diesel]
version = "0.6.1"
default-features = false
features = ["postgres"]

[dependencies.diesel_codegen]
version = "0.6.1"
default-features = false
features = ["nightly", "postgres"]

up.sql:

CREATE TABLE testing (
  id serial primary key,
  arr text[]
)

main.rs:

#![feature(custom_attribute, custom_derive, plugin)]
#![plugin(diesel_codegen)]

#[macro_use]
extern crate diesel;

infer_schema!("postgres://localhost/testing");

#[insertable_into(testing)]
pub struct Testing {
    id: i32,
    arr: Vec<String>,
}

fn main() {
    println!("Hello, world!");
}

[anp]

Also, this seems related but I'm not sure. When I change the line with #[insertable_into(testing)] in the above main.rs to this:

use diesel::{ExpressionMethods, FilterDsl, LoadDsl, Queryable, SaveChangesDsl, Table};
#[changeset_for(testing)]

I get a different super fun error:

$ multirust run nightly cargo build
   Compiling diesel_repro v0.1.0 (file:///home/adam/rust-projects/diesel_repro)
src/main.rs:11:26: 11:26 error: the trait bound `std::vec::Vec<std::string::String>: diesel::Expression` is not satisfied [E0277]
src/main.rs:11 #[changeset_for(testing)]
                                        ^
src/main.rs:11:1: 11:26 note: in this expansion of #[changeset_for] (defined in src/main.rs)
src/main.rs:11:26: 11:26 help: run `rustc --explain E0277` to see a detailed explanation
src/main.rs:1:1: 11:26 error: mismatched types:
 expected `diesel::expression::predicates::Eq<testing::columns::arr, diesel::expression::bound::Bound<diesel::types::Nullable<diesel::types::Array<diesel::types::Text>>, &std::vec::Vec<std::string::String>>>`,
    found `diesel::expression::predicates::Eq<testing::columns::arr, &std::vec::Vec<std::string::String>>`
(expected struct `diesel::expression::bound::Bound`,
    found &-ptr) [E0308]
src/main.rs: 1 #![feature(custom_attribute, custom_derive, plugin)]
src/main.rs: 2 #![plugin(diesel_codegen)]
src/main.rs: 3 
src/main.rs: 4 #[macro_use]
src/main.rs: 5 extern crate diesel;
src/main.rs: 6 
               ...
src/main.rs:11:1: 11:26 note: in this expansion of #[changeset_for] (defined in src/main.rs)
src/main.rs:1:1: 11:26 help: run `rustc --explain E0308` to see a detailed explanation
src/main.rs:11:26: 11:26 error: no method named `get_result` found for type `diesel::query_builder::update_statement::UpdateStatement<diesel::query_source::filter::FilteredQuerySource<testing::table, diesel::expression::predicates::Eq<testing::columns::id, diesel::expression::bound::Bound<diesel::types::Integer, &i32>>>, (diesel::expression::predicates::Eq<testing::columns::arr, diesel::expression::bound::Bound<diesel::types::Nullable<diesel::types::Array<diesel::types::Text>>, &std::vec::Vec<std::string::String>>>,)>` in the current scope
src/main.rs:11 #[changeset_for(testing)]
                                        ^
src/main.rs:11:1: 11:26 note: in this expansion of #[changeset_for] (defined in src/main.rs)
src/main.rs:11:26: 11:26 note: the method `get_result` exists but the following trait bounds were not satisfied: `diesel::query_builder::update_statement::UpdateQuery<(testing::columns::id, testing::columns::arr), diesel::query_builder::update_statement::UpdateStatement<diesel::query_source::filter::FilteredQuerySource<testing::table, diesel::expression::predicates::Eq<testing::columns::id, diesel::expression::bound::Bound<diesel::types::Integer, &i32>>>, (diesel::expression::predicates::Eq<testing::columns::arr, diesel::expression::bound::Bound<diesel::types::Nullable<diesel::types::Array<diesel::types::Text>>, &std::vec::Vec<std::string::String>>>,)>> : diesel::query_builder::QueryFragment<_>`, `&diesel::query_builder::update_statement::UpdateStatement<diesel::query_source::filter::FilteredQuerySource<testing::table, diesel::expression::predicates::Eq<testing::columns::id, diesel::expression::bound::Bound<diesel::types::Integer, &i32>>>, (diesel::expression::predicates::Eq<testing::columns::arr, diesel::expression::bound::Bound<diesel::types::Nullable<diesel::types::Array<diesel::types::Text>>, &std::vec::Vec<std::string::String>>>,)> : diesel::query_builder::AsQuery`, `_ : diesel::query_builder::QueryFragment<_>`, `&diesel::query_builder::update_statement::UpdateStatement<diesel::query_source::filter::FilteredQuerySource<testing::table, diesel::expression::predicates::Eq<testing::columns::id, diesel::expression::bound::Bound<diesel::types::Integer, &i32>>>, (diesel::expression::predicates::Eq<testing::columns::arr, diesel::expression::bound::Bound<diesel::types::Nullable<diesel::types::Array<diesel::types::Text>>, &std::vec::Vec<std::string::String>>>,)> : diesel::query_builder::Query`, `&diesel::query_builder::update_statement::UpdateStatement<diesel::query_source::filter::FilteredQuerySource<testing::table, diesel::expression::predicates::Eq<testing::columns::id, diesel::expression::bound::Bound<diesel::types::Integer, &i32>>>, (diesel::expression::predicates::Eq<testing::columns::arr, diesel::expression::bound::Bound<diesel::types::Nullable<diesel::types::Array<diesel::types::Text>>, &std::vec::Vec<std::string::String>>>,)> : diesel::query_builder::Query`, `diesel::query_builder::update_statement::UpdateStatement<diesel::query_source::filter::FilteredQuerySource<testing::table, diesel::expression::predicates::Eq<testing::columns::id, diesel::expression::bound::Bound<diesel::types::Integer, &i32>>>, (diesel::expression::predicates::Eq<testing::columns::arr, diesel::expression::bound::Bound<diesel::types::Nullable<diesel::types::Array<diesel::types::Text>>, &std::vec::Vec<std::string::String>>>,)> : diesel::query_builder::Query`, `diesel::query_builder::update_statement::UpdateStatement<diesel::query_source::filter::FilteredQuerySource<testing::table, diesel::expression::predicates::Eq<testing::columns::id, diesel::expression::bound::Bound<diesel::types::Integer, &i32>>>, (diesel::expression::predicates::Eq<testing::columns::arr, diesel::expression::bound::Bound<diesel::types::Nullable<diesel::types::Array<diesel::types::Text>>, &std::vec::Vec<std::string::String>>>,)> : diesel::query_builder::Query`, `&mut diesel::query_builder::update_statement::UpdateStatement<diesel::query_source::filter::FilteredQuerySource<testing::table, diesel::expression::predicates::Eq<testing::columns::id, diesel::expression::bound::Bound<diesel::types::Integer, &i32>>>, (diesel::expression::predicates::Eq<testing::columns::arr, diesel::expression::bound::Bound<diesel::types::Nullable<diesel::types::Array<diesel::types::Text>>, &std::vec::Vec<std::string::String>>>,)> : diesel::query_builder::AsQuery`, `_ : diesel::query_builder::QueryFragment<_>`, `&mut diesel::query_builder::update_statement::UpdateStatement<diesel::query_source::filter::FilteredQuerySource<testing::table, diesel::expression::predicates::Eq<testing::columns::id, diesel::expression::bound::Bound<diesel::types::Integer, &i32>>>, (diesel::expression::predicates::Eq<testing::columns::arr, diesel::expression::bound::Bound<diesel::types::Nullable<diesel::types::Array<diesel::types::Text>>, &std::vec::Vec<std::string::String>>>,)> : diesel::query_builder::Query`, `&mut diesel::query_builder::update_statement::UpdateStatement<diesel::query_source::filter::FilteredQuerySource<testing::table, diesel::expression::predicates::Eq<testing::columns::id, diesel::expression::bound::Bound<diesel::types::Integer, &i32>>>, (diesel::expression::predicates::Eq<testing::columns::arr, diesel::expression::bound::Bound<diesel::types::Nullable<diesel::types::Array<diesel::types::Text>>, &std::vec::Vec<std::string::String>>>,)> : diesel::query_builder::Query`, `&mut diesel::query_builder::update_statement::UpdateStatement<diesel::query_source::filter::FilteredQuerySource<testing::table, diesel::expression::predicates::Eq<testing::columns::id, diesel::expression::bound::Bound<diesel::types::Integer, &i32>>>, (diesel::expression::predicates::Eq<testing::columns::arr, diesel::expression::bound::Bound<diesel::types::Nullable<diesel::types::Array<diesel::types::Text>>, &std::vec::Vec<std::string::String>>>,)> : diesel::query_builder::Query`
error: aborting due to 3 previous errors
error: Could not compile `diesel_repro`.

To learn more, run the command again with --verbose.

It looks to me like both errors are choking on a &std::vec::Vec<std::string::String> where I would expect to see std::vec::Vec<std::string::String> (although I'm not really familiar with the codegen systems here).

[sgrif]

It looks to me like both errors are choking on a &std::vec::Vecstd::string::String where I would expect to see std::vec::Vecstd::string::String

It takes a reference because the place where we construct the values is in a method that takes &self on the AST, not self, so each field needs to be borrowed. We actually implement the various traits on &'insert YourStruct not YourStruct directly. This used to be because the structure of the code demanded it. That's no longer the case, but we still borrow the data simply because there's no reason for us to own it, and it's nice to be able to pass in a slice rather than a vector. That said, maybe it's worth re-considering that use case...

None of this is relevant to the actual bug, just wanted to answer the question.

[sgrif]

Couldn't replicate with our test suite's schema. Noticed that our array column is NOT NULL, I think that's the culprit.

[anp]

Thanks for the clarification on references.

OK, tried a few things with the column still allowing nulls and without, and everything appears to be working for my use case. But I have a question about handling nulls in PG arrays.

With insertable_into and a nullable column, if I use Option<Vec<String>>, everything compiles. However, if the column allows NULLs, I would think that properly modelling a nullable text[] would actually be Option<Vec<Option<String>>>, or maybe Vec<Option<String>>, since it seems like postgres will let you put a NULL in the middle of the array. I'm looking at this portion of the docs for the PG array type:

http://www.postgresql.org/docs/current/static/arrays.html

For example, if array myarray currently has 4 elements, it will have six elements after an update that assigns to myarray[6]; myarray[5] will contain null.

How would diesel handle null-in-the-middle arrays when deserializing to Option<Vec<String>>? Also, the postgres docs don't seem clear on whether the quoted operation is permitted on a NOT NULL column (is NOT NULL for an array column referring to the array itself, its contents, or both?).

[sgrif]

Currently we don't support arrays containing null or multidimensional arrays. As you mentioned, neither of these changes are necessarily enforced on the backend side, and we're not sure what the best solution is there.