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

Presto aggregate function min(_,n) has a differences in ordering of "NaN" #8738

Closed
kgpai opened this issue Feb 13, 2024 · 2 comments
Closed
Assignees
Labels
bug Something isn't working fuzzer-found

Comments

@kgpai
Copy link
Contributor

kgpai commented Feb 13, 2024

Description

Seeing differences in Presto and Velox array orders with min
Velox:
["NaN","-Infinity","-Infinity",0.35368847846984863]

Presto:
["-Infinity","-Infinity",0.35368847846984863,"NaN"]

Error Reproduction

Use seed shown below , and run aggregation fuzzer agains this PR : #8732
I20240213 09:09:09.067404 20587618 AggregationFuzzer.cpp:352] ==============================> Started iteration 0 (seed: 2339927284)

Relevant logs

-- Aggregation[SINGLE [g0, g1, g2] a0 := min(ROW["c0"],ROW["c1"])] -> g0:MAP<SMALLINT,BIGINT>, g1:MAP<BIGINT,BIGINT>, g2:ROW<f0:VARCHAR,f1:SMALLINT>, a0:ARRAY<REAL>
  -- Values[1000 rows in 10 vectors] -> c0:REAL, c1:BIGINT, g0:MAP<SMALLINT,BIGINT>, g1:MAP<BIGINT,BIGINT>, g2:ROW<f0:VARCHAR,f1:SMALLINT>
