-
Notifications
You must be signed in to change notification settings - Fork 109
Fix validation rule to detect tuples in projections or groupbys #672
Conversation
82b9ecb
to
c7e7418
Compare
It should not only be distinct, but anything that is projected. A tuple simply can't be projected (neither in Project nor GroupBy) |
d038f7f
to
1ba012b
Compare
@erizocosmico change to detect tuples in projections and groupbys instead of distinct implemented, PTAL when you can. |
Update: while grepping around noticed that there was already a validation rule checking that there are not tuples in Projections or GroupBys (https://github.com/src-d/go-mysql-server/blob/master/sql/analyzer/validation_rules.go#L198), it even have tests with tuples (https://github.com/src-d/go-mysql-server/blob/master/sql/analyzer/validation_rules_test.go#L234) but adding a test with a DISTINCT->Projection->Tuple doesn't detect the error (and it crash like the issue reported if you do it on the REPL), so I'm now investigating why this existing rule doesn't work for this case. I think that what @kuba-- said above that the rule needs to inspect nodes recursively would be the right fix. |
Signed-off-by: Juanjo Alvarez <juanjo@sourced.tech>
Yes, with a recursive inspection the existing rule works beautifully, PTAL again since I removed basically everything from the previous commits and rewrote again. |
Signed-off-by: Juanjo Alvarez <juanjo@sourced.tech>
Signed-off-by: Juanjo Alvarez <juanjo@sourced.tech>
Signed-off-by: Juanjo Alvarez <juanjo@sourced.tech>
Fixes src-d/gitbase/issues/785
Modifies a rule to check if distinct have a tuple argument (
DISTINCT(1,2)
), recursively. Unrelatedly, it also fix ordering in the README for the FROM_BASE64 function (my fault).There are also a couple unrelated files with
go fmt
changes which I decided to add to this PR since it won't hurt.Signed-off-by: Juanjo Alvarez juanjo@sourced.tech