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

Crashing by Division Operators #7026

Closed
suyZhong opened this issue Nov 19, 2023 · 5 comments
Closed

Crashing by Division Operators #7026

suyZhong opened this issue Nov 19, 2023 · 5 comments
Labels
bug Something isn't working customer issue panic

Comments

@suyZhong
Copy link

The following test case crashed go-mysql-server.

CREATE TABLE t0( c1 INT);
INSERT INTO t0(c1) VALUES (1);
SELECT * FROM t0 WHERE ((c1/-364240480)/(880447354))%1;

Here's my error trace:

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x30 pc=0x171dcef]

goroutine 570 [running]:
github.com/dolthub/go-mysql-server/sql/expression.convertToDecimalValue({0x22bd280?, 0xc000014930?}, 0x80?)
        /home/suyang/go/pkg/mod/github.com/dolthub/go-mysql-server@v0.17.1-0.20231117210301-8d70c233f221/sql/expression/div.go:406 +0xcf
github.com/dolthub/go-mysql-server/sql/expression.(*Mod).convertLeftRight(0xc000d6aed0, 0xc0009a7d38?, {0x22bd280, 0xc000014930}, {0x22bd3c0, 0x437ca28})
        /home/suyang/go/pkg/mod/github.com/dolthub/go-mysql-server@v0.17.1-0.20231117210301-8d70c233f221/sql/expression/mod.go:150 +0x173
github.com/dolthub/go-mysql-server/sql/expression.(*Mod).Eval(0xc000bd7f00?, 0x0?, {0xc000df5d40?, 0xc000f7f820?, 0x1?})
        /home/suyang/go/pkg/mod/github.com/dolthub/go-mysql-server@v0.17.1-0.20231117210301-8d70c233f221/sql/expression/mod.go:119 +0x50
github.com/dolthub/go-mysql-server/sql.EvaluateCondition(0xc000bd7e00?, {0x304b6d0?, 0xc000d6aed0?}, {0xc000df5d40?, 0x0?, 0x0?})
        /home/suyang/go/pkg/mod/github.com/dolthub/go-mysql-server@v0.17.1-0.20231117210301-8d70c233f221/sql/core.go:256 +0x3e
github.com/dolthub/go-mysql-server/sql/plan.(*FilterIter).Next(0xc000f7f760, 0x0?)
        /home/suyang/go/pkg/mod/github.com/dolthub/go-mysql-server@v0.17.1-0.20231117210301-8d70c233f221/sql/plan/filter.go:119 +0x7d
github.com/dolthub/go-mysql-server/sql/rowexec.transactionCommittingIter.Next(...)
        /home/suyang/go/pkg/mod/github.com/dolthub/go-mysql-server@v0.17.1-0.20231117210301-8d70c233f221/sql/rowexec/transaction_iters.go:78
github.com/dolthub/go-mysql-server/sql/plan.(*trackedRowIter).Next(0xc000bd7e40, 0x0?)
        /home/suyang/go/pkg/mod/github.com/dolthub/go-mysql-server@v0.17.1-0.20231117210301-8d70c233f221/sql/plan/process.go:347 +0x24
github.com/dolthub/go-mysql-server/server.(*Handler).doQuery.func2()
        /home/suyang/go/pkg/mod/github.com/dolthub/go-mysql-server@v0.17.1-0.20231117210301-8d70c233f221/server/handler.go:286 +0x125
golang.org/x/sync/errgroup.(*Group).Go.func1()
        /home/suyang/go/pkg/mod/golang.org/x/sync@v0.3.0/errgroup/errgroup.go:75 +0x56
created by golang.org/x/sync/errgroup.(*Group).Go in goroutine 497
        /home/suyang/go/pkg/mod/golang.org/x/sync@v0.3.0/errgroup/errgroup.go:72 +0x96

I originally find this by building dolt from source cersion 4cffade. It could also be reproduced in 1.26.1.

@timsehn timsehn added bug Something isn't working panic labels Nov 20, 2023
@max-hoffman
Copy link
Contributor

We appear to have a lot of issues with context-dependent decimal casting:

tmp4> select 1 where ((1/-364240480)/(88044)) != 0;
Empty set (0.00 sec)

tmp4> select 1 where ((1/-364240480)/(880)) != 0;
+---+
| 1 |
+---+
| 1 |
+---+

Generally our decimal casting in SELECT statements seems fine, but are mostly incorrect when we try to cast the same values to boolean in filters.

@max-hoffman
Copy link
Contributor

So fixing the panic is kind of trivial, but while trying to test I'm finding a lot of MySQL incompatibilities:

@@ -551,7 +551,17 @@ func GetPrecisionAndScale(val interface{}) (uint8, uint8) {
        default:
                str = fmt.Sprintf("%v", v)
        }
-       return GetDecimalPrecisionAndScale(str)
+       p, s := GetDecimalPrecisionAndScale(str)
+       if s > types.DecimalTypeMaxScale {
+               s = types.DecimalTypeMaxScale
+       }
+       if p > types.DecimalTypeMaxPrecision {
+               p = types.DecimalTypeMaxPrecision
+       }
+       return p, s
 }

@max-hoffman
Copy link
Contributor

dolthub/go-mysql-server#2154 fixes the panic, it doesn't fix the decimal->bool type conversion in filters

@suyZhong
Copy link
Author

@max-hoffman Thanks for your hard work. Want to ask if we should report decimal-related issues before you solve it.

@max-hoffman
Copy link
Contributor

I think I misspoke yesterday, it looks like we do return the correct value for this script. Anything that fails in v1.28.0 should be fair game to report as a bug.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working customer issue panic
Projects
None yet
Development

No branches or pull requests

4 participants