I20240213 09:09:09.260154 20587922 Task.cpp:1085] All drivers (1) finished for task test_cursor 1 after running for 25 ms.
I20240213 09:09:09.260195 20587922 Task.cpp:1769] Terminating task test_cursor 1 with state Finished after running for 25 ms.
I20240213 09:09:09.261358 20587618 PrestoQueryRunner.cpp:528] Query: DROP TABLE IF EXISTS tmp
I20240213 09:09:09.496459 20587618 PrestoQueryRunner.cpp:528] Query: CREATE TABLE tmp(c0, c1, g0, g1, g2) WITH (format = 'DWRF') AS SELECT cast(null as REAL), cast(null as BIGINT), cast(null as map(SMALLINT, BIGINT)), cast(null as map(BIGINT, BIGINT)), cast(null as row(f0 VARCHAR, f1 SMALLINT))
I20240213 09:09:11.292641 20587618 PrestoQueryRunner.cpp:528] Query: SELECT "$path" FROM tmp
I20240213 09:09:12.073560 20587618 PrestoQueryRunner.cpp:528] Query: DELETE FROM tmp
I20240213 09:09:12.423846 20587618 Writer.cpp:546] Stripe 0: Flush overhead = 1480256, data length = 711711, pre flush mem = 1883520, post flush mem = 2054976. Closing = true
I20240213 09:09:12.425027 20587618 FileSink.cpp:123] closing file: /Users/kpai/presto_install/presto-server-0.284/data/tpch/tmp/fuzzer.dwrf,  total size: 696.92KB
I20240213 09:09:12.431772 20587618 PrestoQueryRunner.cpp:528] Query: SELECT g0, g1, g2, min(c0, c1) as a0 FROM tmp GROUP BY g0, g1, g2
/Users/kpai/src/velox/velox/exec/tests/utils/QueryAssertions.cpp:1129: Failure
Failed
Expected 901, got 901
3 extra rows, 3 missing rows
3 of extra rows:
	null | null | null | ["NaN","-Infinity","-Infinity",0.35368847846984863]
	null | [{"key":624553287982967789,"value":3408273120430831589},{"key":1638632371533948088,"value":null},{"key":3953759512378454070,"value":7732938775053075298},{"key":4577248870163900892,"value":null},{"key":6332027456498242798,"value":5863674525104804843},{"key":7386588018962132295,"value":null},{"key":7779658487962081679,"value":1097175511491423178},{"key":8226199836062350230,"value":7469133982032194519}] | null | ["NaN","NaN",0.08861453086137772,0.09776057302951813,0.34515634179115295,0.36273622512817383,0.4430779814720154,0.5200087428092957,0.7756975889205933,"Infinity",0.5492228269577026,0.6060205698013306,0.7336945533752441,0.7447822690010071,0.7796890139579773,0.8607743382453918,0.9125320315361023,0.9163637757301331,0.9761083126068115,"Infinity","Infinity","Infinity","Infinity","Infinity"]
	null | [{"key":624553287982967789,"value":3408273120430831589},{"key":1638632371533948088,"value":null},{"key":3953759512378454070,"value":7732938775053075298},{"key":4577248870163900892,"value":null},{"key":6332027456498242798,"value":5863674525104804843},{"key":7386588018962132295,"value":null},{"key":7779658487962081679,"value":1097175511491423178},{"key":8226199836062350230,"value":7469133982032194519}] | ["NA2t-nB\"R^&s]it(d{:m.!m!{naRmi6Yj}a89G9\\BG#\\7+hG@)ECM9l}15^G;_chgs]\"yU;)URsHteO,mPF_&vP\"Zv@t<7yMO.V=h']:8ia5`m5z:Patj@[m?IX2KT^`6FdYi_EchaO9J;1O=a5Q#eWD)(X[Q54]x,(<Lw=a-d#`)v=>#[Zvh>:5)-<WUX!pFI:5bCnfS@T'c(H2!t+oD.P+Bb|<dr](^W*|,3.~>8bPq2>xFG?@>aWAO2i\"!nSA)BAB.8-8kmshVPgRF/RdCq`XRXz`>vut[\\(Ulj-n}^zp6!v]DH=?Y4Z9AWCA3E%o:~{jGu\"qb~oeIKeR7,/CqG[pt?A(-3Lap8[{_N.&u=2zkTg/7MlxL^%L*ST^osQD(rq<1=_u08U%:J)itU`#20d\"O}Fva^L2WS\"s@o[?6.Brv;;Q=Zt:kEqg<Zl7|d2h>x\\7Ye6)y~/S~M{bwukIf?#Fwjlt7FltpU<b44:f~5[^b^E2g1b==T;c^K[DKUC045$gm%l,N?sP<I/4{O7QB'f+(eM8u}0jYh0?i[AD{]HMh_M2@&`ZJ$Le}7jN$X@55$f#7iQvkj)F0T\\C8`*5\"IH8ZD/aC:ZIsppGgE|!g2wv)ks{TY$!%\\vnY8G/$]>[egt/77r31I$J/SGZSwwF\",0fXwdx*5tyzjL+('pq5fLe,m0W&P!to2fD9bE.+FEcVKo\\Gy(rM{{qO*Op9!Qk[4xhh$DcP,IHx}e`0T>A=G!\"SKC}'EP>qly,MgjG(W$N-,Z=}r&et)F!g`l>dl~\".^}e(}qfS\\W@H??C[Il._#,!@!_/C!I/U\\n:VAJ-6l}UHtST)IgKFSt8W)<d^Tk(lm)yF*DcJVCJ{aCKBYzpTj):E/I3\\{6w)Ov86v,BBuTG\"[LGNqJpq*u-R?Ir^ntJj0xs7|?mkULF,6H%A?SGY0=+X<HT%gHh=WMR&3sQ:EhY4N}1ucs#-$1`+iFR<JV=;}%nJ%J/A)xbVp\">g#gadGcDCAiug`hSp8yW|\\Jt.w=@\\?-/88\\j>~XIQoKE}~g5)Rt{Rh$dUTgi\\}Yq~#\"~lZRRF_hL-mS'Y:Qg&1`KzHgcKF+}s_DDvcFbSP,@=k6NHhIc-6kq;v$s5P,UhrJwXv2lo.|VwZDqsc0JNzdsk^NOfRsv{AL+p;bUV(!::=x$5K73s5T{-P'9CFV^U/+#f0=]<Jh{tSP3Kt{Xr+@`8Q3[A*qy+a/'M,<%zJ/c`zfPd4o,(6F3\\H-[AG5}tZ5ly>g&26&=Ct7M#TnYjb~veE.l,lNPjM\\lWL(&78DkLye)eq2KOE_`fR9p4#h:+_;e(Wm@9UZz&#/>mp%RCo^SVITp*ko-Oy&5WCO9N^dbR_~DXF?zDd9:5KB;nD!GR%a*DKk5`;c>m^wD$PTe7aL:n8id3exs\\CaEjWAzUnKdq[%`?,`Xe}c6HBa0Q$q!I3\"zwSl(CZR$..:bo&>vRj1.6ks{zZIqk7I\"ampQm>@-*/)}>TdbBR?d@n,)7[kG=n4Ka3~;}OUS}Ds>w?6j`f_|3-2[Od^^PPrC=)/,>r#/GpHxC0uZ$ALYtw2MmKC'Q@BhGW&eLFHoyt'hV#R\"Msdmq>_LNiof^hkEK|FJLRpC_A3Hpk.hr[3vOUAG(LTD*1{G|<XGb3||:%o'U]{@3*CGOQl;MZA^T\\~1e?ty#W1;nLTjmE!.58!`n5c],\\25\\#nW7TE'8){,ARv0^[IDQ7*6$c[&\"%xcj^yBO\\s#*9f\";jG%UtNE3tqJbFPtwk3Pb5ceJRW$/\"_VkPJM'3;sm<={qI0fW]]diMnwQW(tz==!PoMo|fM`'KD)@do(hvk<zQTh)k/Wsd<9!>K!UyR]<h3-G4~3K*B8nukg2$mcJt]]4=1upGewXw*+Vtrs:mXdr/G*?C@2u1o:[k/DZ5C}Z(u9tbf#7$5b\"5oGgN_42F7/3oP<#FG5Bm/w}Y4b6OIJHM|u:ArXw3p#1p8DP0&dj5S;B+]R*C%QPG&Md+|3%ayzfET^rQ?$B'OtZo7(Psx9$]@-W\"zy}+1FLi/opR5\\}|k_v(t5Z~><Y@}T:,KHT@z9@NwIhnSSP:fP}_qr:7DI[$|p[c:nS*L[YHn}Ir!x6kg$P%~:O5vT28|X&3,VPF#q)^X-lXo!*_vUvZQJGM32-2{`sRw`=1.(o1a!c:m{_,NJn.0e;OL;cv\"6_bXX}mK+G#H=egY(it-SFmB>^-&CvY9.Yy9'.$KE'W,$A@%*\"otCMBK4YxVB&0cpwFBec}G`/58G0JPNaXU%2OL}-4@n{|qT`/*^q>&M$p=rE6B,V-%Pq~WPoO{UQr#Rgr<7\\m(%tm+$TTs3llt3zm9?pvGLgdhQ9fmw:g6*rP,|dE&tlP]2Z$y{6A%v3QHhTCA@M-@gj{YoD1'0zRCS~'X;TM3=mf7FOUb7wGx&rBH'6_wN0Ch}HQlQPkL/^G",3567] | [0.4430779814720154,"NaN",0.5607396960258484,0.6060205698013306]

