Skip to content

Conversation

@amaliujia
Copy link
Contributor

@amaliujia amaliujia commented Jun 29, 2023

What changes were proposed in this pull request?

Moving some QueryExecutionErrors that are used by data types to sql/api and name those as DataType erros so that DataType can use those if DataType only stay in sql/api module.

Why are the changes needed?

Towards a simpler DataType interface.

Does this PR introduce any user-facing change?

No

How was this patch tested?

Existing test

@amaliujia
Copy link
Contributor Author

@cloud-fan @hvanhovell

@amaliujia amaliujia force-pushed the datatype_more_refactors branch from c3c39c1 to 1d9c33c Compare June 29, 2023 23:28
Copy link
Contributor

Choose a reason for hiding this comment

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

Do other similar items need to be moved? eg:
QueryExecutionErrors.unscaledValueTooLargeForPrecisionError(this, precision, scale)
It seems that it is only called in DataType.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For those we haven't moved, they are using SQLConf inside and we are still discussing how to better deal withSQLConf now.

sql/api/pom.xml Outdated
Copy link
Member

Choose a reason for hiding this comment

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

Which one requires this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

def outOfDecimalTypeRangeError(str: UTF8String): SparkArithmeticException

Copy link
Contributor Author

Choose a reason for hiding this comment

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

UTF8String stays into spark-unsafe.

Copy link
Contributor

Choose a reason for hiding this comment

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

DataTypeErrors?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@amaliujia amaliujia force-pushed the datatype_more_refactors branch from fa4f479 to 208e5c6 Compare June 30, 2023 17:02
@MaxGekk
Copy link
Member

MaxGekk commented Jul 1, 2023

@amaliujia Could you adjust PR title and description according to your recent commit, please.

@amaliujia amaliujia changed the title [SPARK-44254][SQL] Move QueryExecutionErrors to sql/api [SPARK-44254][SQL] Move QueryExecutionErrors that used by DataType to sql/api as DataTypeErrors Jul 2, 2023
@amaliujia
Copy link
Contributor Author

@MaxGekk tried to update title and description.

@MaxGekk
Copy link
Member

MaxGekk commented Jul 2, 2023

+1, LGTM. Merging to master.
Thank you, @amaliujia and @cloud-fan @panbingkun for review.

@MaxGekk MaxGekk closed this in cf852b2 Jul 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants