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

Multi-join query generates query with syntax error (no type error) #89

Open
bitemyapp opened this issue Jan 16, 2015 · 4 comments
Open

Comments

@bitemyapp
Copy link
Contributor

fetchStudentByTeacher :: MonadIO m =>
                         (Key Teacher) -> (Key Student)
                         -> SqlPersistT m (Maybe (Entity Student))
fetchStudentByTeacher teacherId studentId = liftM listToMaybe students
  where students = select $
                   from $
                   \(student `InnerJoin` courseMember
                     `InnerJoin` course `InnerJoin` teacher) -> do
                     on (teacher ^. TeacherId ==. course ^. CourseTeacherId)
                     on (courseMember ^. CourseMembershipCourseId ==. course ^. CourseId)
                     where_ (courseMember ^. CourseMembershipStudentId ==. val studentId)
                     where_ (student ^. StudentId ==. val studentId)
                     where_ (teacher ^. TeacherId ==. val teacherId)
                     return student

When I try to run it:

Prelude> runDevDB $ fetchStudentByTeacher (toSqlKey 1) (toSqlKey 1)
15/Jan/2015:19:09:54 -0600 [Debug#SQL] "SELECT \"students\".\"id\", \"students\".\"first_name\", \"students\".\"last_name\", \"students\".\"coins_today\", \"students\".\"coins_total\", \"students\".\"created_at\", \"students\".\"updated_at\", \"students\".\"teacher_id\"\nFROM \"students\" INNER JOIN \"course_memberships\" INNER JOIN \"courses\" ON \"course_memberships\".\"course_id\" = \"courses\".\"id\" INNER JOIN \"teachers\" ON \"teachers\".\"id\" = \"courses\".\"teacher_id\"\nWHERE (\"course_memberships\".\"student_id\" = ?) AND ((\"students\".\"id\" = ?) AND (\"teachers\".\"id\" = ?))\n" [PersistInt64 1,PersistInt64 1,PersistInt64 1] @(persistent-2.1.1.3:Database.Persist.Sql.Raw ./Database/Persist/Sql/Raw.hs:42:26)
*** Exception: user error (Postgresql.withStmt': bad result status FatalError (("PGRES_FATAL_ERROR","ERROR:  syntax error at or near \"WHERE\"\nLINE 3: WHERE (\"course_memberships\".\"student_id\" = 1) AND ((\"student...\n        ^\n")))
@bitemyapp
Copy link
Contributor Author

I was able to make it work by changing the code to:

fetchStudentByTeacher :: MonadIO m =>
                         (Key Teacher) -> (Key Student)
                         -> SqlPersistT m (Maybe (Entity Student))
fetchStudentByTeacher teacherId studentId = liftM listToMaybe students
  where students = select $
                   from $
                   \(student `InnerJoin` courseMember
                     `InnerJoin` course `InnerJoin` teacher) -> do
                     on (teacher ^. TeacherId ==. course ^. CourseTeacherId)
                     on (courseMember ^. CourseMembershipCourseId ==. course ^. CourseId)
                     on (courseMember ^. CourseMembershipStudentId ==. student ^. StudentId)
                     where_ (student ^. StudentId ==. val studentId)
                     where_ (teacher ^. TeacherId ==. val teacherId)
                     return student

@meteficha
Copy link
Member

This is a known gotcha with explicit joins. I guess we could catch this before querying PostgreSQL and give a better error message, though.

@bitemyapp
Copy link
Contributor Author

@meteficha is there anything that can be done to either eliminate the inflexibility (preferable) or at least make it a type-error? (Linear types'ish? Ick.)

Edit: To clarify, I'm asking because I want to know what it would take to fix the fundamental problem, even if it's impossible or impractical.

@meteficha
Copy link
Member

First of all, one needs to think about which errors should be checked. Verify the number of ons match the number of explicit joins? A bit hard but probably doable. Verify that each on refers at least to one of the tables in question? A lot harder while still supporting edge cases.

If using the current syntax, you'll probably need indexed monads to keep track of the joins. Using RebindableSyntax it could be made pretty but would wreck havoc on everything else, to keep your sanity you'd need to isolate esqueleto code from all other code.

There's also the option of using another syntax. By syntax I mean the EDSL syntax. Probably at least from's type would have to be changed so that it can distinguish the ons from the rest.

All in all, not an easy job to make these compile time errors using current esqueleto, at least not without some insight I'm missing :).

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

No branches or pull requests

2 participants