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

SNOW-918949: Add row index information to all exceptions #590

Merged
merged 45 commits into from
Sep 28, 2023

Conversation

sfc-gh-tzhang
Copy link
Contributor

@sfc-gh-tzhang sfc-gh-tzhang commented Sep 20, 2023

Add row index information to all exceptions, also unify the format so it can be easily parsed

@sfc-gh-tzhang sfc-gh-tzhang requested review from a team as code owners September 20, 2023 22:51
@@ -210,8 +210,8 @@ private float addRow(
throw new SFException(
Copy link
Contributor

Choose a reason for hiding this comment

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

Alternatively, is it worth having a list of longs that we store in a SFException that can be accessed via something like getRowIndexes? This will reduce the need to parse an error message which is error prone.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We already doing that with CONTINUE, this is for ABORT where we thrown an exception directly

Copy link
Collaborator

@sfc-gh-rcheng sfc-gh-rcheng left a comment

Choose a reason for hiding this comment

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

lgtm. small nit: i think insertRowsCurrIndex is more descriptive for the exception and log parsing than Row Index

Copy link
Collaborator

@sfc-gh-japatel sfc-gh-japatel left a comment

Choose a reason for hiding this comment

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

lgtm!

@sfc-gh-japatel
Copy link
Collaborator

lgtm. small nit: i think insertRowsCurrIndex is more descriptive for the exception and log parsing than Row Index

+1

@sfc-gh-tzhang
Copy link
Contributor Author

lgtm. small nit: i think insertRowsCurrIndex is more descriptive for the exception and log parsing than Row Index

+1

I updated everything to rowIndex, I think it's better than insertRowsCurrIndex

@sfc-gh-tzhang sfc-gh-tzhang merged commit 6a171c1 into master Sep 28, 2023
@sfc-gh-tzhang sfc-gh-tzhang deleted the tzhang-si-fixes branch September 28, 2023 01:39
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.

5 participants