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

Bug Report: Joins using lookup vindexes seems to be broken on v18 #16990

Closed
arthurschreiber opened this issue Oct 17, 2024 · 4 comments
Closed

Comments

@arthurschreiber
Copy link
Contributor

Overview of the Issue

Joining more than 3 tables using lookup vindexes seems to be broken on v18. I think this was broken recently (past few days).

Queries will fail due to missing bind variables - even though no bind variables are in use at all.

Reproduction Steps

SELECT `music`.id
FROM `music`
INNER JOIN music `music2` ON `music2`.`id` = `music`.`id`
INNER JOIN music `music3` ON `music3`.`id` = `music2`.`id`
INNER JOIN music `music4` ON `music4`.`id` = `music3`.`id`

Notice the :music3_id bind variable in the generated query.

{
  "QueryType": "SELECT",
  "Original": "SELECT `music`.id FROM `music` INNER JOIN music `music2` ON `music2`.`id` = `music`.`id` INNER JOIN music `music3` ON `music3`.`id` = `music2`.`id` INNER JOIN music `music4` ON `music4`.`id` = `music3`.`id`",
  "Instructions": {
    "OperatorType": "Route",
    "Variant": "Scatter",
    "Keyspace": {
      "Name": "user",
      "Sharded": true
    },
    "FieldQuery": "select music.id from music, music as music2, music as music3, music as music4 where 1 != 1",
    "Query": "select music.id from music, music as music2, music as music3, music as music4 where music2.id = :music3_id and music2.id = music.id and music4.id = music3.id and music3.id = music2.id",
    "Table": "music"
  },
  "TablesUsed": [
    "user.music"
  ]
}

Binary Version

v18

Operating System and Environment details

N/A

Log Fragments

No response

@arthurschreiber arthurschreiber added Type: Bug Needs Triage This issue needs to be correctly labelled and triaged labels Oct 17, 2024
@arthurschreiber
Copy link
Contributor Author

Using git bisect, I was able to track down afb135d as the commit that introduced this regression.

@arthurschreiber
Copy link
Contributor Author

Also, this does not seem to be an issue on v19 and later. 🤔

@arthurschreiber arthurschreiber added Component: Query Serving and removed Needs Triage This issue needs to be correctly labelled and triaged labels Oct 17, 2024
@arthurschreiber
Copy link
Contributor Author

#17069 has fixed the query described in this issue, but we found another query structure that's still broken and generates incorrect bind variables:

SELECT `music`.id
FROM `music`
INNER JOIN music `music2` ON `music2`.`id` = `music`.`id`
INNER JOIN music `music3` ON `music3`.`id` = `music2`.`id`
INNER JOIN music `music4` ON `music4`.`id` = `music3`.`id`
WHERE music.user_index = music4.user_index

Generates the following query plan:

{
  "QueryType": "SELECT",
  "Original": "SELECT `music`.id FROM `music` INNER JOIN music `music2` ON `music2`.`id` = `music`.`id` INNER JOIN music `music3` ON `music3`.`id` = `music2`.`id` INNER JOIN music `music4` ON `music4`.`id` = `music3`.`id` WHERE music.user_index = music4.user_index",
  "Instructions": {
    "OperatorType": "Route",
    "Variant": "Scatter",
    "Keyspace": {
      "Name": "user",
      "Sharded": true
    },
    "FieldQuery": "select music.id from music, music as music2, music as music3, music as music4 where 1 != 1",
    "Query": "select music.id from music, music as music2, music as music3, music as music4 where music2.id = :music3_id and music2.id = music.id and music.user_index = :music4_user_index and music4.id = music3.id and music3.id = :music2_id and music4.user_index = :music_user_index and music3.id = music2.id and music.user_index = music4.user_index",
    "Table": "music"
  },
  "TablesUsed": [
    "user.music"
  ]
}

Note that there's two "wrong" conditions added to this query: music.user_index = :music4_user_index and music4.user_index = :music_user_index.

@arthurschreiber
Copy link
Contributor Author

Closing this as #17069 and #17099 have fixed all known cases of this. 👍

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