Skip to content

feat(sql): unparse array_has as ANY for Postgres#20654

Merged
alamb merged 4 commits intoapache:mainfrom
vimeh:vinay/any-array-has-postgres
Mar 31, 2026
Merged

feat(sql): unparse array_has as ANY for Postgres#20654
alamb merged 4 commits intoapache:mainfrom
vimeh:vinay/any-array-has-postgres

Conversation

@vimeh
Copy link
Copy Markdown
Contributor

@vimeh vimeh commented Mar 2, 2026

Which issue does this PR close?

Rationale for this change

PostgreSQL does not have an array_has() function. When the SQL unparser emits array_has(haystack, needle) for the Postgres dialect, it produces invalid SQL. The idiomatic equivalent is needle = ANY(haystack).

What changes are included in this PR?

Adds a scalar_function_to_sql_overrides entry in PostgreSqlDialect that converts array_has(haystack, needle) to needle = ANY(haystack) via ast::Expr::AnyOp.

Are these changes tested?

Yep, with a unit test comparing default vs Postgres dialect output and a roundtrip integration test for = ANY() in a WHERE clause.

Are there any user-facing changes?

No breaking changes. array_has expressions are now unparsed as valid Postgres syntax when using PostgreSqlDialect.

@github-actions github-actions Bot added the sql SQL Planner label Mar 2, 2026
@vimeh vimeh changed the title sql: unparse array_has as ANY for Postgres fix(sql): unparse array_has as ANY for Postgres Mar 2, 2026
@vimeh vimeh changed the title fix(sql): unparse array_has as ANY for Postgres feat(sql): unparse array_has as ANY for Postgres Mar 2, 2026
Copy link
Copy Markdown
Contributor

@nuno-faria nuno-faria left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @vimeh.
cc: @sgrebnov since you are also familiar with unparsing.

Comment on lines +374 to +376
let [haystack, needle] = args else {
return Ok(None);
};
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: Since args must always contain [haystack, needle] in this case, I wonder if setting the else to unreachable!() would be clearer.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@vimeh would you like to respond to @nuno-faria before we merge this PR in?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Have replaced with an internal_err to match existing patterns I saw.

also went ahead and rebased

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks @vimeh.

@vimeh vimeh force-pushed the vinay/any-array-has-postgres branch from 544d6fa to 3781668 Compare March 30, 2026 18:56
vimeh added 2 commits March 30, 2026 11:57
PostgreSQL does not have an array_has() function — it uses the
`= ANY(array)` operator for array containment checks. Without this
dialect override, SQL unparsed for Postgres federation produces
invalid syntax. This converts array_has(haystack, needle) to
needle = ANY(haystack) in the PostgreSqlDialect.
Replace `Ok(None)` with `internal_err!` in the else branch of the
array_has argument destructure, since array_has always takes exactly
2 arguments and the branch is unreachable.
@vimeh vimeh force-pushed the vinay/any-array-has-postgres branch from 3781668 to 59eefa2 Compare March 30, 2026 18:57
@vimeh vimeh requested review from alamb and nuno-faria March 30, 2026 18:59
Copy link
Copy Markdown
Contributor

@nuno-faria nuno-faria left a comment

Choose a reason for hiding this comment

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

@vimeh there's a small problem with the formatting, but other than that LGTM.

@alamb
Copy link
Copy Markdown
Contributor

alamb commented Mar 31, 2026

I took the liberty of merging up from main and pushing a fix for fmt

@alamb alamb added this pull request to the merge queue Mar 31, 2026
@alamb
Copy link
Copy Markdown
Contributor

alamb commented Mar 31, 2026

Thanks again @nuno-faria and @vimeh

Merged via the queue into apache:main with commit e75ed5b Mar 31, 2026
34 checks passed
Rich-T-kid pushed a commit to Rich-T-kid/datafusion that referenced this pull request Apr 21, 2026
## Which issue does this PR close?

<!--
We generally require a GitHub issue to be filed for all bug fixes and
enhancements and this helps us generate change logs for our releases.
You can link an issue to this PR using the GitHub syntax. For example
`Closes apache#123` indicates that this PR will close issue apache#123.
-->

- Closes apache#20653 .

## Rationale for this change

<!--
Why are you proposing this change? If this is already explained clearly
in the issue then this section is not needed.
Explaining clearly why changes are proposed helps reviewers understand
your changes and offer better suggestions for fixes.
-->

PostgreSQL does not have an `array_has()` function. When the SQL
unparser emits `array_has(haystack, needle)` for the Postgres dialect,
it produces invalid SQL. The idiomatic equivalent is `needle =
ANY(haystack)`.

## What changes are included in this PR?

<!--
There is no need to duplicate the description in the issue here but it
is sometimes worth providing a summary of the individual changes in this
PR.
-->

Adds a `scalar_function_to_sql_overrides` entry in `PostgreSqlDialect`
that converts `array_has(haystack, needle)` to `needle = ANY(haystack)`
via `ast::Expr::AnyOp`.

## Are these changes tested?

<!--
We typically require tests for all PRs in order to:
1. Prevent the code from being accidentally broken by subsequent changes
2. Serve as another way to document the expected behavior of the code

If tests are not included in your PR, please explain why (for example,
are they covered by existing tests)?
-->

Yep, with a unit test comparing default vs Postgres dialect output and a
roundtrip integration test for `= ANY()` in a WHERE clause.

## Are there any user-facing changes?

<!--
If there are user-facing changes then we may require documentation to be
updated before approving the PR.
-->

<!--
If there are any breaking changes to public APIs, please add the `api
change` label.
-->

No breaking changes. `array_has` expressions are now unparsed as valid
Postgres syntax when using `PostgreSqlDialect`.

---------

Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

sql SQL Planner

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unparse array_has as = ANY() for PostgreSQL dialect

3 participants