re-enable sort_query_fuzzer_runner

[adriangb]

• Related to SortQueryFuzzer found a failing case on main #16452

1. Re-enable test, verify CI fails, might need to run a couple times?
2. Revert TopK dynamic filter pushdown attempt 2 #15770 (suspected cause).
3. Verify CI doesn't fail after multiple runs.

[alamb]

Context for anyone interested: #16452 (comment)

[adriangb]

This is temporary until the upstream bug gets fixed in arrow, plus it's necessarily already in the dependency tree because arrow uses it.

[adriangb]

I think with these fixes to Display<ScalarValue> the tests will pass consistently.

I used this script to test:

#!/usr/bin/env python3importargparseimportsubprocessfromconcurrent.futuresimportThreadPoolExecutor, as_completedfromthreadingimportEventdefrun_test(command, run_num, total_runs, stop_event):
"""Run a single test and return result"""ifstop_event.is_set():
returnrun_num, "SKIPPED", Nonetry:
result=subprocess.run(command, shell=True, capture_output=True, text=True)
status="PASS"ifresult.returncode==0else"FAIL"print(f"Run {run_num}/{total_runs}: {status}")
returnrun_num, status, resultexceptExceptionase:
print(f"Run {run_num}/{total_runs}: ERROR - {e}")
returnrun_num, "ERROR", Nonedefmain():
parser=argparse.ArgumentParser(description="Run a command multiple times and report failure rate")
parser.add_argument("-P", "--parallel", type=int, default=1, help="Number of parallel jobs (default: 1)")
parser.add_argument("-n", "--runs", type=int, default=100, help="Number of runs (default: 100)")
parser.add_argument("-x", "--stop-on-failure", action="store_true", help="Stop at first failure")
parser.add_argument("command", nargs=argparse.REMAINDER, help="Command to run")
args=parser.parse_args()
command=" ".join(args.command)
print(f"Running command {args.runs} times with {args.parallel} parallel jobs...")
print(f"Command: {command}")
print("----------------------------------------")
stop_event=Event()
failures=0completed_runs=0failure_outputs= []
withThreadPoolExecutor(max_workers=args.parallel) asexecutor:
# Submit all jobsfutures= []
foriinrange(1, args.runs+1):
future=executor.submit(run_test, command, i, args.runs, stop_event)
futures.append(future)
# Process results as they completeforfutureinas_completed(futures):
run_num, status, result=future.result()
completed_runs+=1ifstatus=="FAIL"orstatus=="ERROR":
failures+=1ifresultand (result.stdoutorresult.stderr):
failure_outputs.append((run_num, result.stdout, result.stderr))
ifargs.stop_on_failure:
print(f"Stopping at first failure (run {run_num})")
stop_event.set()
# Cancel remaining futuresforfinfutures:
f.cancel()
breakprint("----------------------------------------")
print("Results:")
print(f"Total runs: {completed_runs}")
print(f"Failures: {failures}")
print(f"Passes: {completed_runs-failures}")
ifcompleted_runs>0:
failure_rate= (failures*100) /completed_runsprint(f"Failure rate: {failure_rate:.2f}%")
else:
print("Failure rate: 0%")
# Print failure outputsiffailure_outputs:
print("\n"+"="*50)
print("FAILURE OUTPUTS:")
print("="*50)
forrun_num, stdout, stderrinfailure_outputs:
print(f"\n--- Run {run_num} ---")
ifstdout:
print("STDOUT:")
print(stdout)
ifstderr:
print("STDERR:")
print(stderr)
if__name__=="__main__":
main()

And was able to run with no errors:

./run-test.py -P 10 -n 600 -x cargo test --package datafusion --test fuzz -- fuzz_cases::sort_query_fuzz::sort_query_fuzzer_runner --exact --show-output

I'm running a 1200 run to confirm now.

[adriangb]

I understand why but I do find it kind of strange that Literal::new() calls Display on ScalarValue. I wonder if we could just make the Field name "lit"?

[adriangb]

For context the failures reported here and here are both related to this overflow error fixed in this PR. If someone has seen a different error for sort_query_fuzzer_runner since #16465 was merged please share it!

[adriangb]

@AdamGS@alamb@blaginin could you please review?

[AdamGS]

LGTM. IDK if there's a precedence to formatting the error case as an empty string elsewhere in Datafusion. It seems like format_option! uses "NULL" as sort of a display sentinel value, maybe values like this need their own placeholder?

[alamb]

LGTM. IDK if there's a precedence to formatting the error case as an empty string elsewhere in Datafusion. It seems like format_option! uses "NULL" as sort of a display sentinel value, maybe values like this need their own placeholder?

I think @adriangb also fixed this in

• use 'lit' as the field name for literal values #16498

[alamb]

The code and fix looks good to me. Thank you for tracking this down @adriangb and @AdamGS

I am trying to verify that I can reproduce the error locally but so far I can't cause an error with main nor this branch. I'll report back if I am able to

Here is my reproducer (not as fancy as what you used)

set -e
foriin`seq 1 100`;doecho"*** Iteration $i"
cargo test --test fuzz -- sort_query_fuzzer_runner &
cargo test --test fuzz -- sort_query_fuzzer_runner &
cargo test --test fuzz -- sort_query_fuzzer_runner &
cargo test --test fuzz -- sort_query_fuzzer_runner &
cargo test --test fuzz -- sort_query_fuzzer_runner &
cargo test --test fuzz -- sort_query_fuzzer_runner &
cargo test --test fuzz -- sort_query_fuzzer_runner &
cargo test --test fuzz -- sort_query_fuzzer_runner &waitdone

[adriangb]

Should we go ahead and merge and get the test running again (or find out quickly with the many CI runs it's still broken)? Or do you want to wait for your local testing?

[alamb]

Should we go ahead and merge and get the test running again (or find out quickly with the many CI runs it's still broken)? Or do you want to wait for your local testing?

I think we should merge it

[alamb]

Thank you @adriangb