Skip to content

feat: Add support for Spark Acosh, Asinh, Atanh math expressions#3787

Merged
andygrove merged 4 commits intoapache:mainfrom
rafafrdz:feat/add-acosh-asinh-atanh-functions
May 5, 2026
Merged

feat: Add support for Spark Acosh, Asinh, Atanh math expressions#3787
andygrove merged 4 commits intoapache:mainfrom
rafafrdz:feat/add-acosh-asinh-atanh-functions

Conversation

@rafafrdz
Copy link
Copy Markdown
Contributor

@rafafrdz rafafrdz commented Mar 25, 2026

Summary

  • Add support for Spark's Acosh, Asinh, and Atanh inverse hyperbolic trigonometric expressions
  • Delegates to DataFusion's built-in acosh, asinh, atanh functions via CometScalarFunction
  • No native Rust code needed — DataFusion's function registry handles execution
  • SQL tests added covering column values, literal arguments, and edge cases (NaN, Infinity, NULL, out-of-domain)

Test plan

  • SQL tests added: acosh.sql, asinh.sql, atanh.sql
  • CometSqlFileTestSuite verified — all 6 tests pass (2 per function × dictionary=false/true)

Related to #2084

rafafrdz and others added 3 commits March 25, 2026 07:23
Add inverse hyperbolic trigonometric functions by delegating to
DataFusion's built-in implementations via CometScalarFunction.
Includes SQL tests for column and literal arguments with edge cases.
-- specific language governing permissions and limitations
-- under the License.

-- ConfigMatrix: parquet.enable.dictionary=false,true
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

No need to test with dictionary encoding. Dictionaries are unpacked before expressions are evaluated.

Suggested change
-- ConfigMatrix: parquet.enable.dictionary=false,true

-- specific language governing permissions and limitations
-- under the License.

-- ConfigMatrix: parquet.enable.dictionary=false,true
Copy link
Copy Markdown
Member

@andygrove andygrove May 5, 2026

Choose a reason for hiding this comment

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

No need to test with dictionary encoding. Dictionaries are unpacked before expressions are evaluated.

Suggested change
-- ConfigMatrix: parquet.enable.dictionary=false,true

-- specific language governing permissions and limitations
-- under the License.

-- ConfigMatrix: parquet.enable.dictionary=false,true
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

No need to test with dictionary encoding. Dictionaries are unpacked before expressions are evaluated.

Suggested change
-- ConfigMatrix: parquet.enable.dictionary=false,true

Copy link
Copy Markdown
Member

@andygrove andygrove left a comment

Choose a reason for hiding this comment

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

LGTM pending CI. Thanks @rafafrdz

@andygrove andygrove merged commit a4f0229 into apache:main May 5, 2026
138 checks passed
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.

2 participants