Commit 1102cb2
committed
Detect long (BIGINT) arithmetic overflow instead of silently wrapping
PPL/SQL long arithmetic (+, -, *) in eval/SELECT expressions silently
wrapped on overflow in the Calcite engine (e.g. long_field * 999...9 wrapped
to a negative value with HTTP 200). SqlStdOperatorTable.PLUS/MINUS/MULTIPLY
generate plain Java +/-/* in the Enumerable code path, which wrap.
Rewrite long +/-/* to their overflow-checked variants (CHECKED_PLUS /
CHECKED_MINUS / CHECKED_MULTIPLY), which generate Math.addExact etc. and throw
ArithmeticException on overflow. The exception is caught in QueryService and
surfaced as a 4xx client error instead of wrapping or falling back to V2.
Scoped to BIGINT operands only: narrower integer arithmetic (byte/short/int)
is widened to a type that cannot overflow before this rewrite runs (PPLFuncImpTable
promotes byte/short to INT and any int/long to BIGINT for +/-/*), so long — which
has no wider integer type — is the sole remaining overflow case on the Calcite
engine. Float/double/decimal follow IEEE 754 / decimal semantics and have no
CHECKED_* runtime, so they are left untouched. The rewrite runs after the
analytics-engine fork, so only the Calcite path is affected (the DataFusion
backend handles its own overflow).
The rewrite runs before pushdown so both coordinator-executed and pushed-down
(script) arithmetic are checked; PPLAggregateConvertRule, OpenSearchRelOptUtil,
and ExtendedRelJson recognize the CHECKED_* kinds so aggregate/sort pushdown and
script serialization keep working. Adds a REST yaml test (issues/5164.yml) and
updates the affected explain fixtures.
Resolves #5164
Signed-off-by: Kai Huang <ahkcs@amazon.com>1 parent 4985a39 commit 1102cb2
16 files changed
Lines changed: 304 additions & 18 deletions
File tree
- core/src/main/java/org/opensearch/sql
- calcite/plan/rule
- executor
- docs/user/ppl/functions
- integ-test/src
- test/resources/expectedOutput/calcite
- yamlRestTest/resources/rest-api-spec/test/issues
- opensearch/src/main/java/org/opensearch/sql/opensearch
- storage/serde
- util
Lines changed: 9 additions & 2 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
135 | 135 | | |
136 | 136 | | |
137 | 137 | | |
138 | | - | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
139 | 141 | | |
140 | 142 | | |
141 | 143 | | |
| |||
280 | 282 | | |
281 | 283 | | |
282 | 284 | | |
283 | | - | |
| 285 | + | |
| 286 | + | |
| 287 | + | |
| 288 | + | |
| 289 | + | |
| 290 | + | |
284 | 291 | | |
285 | 292 | | |
286 | 293 | | |
| |||
Lines changed: 105 additions & 3 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
16 | 16 | | |
17 | 17 | | |
18 | 18 | | |
| 19 | + | |
19 | 20 | | |
20 | 21 | | |
21 | 22 | | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
22 | 26 | | |
| 27 | + | |
| 28 | + | |
23 | 29 | | |
24 | 30 | | |
25 | 31 | | |
| |||
174 | 180 | | |
175 | 181 | | |
176 | 182 | | |
177 | | - | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
178 | 186 | | |
179 | 187 | | |
180 | 188 | | |
| |||
187 | 195 | | |
188 | 196 | | |
189 | 197 | | |
190 | | - | |
| 198 | + | |
| 199 | + | |
| 200 | + | |
| 201 | + | |
| 202 | + | |
| 203 | + | |
| 204 | + | |
| 205 | + | |
| 206 | + | |
191 | 207 | | |
192 | 208 | | |
193 | 209 | | |
| |||
227 | 243 | | |
228 | 244 | | |
229 | 245 | | |
230 | | - | |
| 246 | + | |
| 247 | + | |
231 | 248 | | |
232 | 249 | | |
233 | 250 | | |
| |||
383 | 400 | | |
384 | 401 | | |
385 | 402 | | |
| 403 | + | |
| 404 | + | |
| 405 | + | |
| 406 | + | |
| 407 | + | |
| 408 | + | |
| 409 | + | |
| 410 | + | |
| 411 | + | |
| 412 | + | |
| 413 | + | |
| 414 | + | |
| 415 | + | |
| 416 | + | |
| 417 | + | |
| 418 | + | |
| 419 | + | |
| 420 | + | |
| 421 | + | |
| 422 | + | |
| 423 | + | |
| 424 | + | |
| 425 | + | |
| 426 | + | |
| 427 | + | |
| 428 | + | |
| 429 | + | |
| 430 | + | |
| 431 | + | |
| 432 | + | |
| 433 | + | |
| 434 | + | |
| 435 | + | |
| 436 | + | |
| 437 | + | |
| 438 | + | |
| 439 | + | |
| 440 | + | |
| 441 | + | |
| 442 | + | |
| 443 | + | |
| 444 | + | |
| 445 | + | |
| 446 | + | |
| 447 | + | |
| 448 | + | |
| 449 | + | |
| 450 | + | |
| 451 | + | |
| 452 | + | |
| 453 | + | |
| 454 | + | |
| 455 | + | |
| 456 | + | |
| 457 | + | |
| 458 | + | |
| 459 | + | |
| 460 | + | |
| 461 | + | |
| 462 | + | |
| 463 | + | |
| 464 | + | |
| 465 | + | |
| 466 | + | |
| 467 | + | |
| 468 | + | |
| 469 | + | |
| 470 | + | |
| 471 | + | |
| 472 | + | |
| 473 | + | |
| 474 | + | |
| 475 | + | |
| 476 | + | |
| 477 | + | |
| 478 | + | |
| 479 | + | |
| 480 | + | |
| 481 | + | |
| 482 | + | |
| 483 | + | |
| 484 | + | |
| 485 | + | |
| 486 | + | |
| 487 | + | |
386 | 488 | | |
387 | 489 | | |
388 | 490 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
11 | 11 | | |
12 | 12 | | |
13 | 13 | | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
14 | 18 | | |
15 | 19 | | |
16 | 20 | | |
| |||
Lines changed: 1 addition & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
6 | 6 | | |
7 | 7 | | |
8 | 8 | | |
9 | | - | |
| 9 | + | |
Lines changed: 1 addition & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
6 | 6 | | |
7 | 7 | | |
8 | 8 | | |
9 | | - | |
| 9 | + | |
Lines changed: 1 addition & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
6 | 6 | | |
7 | 7 | | |
8 | 8 | | |
9 | | - | |
| 9 | + | |
0 commit comments