Skip to content

[fix](expr opt) Preserve division denominator boundaries - #67892

Open
morrySnow wants to merge 1 commit into
apache:masterfrom
morrySnow:fix/preserve-division-denominator-boundaries
Open

[fix](expr opt) Preserve division denominator boundaries#67892
morrySnow wants to merge 1 commit into
apache:masterfrom
morrySnow:fix/preserve-division-denominator-boundaries

Conversation

@morrySnow

Copy link
Copy Markdown
Contributor

Problem

Arithmetic simplification could change query results by moving factors across a nested division denominator. In particular, zero and nullable inputs could produce a non-NULL value after rewriting.

Root cause

The multiply/divide flattening traversal propagated the outer denominator polarity into the denominator subtree. It then inverted or moved the subtree multiplication and division operands into the enclosing expression, even though a denominator is an evaluation boundary.

Reproduction

1 / (1 / number) was rewritten to number * 1. For number = 0, the original expression evaluates to NULL while the rewritten expression evaluates to 0. The same unsafe flattening affected shapes such as x / (y / z) and x / (y * z).

Fix

Keep every complete subtree reached in denominator position as an atomic operand during multiply/divide flattening. The normal recursive rewrite still simplifies within that subtree, but no factor can be inverted or moved across its enclosing division boundary. Add focused unit coverage for integer, nullable, double, decimal, and zero cases, plus end-to-end plan and result checks.

Tests

  • ./run-fe-ut.sh --run org.apache.doris.nereids.rules.expression.SimplifyArithmeticRuleTest
  • DISABLE_BUILD_UI=ON ./build.sh --fe
  • ./run-regression-test.sh --conf /tmp/env2-range-regression-conf.groovy --run -f regression-test/suites/nereids_rules_p0/expression/simplify_arithmetic/test_simplify_arithmetic.groovy -forceGenOut
  • ./run-regression-test.sh --conf /tmp/env2-range-regression-conf.groovy --run -f regression-test/suites/nereids_rules_p0/expression/simplify_arithmetic/test_simplify_arithmetic.groovy

### What problem does this PR solve?

Issue Number: None

Related PR: None

Problem Summary: SimplifyArithmeticRule flattened multiplication and division across a nested denominator. For example, it rewrote 1 / (1 / number) to number * 1, changing the result for zero from NULL to 0. This reassociation can also change null, overflow, and floating-point behavior.

Treat a subtree reached in denominator position during multiply/divide flattening as an atomic operand. The regular recursive rewrite can still simplify inside that subtree, but its factors cannot cross the enclosing division boundary. Unit and regression tests cover nested division and multiplication denominators, zero and nullable values, and integer, double, and decimal expressions.

### Release note

Fix incorrect query results when arithmetic simplification reassociates expressions across nested division denominators.

### Check List (For Author)

- Test: Unit Test and Regression test
    - ./run-fe-ut.sh --run org.apache.doris.nereids.rules.expression.SimplifyArithmeticRuleTest
    - ./run-regression-test.sh --conf /tmp/env2-range-regression-conf.groovy --run -f regression-test/suites/nereids_rules_p0/expression/simplify_arithmetic/test_simplify_arithmetic.groovy
    - DISABLE_BUILD_UI=ON ./build.sh --fe
- Behavior changed: Yes. Nested denominator expressions remain evaluation boundaries, preserving zero, null, overflow, and floating-point semantics.
- Does this need documentation: No
@morrySnow
morrySnow requested a review from yujun777 as a code owner September 11, 2026 20:20
@hello-stephen

Copy link
Copy Markdown
Contributor

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR.