3 of missing rows:
	null | null | null | ["-Infinity","-Infinity",0.35368847846984863,"NaN"]
	null | [{"key":624553287982967789,"value":3408273120430831589},{"key":1638632371533948088,"value":null},{"key":3953759512378454070,"value":7732938775053075298},{"key":4577248870163900892,"value":null},{"key":6332027456498242798,"value":5863674525104804843},{"key":7386588018962132295,"value":null},{"key":7779658487962081679,"value":1097175511491423178},{"key":8226199836062350230,"value":7469133982032194519}] | null | [0.08861453086137772,0.09776057302951813,0.34515634179115295,0.36273622512817383,0.4430779814720154,0.5200087428092957,0.5492228269577026,0.6060205698013306,0.7336945533752441,0.7447822690010071,0.7756975889205933,0.7796890139579773,0.8607743382453918,0.9125320315361023,0.9163637757301331,0.9761083126068115,"Infinity","Infinity","Infinity","Infinity","Infinity","Infinity","NaN","NaN"]
	null | [{"key":624553287982967789,"value":3408273120430831589},{"key":1638632371533948088,"value":null},{"key":3953759512378454070,"value":7732938775053075298},{"key":4577248870163900892,"value":null},{"key":6332027456498242798,"value":5863674525104804843},{"key":7386588018962132295,"value":null},{"key":7779658487962081679,"value":1097175511491423178},{"key":8226199836062350230,"value":7469133982032194519}] | ["NA2t-nB\"R^&s]it(d{:m.!m!{naRmi6Yj}a89G9\\BG#\\7+hG@)ECM9l}15^G;_chgs]\"yU;)URsHteO,mPF_&vP\"Zv@t<7yMO.V=h']:8ia5`m5z:Patj@[m?IX2KT^`6FdYi_EchaO9J;1O=a5Q#eWD)(X[Q54]x,(<Lw=a-d#`)v=>#[Zvh>:5)-<WUX!pFI:5bCnfS@T'c(H2!t+oD.P+Bb|<dr](^W*|,3.~>8bPq2>xFG?@>aWAO2i\"!nSA)BAB.8-8kmshVPgRF/RdCq`XRXz`>vut[\\(Ulj-n}^zp6!v]DH=?Y4Z9AWCA3E%o:~{jGu\"qb~oeIKeR7,/CqG[pt?A(-3Lap8[{_N.&u=2zkTg/7MlxL^%L*ST^osQD(rq<1=_u08U%:J)itU`#20d\"O}Fva^L2WS\"s@o[?6.Brv;;Q=Zt:kEqg<Zl7|d2h>x\\7Ye6)y~/S~M{bwukIf?#Fwjlt7FltpU<b44:f~5[^b^E2g1b==T;c^K[DKUC045$gm%l,N?sP<I/4{O7QB'f+(eM8u}0jYh0?i[AD{]HMh_M2@&`ZJ$Le}7jN$X@55$f#7iQvkj)F0T\\C8`*5\"IH8ZD/aC:ZIsppGgE|!g2wv)ks{TY$!%\\vnY8G/$]>[egt/77r31I$J/SGZSwwF\",0fXwdx*5tyzjL+('pq5fLe,m0W&P!to2fD9bE.+FEcVKo\\Gy(rM{{qO*Op9!Qk[4xhh$DcP,IHx}e`0T>A=G!\"SKC}'EP>qly,MgjG(W$N-,Z=}r&et)F!g`l>dl~\".^}e(}qfS\\W@H??C[Il._#,!@!_/C!I/U\\n:VAJ-6l}UHtST)IgKFSt8W)<d^Tk(lm)yF*DcJVCJ{aCKBYzpTj):E/I3\\{6w)Ov86v,BBuTG\"[LGNqJpq*u-R?Ir^ntJj0xs7|?mkULF,6H%A?SGY0=+X<HT%gHh=WMR&3sQ:EhY4N}1ucs#-$1`+iFR<JV=;}%nJ%J/A)xbVp\">g#gadGcDCAiug`hSp8yW|\\Jt.w=@\\?-/88\\j>~XIQoKE}~g5)Rt{Rh$dUTgi\\}Yq~#\"~lZRRF_hL-mS'Y:Qg&1`KzHgcKF+}s_DDvcFbSP,@=k6NHhIc-6kq;v$s5P,UhrJwXv2lo.|VwZDqsc0JNzdsk^NOfRsv{AL+p;bUV(!::=x$5K73s5T{-P'9CFV^U/+#f0=]<Jh{tSP3Kt{Xr+@`8Q3[A*qy+a/'M,<%zJ/c`zfPd4o,(6F3\\H-[AG5}tZ5ly>g&26&=Ct7M#TnYjb~veE.l,lNPjM\\lWL(&78DkLye)eq2KOE_`fR9p4#h:+_;e(Wm@9UZz&#/>mp%RCo^SVITp*ko-Oy&5WCO9N^dbR_~DXF?zDd9:5KB;nD!GR%a*DKk5`;c>m^wD$PTe7aL:n8id3exs\\CaEjWAzUnKdq[%`?,`Xe}c6HBa0Q$q!I3\"zwSl(CZR$..:bo&>vRj1.6ks{zZIqk7I\"ampQm>@-*/)}>TdbBR?d@n,)7[kG=n4Ka3~;}OUS}Ds>w?6j`f_|3-2[Od^^PPrC=)/,>r#/GpHxC0uZ$ALYtw2MmKC'Q@BhGW&eLFHoyt'hV#R\"Msdmq>_LNiof^hkEK|FJLRpC_A3Hpk.hr[3vOUAG(LTD*1{G|<XGb3||:%o'U]{@3*CGOQl;MZA^T\\~1e?ty#W1;nLTjmE!.58!`n5c],\\25\\#nW7TE'8){,ARv0^[IDQ7*6$c[&\"%xcj^yBO\\s#*9f\";jG%UtNE3tqJbFPtwk3Pb5ceJRW$/\"_VkPJM'3;sm<={qI0fW]]diMnwQW(tz==!PoMo|fM`'KD)@do(hvk<zQTh)k/Wsd<9!>K!UyR]<h3-G4~3K*B8nukg2$mcJt]]4=1upGewXw*+Vtrs:mXdr/G*?C@2u1o:[k/DZ5C}Z(u9tbf#7$5b\"5oGgN_42F7/3oP<#FG5Bm/w}Y4b6OIJHM|u:ArXw3p#1p8DP0&dj5S;B+]R*C%QPG&Md+|3%ayzfET^rQ?$B'OtZo7(Psx9$]@-W\"zy}+1FLi/opR5\\}|k_v(t5Z~><Y@}T:,KHT@z9@NwIhnSSP:fP}_qr:7DI[$|p[c:nS*L[YHn}Ir!x6kg$P%~:O5vT28|X&3,VPF#q)^X-lXo!*_vUvZQJGM32-2{`sRw`=1.(o1a!c:m{_,NJn.0e;OL;cv\"6_bXX}mK+G#H=egY(it-SFmB>^-&CvY9.Yy9'.$KE'W,$A@%*\"otCMBK4YxVB&0cpwFBec}G`/58G0JPNaXU%2OL}-4@n{|qT`/*^q>&M$p=rE6B,V-%Pq~WPoO{UQr#Rgr<7\\m(%tm+$TTs3llt3zm9?pvGLgdhQ9fmw:g6*rP,|dE&tlP]2Z$y{6A%v3QHhTCA@M-@gj{YoD1'0zRCS~'X;TM3=mf7FOUb7wGx&rBH'6_wN0Ch}HQlQPkL/^G",3567] | [0.4430779814720154,0.5607396960258484,0.6060205698013306,"NaN"]

