-
Notifications
You must be signed in to change notification settings - Fork 110
sql: use custom boolean coercion logic in condition evaluation #768
Conversation
Signed-off-by: Miguel Molina <[email protected]>
ee455f0 to
568f586
Compare
ajnavarro
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice catch.
| return b, nil | ||
| case int, int64, int32, int16, int8, uint, uint64, uint32, uint16, uint8: | ||
| return b != 0, nil | ||
| case time.Duration: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should a Duration or time really ever evaluate to false?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
when it's 0, i guess
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But those are valid dates/durations. Anyway it's very minor; we should probably just do whatever MySQL do in these cases.
| case float32, float64: | ||
| return int(math.Round(v.(float64))) != 0, nil | ||
| default: | ||
| return false, nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how about the string:
select 1 from dual where "1";
basically it false, but for above string it's true for mysql.
Part of the fix for src-d/gitbase#902
The idea is to use
EvaluateConditionas a common logic for filtering in both gitbase squashed tables and go-mysql-server.Right now, in go-mysql-server we just check the result
== true, which not only is not correct but is not consistent with how gitbase does it.In gitbase we use
sql.Boolean.Convert, but by definition the convert method of boolean can't handle some types.That's why I added some helper function for this with the type coercion logic that we can also reuse on gitbase.