Please clearly describe your PR:

  1. What problem was fixed (it's best to include specific error reporting information). How it was fixed.
  2. Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
  3. What features were added. Why was this function added?
  4. Which code was refactored and why was this part of the code refactored?
  5. Which functions were optimized and what is the difference before and after the optimization?

@morrySnow

Copy link
Copy Markdown
Contributor Author

run buildall

@hello-stephen

Copy link
Copy Markdown
Contributor

FE UT Coverage Report

Increment line coverage 100.00% (3/3) 🎉
Increment coverage report
Complete coverage report

@morrySnow morrySnow changed the title [fix](nereids) Preserve division denominator boundaries [fix](expr opt) Preserve division denominator boundaries Sep 12, 2026
@hello-stephen

Copy link
Copy Markdown
Contributor
TPC-H: Total hot run time: 16768 ms
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit 0290c8ca516c41068278fe94b4af07c0bdb0e98c, data reload: false

------ Round 1 ----------------------------------
============================================
q1	17592	3078	3087	3078
q2	2100	257	216	216
q3	10230	979	519	519
q4	4667	250	203	203
q5	7683	548	387	387
q6	131	114	93	93
q7	522	486	383	383
q8	9236	786	898	786
q9	3465	2368	2375	2368
q10	6535	852	709	709
q11	395	204	176	176
q12	620	259	199	199
q13	18120	1542	1158	1158
q14	160	155	138	138
q15	q16	448	401	376	376
q17	1340	884	853	853
q18	3081	2279	2258	2258
q19	1292	819	767	767
q20	366	281	199	199
q21	5628	1673	1871	1673
q22	328	273	229	229
Total cold run time: 93939 ms
Total hot run time: 16768 ms

----- Round 2, with runtime_filter_mode=off -----
============================================
q1	3493	3390	3385	3385
q2	499	403	374	374
q3	2249	2306	2225	2225
q4	1190	1155	888	888
q5	2189	2099	2095	2095
q6	164	116	86	86
q7	1011	931	854	854
q8	1591	1409	1410	1409
q9	3126	3078	3099	3078
q10	1854	1824	1651	1651
q11	360	273	249	249
q12	451	422	337	337
q13	1486	1516	1127	1127
q14	159	168	164	164
q15	q16	404	402	362	362
q17	3694	3373	3259	3259
q18	4778	4387	4725	4387
q19	874	899	867	867
q20	987	978	833	833
q21	3879	3231	3226	3226
q22	386	356	326	326
Total cold run time: 34824 ms
Total hot run time: 31182 ms

@hello-stephen

Copy link
Copy Markdown
Contributor
TPC-DS: Total hot run time: 82062 ms
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpcds-tools
TPC-DS sf100 test result on commit 0290c8ca516c41068278fe94b4af07c0bdb0e98c, data reload: false

query5	4247	409	350	350
query6	380	130	127	127
query7	4945	413	213	213
query8	285	122	118	118
query9	8733	2857	2874	2857
query10	390	215	185	185
query11	5381	1024	901	901
query12	127	71	72	71
query13	1198	451	314	314
query14	6109	2196	2081	2081
query14_1	2008	1958	1957	1957
query15	174	118	112	112
query16	914	361	380	361
query17	790	453	359	359
query18	2332	329	234	234
query19	154	136	106	106
query20	71	71	72	71
query21	206	103	92	92
query22	5492	5391	5264	5264
query23	6852	6249	6171	6171
query23_1	6015	6108	6079	6079
query24	7323	1078	776	776
query24_1	746	779	792	779
query25	438	294	251	251
query26	1235	231	127	127
query27	2798	398	258	258
query28	4688	1496	1525	1496
query29	923	421	352	352
query30	249	155	134	134
query31	828	394	335	335
query32	137	73	78	73
query33	469	224	177	177
query34	996	792	503	503
query35	404	402	346	346
query36	573	580	531	531
query37	130	79	70	70
query38	1029	856	850	850
query39	487	479	490	479
query39_1	450	444	437	437
query40	203	90	77	77
query41	58	57	58	57
query42	74	72	71	71
query43	241	242	209	209
query44	988	534	539	534
query45	109	111	105	105
query46	768	841	532	532
query47	767	771	719	719
query48	314	342	232	232
query49	536	250	175	175
query50	723	255	183	183
query51	8053	7955	8043	7955
query52	69	69	61	61
query53	194	196	150	150
query54	204	169	157	157
query55	78	59	55	55
query56	189	157	163	157
query57	671	731	664	664
query58	208	156	149	149
query59	1201	1224	1102	1102
query60	229	195	189	189
query61	114	127	120	120
query62	353	211	193	193
query63	167	170	144	144
query64	2649	683	607	607
query65	1570	1639	1622	1622
query66	1942	270	217	217
query67	9951	9804	9707	9707
query68	3001	1212	752	752
query69	358	229	206	206
query70	664	595	627	595
query71	256	181	166	166
query72	2344	1701	1491	1491
query73	639	564	362	362
query74	2000	1199	1148	1148
query75	1173	1098	968	968
query76	2395	708	523	523
query77	261	255	217	217
query78	3841	3622	3169	3169
query79	2310	783	578	578
query80	1585	319	273	273
query81	493	157	132	132
query82	641	127	99	99
query83	283	206	185	185
query84	298	104	88	88
query85	772	338	276	276
query86	394	173	176	173
query87	991	964	885	885
query88	2767	2109	2091	2091
query89	287	195	173	173
query90	1991	127	127	127
query91	130	116	93	93
query92	81	70	68	68
query93	1502	1132	718	718
query94	652	252	205	205
query95	525	246	312	246
query96	790	562	276	276
query97	1040	1041	1042	1041
query98	162	133	138	133
query99	418	345	309	309
Total cold run time: 177775 ms
Total hot run time: 82062 ms

@hello-stephen

Copy link
Copy Markdown
Contributor
ClickBench: Total hot run time: 14.64 s
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/clickbench-tools
ClickBench test result on commit 0290c8ca516c41068278fe94b4af07c0bdb0e98c, data reload: false

query1	0.01	0.00	0.01
query2	0.07	0.04	0.04
query3	0.25	0.11	0.10
query4	1.60	0.10	0.09
query5	0.18	0.17	0.16
query6	1.23	0.67	0.68
query7	0.03	0.00	0.00
query8	0.04	0.03	0.03
query9	0.28	0.21	0.22
query10	0.35	0.34	0.34
query11	0.15	0.11	0.13
query12	0.14	0.12	0.12
query13	0.30	0.31	0.30
query14	0.46	0.45	0.45
query15	0.38	0.36	0.36
query16	0.23	0.23	0.22
query17	0.62	0.72	0.69
query18	0.19	0.17	0.18
query19	1.16	1.13	1.15
query20	0.01	0.00	0.00
query21	15.45	0.16	0.11
query22	5.10	0.04	0.04
query23	16.19	0.25	0.10
query24	2.97	0.30	0.25
query25	0.10	0.05	0.03
query26	0.85	0.16	0.12
query27	0.04	0.03	0.02
query28	3.69	0.56	0.27
query29	12.41	3.17	2.56
query30	0.26	0.12	0.12
query31	2.75	0.38	0.17
query32	3.52	0.32	0.22
query33	1.42	1.39	1.58
query34	15.39	2.20	1.81
query35	1.77	1.72	1.72
query36	0.46	0.29	0.28
query37	0.06	0.04	0.04
query38	0.04	0.03	0.02
query39	0.03	0.03	0.02
query40	0.11	0.08	0.08
query41	0.08	0.03	0.03
query42	0.04	0.02	0.02
query43	0.04	0.03	0.03
Total cold run time: 90.45 s
Total hot run time: 14.64 s

hello-stephen pushed a commit that referenced this pull request Sep 12, 2026
…67897)

