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

left_join incorrectly compiling if additional joins are done afterwards #1459

Closed
sgrif opened this issue Jan 7, 2018 · 7 comments
Closed

Comments

@sgrif
Copy link
Member

sgrif commented Jan 7, 2018

Reported via gitter. This query compiles successfully:

build::table
        .left_join(revision::table)
        .inner_join(job::table)
        .select((
                job::name,
                build::build_no,
                revision::hash,
        ))
        .load::<Build>(conn)

It should fail to compile unless .nullable has been called on revision::hash. Reversing the order of the join calls causes this code to fail to compile as it should

@sgrif
Copy link
Member Author

sgrif commented Jan 7, 2018

I actually don't know if we can fix this until disjointness based on associated types lands. We need to change this impl:

        impl<Left, Right> SelectableExpression<
            Join<Left, Right, Inner>,
        > for $column_name where
            $column_name: AppearsOnTable<Join<Left, Right, Inner>>,
            Join<Left, Right, Inner>: AppearsInFromClause<$($table)::*, Count=Once>,
        {
        }

to be these two impls:


        impl<Left, Right> SelectableExpression<
            Join<Left, Right, Inner>,
        > for $column_name where
            $column_name: SelectableExpression<Left>,
            Right: AppearsInFromClause<$($table)::*, Count=Never>,
        {
        }

        impl<Left, Right> SelectableExpression<
            Join<Left, Right, Inner>,
        > for $column_name where
            $column_name: SelectableExpression<Right>,
            Left: AppearsInFromClause<$($table)::*, Count=Never>,
        {
        }

I need to think about this more to come up with a way to fix this today.

@Ten0
Copy link
Member

Ten0 commented May 14, 2019

I have the same issue.
The following incorrectly compiles :

CREATE TABLE "integrations" (
	"id" Integer GENERATED ALWAYS AS IDENTITY PRIMARY KEY,
	"label" Character Varying ( 100 ) NOT NULL UNIQUE,
	"field_z" Text [] NOT NULL
);

CREATE TABLE "user" ( 
	"id" Integer GENERATED ALWAYS AS IDENTITY,
	"name" Character Varying( 128 ) NOT NULL UNIQUE,
	"integration"  Integer NOT NULL REFERENCES integrations(id),
	PRIMARY KEY ( "id" )
);

CREATE TABLE "params1" (
	"user_id" Integer NOT NULL REFERENCES users(id) PRIMARY KEY,
	"field_x" Boolean NOT NULL
);

CREATE TABLE "params2" (
	"user_id" Integer NOT NULL REFERENCES users(id) PRIMARY KEY,
	"field_y" Boolean NOT NULL
);

CREATE TABLE user_tokens ( 
	"token" Character Varying( 64 ) NOT NULL,
	"user_id" Integer NOT NULL REFERENCES users(id),
	PRIMARY KEY ( "token" )
);
#[derive(Debug, Queryable)]
pub struct User {
	pub id: i32,
	pub field_x: bool,
	pub field_y: bool,
	pub field_z: Vec<String>,
}
		let query = schema::user_tokens::table
			.inner_join(
				schema::users::table
					.inner_join(schema::integrations::table)
					.left_join(schema::params1::table)
					.left_join(schema::params2::table),
			)
			.select((
				schema::user_tokens::user_id,
				schema::params1::field_x,
				schema::params2::field_y,
				schema::integrations::field_z,
			))
			.filter(schema::user_tokens::token.eq(token));
		let user = query.first::<User>(&db()?);

This compiles even though there is no guarantee that there is a params1 that matches the user.

As a temporary workaround if this can't be fixed easily, I'd rather the database call fails or even panics than randomly set my booleans to false (and I don't know what to other types) when receiving nulls.

I've tried to reverse the order of the inner and left joins, but that didn't change anything.

@Ten0
Copy link
Member

Ten0 commented May 14, 2019

Digging into it, it seems the workaround would be:
https://github.com/diesel-rs/diesel/blob/master/diesel/src/pg/types/primitives.rs#L12

impl FromSql<sql_types::Bool, Pg> for bool {
    fn from_sql(bytes: Option<&[u8]>) -> deserialize::Result<Self> {
        Ok(not_none!(bytes)[0] != 0)
    }
}

Why was the choice made to have an Ok(false) if None there ?

@sgrif
Copy link
Member Author

sgrif commented May 14, 2019

26720d7

@sgrif
Copy link
Member Author

sgrif commented May 14, 2019

#104

@Ten0
Copy link
Member

Ten0 commented Jun 18, 2020

Isn't this a duplicate of #2001 ?

@weiznich
Copy link
Member

Yes that's a duplicate of #2001

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

No branches or pull requests

3 participants