Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

handle InvalidCodePointException #7731

Closed
wants to merge 0 commits into from
Closed

handle InvalidCodePointException #7731

wants to merge 0 commits into from

Conversation

JoJoBizarreAdventure
Copy link
Contributor

A pull request for #7597
Handling cases for codePoint is 0xE000
Using keep decreasing result 0xD7FF as the modified code point instead of 0xDFFF

@cla-bot
Copy link

cla-bot bot commented Apr 24, 2021

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please submit the signed CLA to [email protected]. For more information, see https://github.com/trinodb/cla.

@JoJoBizarreAdventure
Copy link
Contributor Author

It seems there is a conflict. Shall I change the nCopies() one to using toAdd?

@cla-bot
Copy link

cla-bot bot commented Apr 24, 2021

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please submit the signed CLA to [email protected]. For more information, see https://github.com/trinodb/cla.

1 similar comment
@cla-bot
Copy link

cla-bot bot commented Apr 24, 2021

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please submit the signed CLA to [email protected]. For more information, see https://github.com/trinodb/cla.

Copy link
Member

@hashhar hashhar left a comment

Choose a reason for hiding this comment

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

Thanks.

Please squash the commits. We prefer to keep the fix and tests in the same commit to help when bisecting.

@@ -111,7 +111,12 @@ public static Slice varcharToCharSaturatedFloorCast(@LiteralParameter("y") long
return Slices.allocate(toIntExact(y));
}

codePoints.set(codePoints.size() - 1, codePoints.get(codePoints.size() - 1) - 1);
int lastCodePoint = codePoints.get(codePoints.size() - 1) - 1;
if (55296 <= lastCodePoint && lastCodePoint <= 57343) {
Copy link
Member

Choose a reason for hiding this comment

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

Either use constants if there are (or create them) or use the hex representation with a comment about the significance of these values.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, I will change it to hex reperesentation

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I'm not really familiar with git, I'm making try to make it right

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I create a new pull request at #7734
Please check if it is right this time @hashhar

@hashhar hashhar requested a review from skrzypo987 April 24, 2021 06:44
@cla-bot
Copy link

cla-bot bot commented Apr 24, 2021

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please submit the signed CLA to [email protected]. For more information, see https://github.com/trinodb/cla.

@cla-bot cla-bot bot added the cla-signed label Apr 24, 2021
@cla-bot
Copy link

cla-bot bot commented Apr 24, 2021

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please submit the signed CLA to [email protected]. For more information, see https://github.com/trinodb/cla.

@cla-bot cla-bot bot added cla-signed and removed cla-signed labels Apr 24, 2021
@hashhar hashhar removed the request for review from skrzypo987 April 24, 2021 10:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

2 participants