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

sql: decimal round can DOS the server #8633

Closed
maddyblue opened this issue Aug 18, 2016 · 5 comments
Closed

sql: decimal round can DOS the server #8633

maddyblue opened this issue Aug 18, 2016 · 5 comments
Assignees
Labels
C-investigation Further steps needed to qualify. C-label will change. C-performance Perf of queries or internals. Solution not expected to change functional behavior.

Comments

@maddyblue
Copy link
Contributor

SELECT round(-0.32096519520627553, -2438602134409251682)

The above query will peg one CPU core at 100% for many minutes until it completes (I have not run it long enough to see how long that is). This is because the implementation of decimal rounding does some quotient remainder calculations that take a while or something.

In any case it's not obvious to me how to handle this, but something should be done since this makes it trivial to shut down a server.

Found with RSG.

@maddyblue maddyblue changed the title sql: decimal round can DDOS the server sql: decimal round can DOS the server Aug 18, 2016
@knz
Copy link
Contributor

knz commented Aug 19, 2016

cc @nvanbenschoten.

@knz knz added C-investigation Further steps needed to qualify. C-label will change. C-performance Perf of queries or internals. Solution not expected to change functional behavior. labels Aug 19, 2016
@knz
Copy link
Contributor

knz commented Aug 19, 2016

I think we shouldn't allow abnormally large scales in the 2nd argument to round, and it seems to me that negative scales are meaningless. WDYT?

@RaduBerinde
Copy link
Member

Wow, yeah the second argument is the number of decimals, definitely needs to be non-negative. As for large values, I think the operation should be a no-op if the number we are rounding already has less decimals than we want. If adding such a check is easy, I think it's the way to go because we don't need to set a hard limit specifically for round.

@maddyblue maddyblue self-assigned this Aug 19, 2016
@maddyblue
Copy link
Contributor Author

Postgres' behavior when the 2nd arg is higher than the number of decimals is to append 0s:

# select round(-0.32096519520627553, 100);
                                                  round                                                  
---------------------------------------------------------------------------------------------------------
 -0.3209651952062755300000000000000000000000000000000000000000000000000000000000000000000000000000000000
(1 row)

Notably, though, postgres limits the number of digits after the decimal point to 2000. I'm not sure if that's just with round or with their decimal implementation in general.


Upon investigation, this is caused (in cockroach) by the Exp method in decimal. We can easily reproduce this with:

select exp(1845949582::decimal)

Which similarly pegs a core. That query in postgres says:

=# select exp(1845949582::decimal);
ERROR:  argument for function "exp" too big

But I can't find in the source where that error is coming from to see what they do there.

@knz
Copy link
Contributor

knz commented Aug 25, 2016

FYI the code in pg resides in src/backend/util/adt (numeric.c and float.c).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-investigation Further steps needed to qualify. C-label will change. C-performance Perf of queries or internals. Solution not expected to change functional behavior.
Projects
None yet
Development

No branches or pull requests

3 participants