Skip to content

Fix error classification for array with null elements#25187

Merged
rschlussel merged 1 commit intoprestodb:masterfrom
Zhiying12:array-null-error
Jun 10, 2025
Merged

Fix error classification for array with null elements#25187
rschlussel merged 1 commit intoprestodb:masterfrom
Zhiying12:array-null-error

Conversation

@Zhiying12
Copy link
Contributor

@Zhiying12 Zhiying12 commented May 23, 2025

Description

Presto does not support array comparison with null elements. Previously, it was classified as internal InvalidFunctionArgumentException. It is converted as a user error by using PrestoException.

Motivation and Context

This unsupported operation was misclassified, resulting in inaccuracies.

Impact

No impact.

Test Plan

presto-cli/target/presto-cli-*-executable.jar --catalog hive --schema tpch --debug
presto:tpch> CREATE TABLE customers ( id INTEGER, name VARCHAR, phone_numbers ARRAY(VARCHAR) );
CREATE TABLE
presto:tpch> SELECT * FROM customers WHERE phone_numbers = array[NULL];
Query 20250603_225924_00027_fafa4 failed: ARRAY comparison not supported for arrays with null elements

Contributor checklist

  • Please make sure your submission complies with our contributing guide, in particular code style and commit standards.
  • PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced.
  • Documented new properties (with its default value), SQL syntax, functions, or other functionality.
  • If release notes are required, they follow the release notes guidelines.
  • Adequate tests were added if applicable.
  • CI passed.

Release Notes

Please follow release notes guidelines and fill in the release notes below.

== RELEASE NOTES ==

General Changes
* Fix error classification for unsupported array comparison with null elements, 
converting it as a user error.

@Zhiying12 Zhiying12 requested a review from a team as a code owner May 23, 2025 17:28
@Zhiying12 Zhiying12 requested a review from ZacBlanco May 23, 2025 17:28
Copy link
Collaborator

@kewang1024 kewang1024 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you also append the test result in the summary, thanks!

@ZacBlanco
Copy link
Contributor

ZacBlanco commented Jun 4, 2025

Since this is a user-facing change, please write a short release note in the PR description describing that this is now a user error

Copy link
Contributor

@rschlussel rschlussel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just fix the format of the release notes entry so the CI passes. you can look at some other prs with release notes for examples on what it should look liek

@Zhiying12
Copy link
Contributor Author

@rschlussel Thank you. CI has passed.

@rschlussel rschlussel merged commit 7587c15 into prestodb:master Jun 10, 2025
197 of 202 checks passed
@prestodb-ci prestodb-ci mentioned this pull request Jul 28, 2025
6 tasks
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.

5 participants