Unexpected results
E20240213 09:09:14.601521 20587618 Exceptions.h:69] Line: /Users/kpai/src/velox/velox/exec/fuzzer/AggregationFuzzer.cpp:1097, Function:compareEquivalentPlanResults, Expression: assertEqualResults( expectedResult.value(), firstPlan->outputType(), {resultOrError.result}) Velox and reference DB results don't match, Source: RUNTIME, ErrorCode: INVALID_STATE
I20240213 09:09:14.664726 20587618 AggregationFuzzerBase.cpp:655] Persisted aggregation plans to /tmp/aggregate_fuzzer_repro/velox_aggregationVerifier_r1rbf7/plan_nodes
libc++abi: terminating due to uncaught exception of type facebook::velox::VeloxRuntimeError: Exception: VeloxRuntimeError
Error Source: RUNTIME
Error Code: INVALID_STATE
Reason: Velox and reference DB results don't match
Retriable: False
Expression: assertEqualResults( expectedResult.value(), firstPlan->outputType(), {resultOrError.result})
Function: compareEquivalentPlanResults
File: /Users/kpai/src/velox/velox/exec/fuzzer/AggregationFuzzer.cpp
@kgpai kgpai added bug Something isn't working fuzzer-found labels Feb 13, 2024
@kgpai kgpai self-assigned this Feb 13, 2024
@kgpai
Copy link
Contributor Author

kgpai commented Feb 13, 2024

An initial theory seemed to be that this was a difference in how Nan ordering is handled between Java and C++ , But that doesnt seem so with the following test :

Presto:
 select ARRAY_SORT(a) from  (values array[NAN(), INFINITY(), -INFINITY(), 1]) as T(a); 
— ["-Infinity", 1, "Infinity", "NaN"] 
Velox test: 
See P1184121984 
Result :  ArraySortTest.cpp:692] 0: 4 elements starting at 0 {-Infinity, 1, Infinity, NaN}
These results seem similar, so it might be something in min thats causing the above bug (There seems to be no special handling in array_sort).

@kgpai kgpai changed the title Presto aggregate function min(_,n) has a differences in ordering Presto aggregate function min(_,n) has a differences in ordering of "NaN" Feb 13, 2024
@mbasmanova
Copy link
Contributor

CC: @aditi-pandit

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

No branches or pull requests

3 participants