Since today every `Doris_DorisCloudRegression_VaultP0` run dies in the
`run` step before executing a single test, e.g.
#67883 (TeamCity build 39010) and
#67881 / #67882 / #67885 / #67886 / #67892 / #67893:

```
doris-external--minio Pulling
doris-external--minio Error
Error response from daemon: pull access denied for minio/minio, repository does not exist or may require 'docker login': denied: requested access to the resource is denied
ERROR: start minio docker twice failed
```

MinIO stopped publishing container images in October 2025 (the project
is a source-only distribution now, see minio/minio#21647) and the
`minio/minio` and `minio/mc` repositories have since been removed from
Docker Hub altogether
(`https://hub.docker.com/v2/repositories/minio/minio/` answers 404, same
for `minio/mc`). The few VaultP0 runs that still pass do so only on
agents that have the image cached locally (their logs have no `Pulling`
line). The iceberg, hudi and polaris third-party fixtures,
`test_file_cache_warmup_read_metrics_docker` (which runs a `docker run
minio/minio` itself), the all-in-one `cloud.yml` and the datalake
samples reference the same images and are one cache eviction away from
the same failure.

`quay.io/minio/minio` and `quay.io/minio/mc` still serve every tag we
use -- `RELEASE.2024-11-07T00-52-20Z`, `RELEASE.2025-01-20T14-49-07Z`,
mc `RELEASE.2025-01-17T23-25-50Z`, the two 2022 tags of the samples and
`latest` -- and MinIO keeps pushing hotfix tags there (latest one dated
2026-04). `docker manifest inspect` resolves all of them (amd64 / arm64
/ ppc64le). So every reference gets the `quay.io/` prefix and the tags
stay exactly as they were: same builds, different registry. The CI
agents already pull from quay.io for the OceanBase fixture.

A longer-term option is to mirror these three tags into the project's
own `doristhirdpartydocker` namespace, which already hosts hive /
zookeeper / kafka / trinodb; that needs someone with push access to that
Docker Hub organization and can follow separately.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants