Skip to content

Conversation

@xiedeyantu
Copy link
Member

See CALCITE-6066

Referring to #3489, it appears to be complete, but there are currently conflicts with merging into the main branch, and it hasn't been maintained for a long time. I've resubmitted this PR.

Copy link
Member

@xuzifu666 xuzifu666 left a comment

Choose a reason for hiding this comment

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

Compared to the previous implementation, the overall implementation seems OK. I've left some minor comments.

Copy link
Member

@xuzifu666 xuzifu666 left a comment

Choose a reason for hiding this comment

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

LGTM

f0.checkFails("^hypot(3, 4)^",
"No match found for function signature HYPOT\\(<NUMERIC>, <NUMERIC>\\)",
false);
final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.SPARK);
Copy link
Member

Choose a reason for hiding this comment

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

This PR also supports ClickHouse, so should the Library section also include ClickHouse?

Copy link
Member Author

Choose a reason for hiding this comment

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

Addressed

Copy link
Contributor

@NobiGo NobiGo left a comment

Choose a reason for hiding this comment

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

LGTM

@xiedeyantu xiedeyantu added the LGTM-will-merge-soon Overall PR looks OK. Only minor things left. label Jan 4, 2026
@sonarqubecloud
Copy link

sonarqubecloud bot commented Jan 4, 2026

@xuzifu666 xuzifu666 merged commit 17f2f97 into apache:main Jan 5, 2026
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

LGTM-will-merge-soon Overall PR looks OK. Only minor things left.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants