Skip to content

Adds support for ANSI mode in negative function#20189

Open
SubhamSinghal wants to merge 2 commits intoapache:mainfrom
SubhamSinghal:support-ansi-mode-for-negative-function
Open

Adds support for ANSI mode in negative function#20189
SubhamSinghal wants to merge 2 commits intoapache:mainfrom
SubhamSinghal:support-ansi-mode-for-negative-function

Conversation

@SubhamSinghal
Copy link
Contributor

Which issue does this PR close?

#20034

Rationale for this change

ANSI mode support for negative function

What changes are included in this PR?

Added support for ANSI mode for negative function

Are these changes tested?

yes through UT

Are there any user-facing changes?

yes, adds ANSI support to existing function.

@github-actions github-actions bot added sqllogictest SQL Logic Tests (.slt) spark labels Feb 6, 2026
Copy link
Contributor

@comphead comphead left a comment

Choose a reason for hiding this comment

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

Thanks @SubhamSinghal I'll check it soon, however ansi mode now makes me think how to better structure tests. What I mean is ANSI edge cases are rare and most of cases should have same results no matter of ANSI flag. That can generate test scenarios duplication which would be nice to avoid

@SubhamSinghal
Copy link
Contributor Author

@comphead Modified UTs to contain only overflow tests in ANSI mode.

let result: PrimitiveArray<Int8Type> = if enable_ansi_mode {
array.try_unary(|x| {
x.checked_neg().ok_or_else(|| {
ArrowError::ComputeError(format!(
Copy link
Contributor

Choose a reason for hiding this comment

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

lets use
exec_err! to be in sync with other functions

let array = array.as_primitive::<Decimal128Type>();
let result: PrimitiveArray<Decimal128Type> =
array.unary(|x| x.wrapping_neg());
let result: PrimitiveArray<Decimal128Type> = if enable_ansi_mode {
Copy link
Contributor

Choose a reason for hiding this comment

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

I would suggest having a local macro or generic to reduce boilerplate

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

spark sqllogictest SQL Logic Tests (.slt)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants