vtgate: list only the problem values when insert fails due to vindex.Verify

[mingjianliu]

vtgate: list only the problem values when insert fails due to vindex.Verify

Before this change a batch insert failure due to a vindex Verify error would include all the values given for the vindex column for all rows inserted: both valid and invalid.

That can be confusing when many or most values are valid. Even if you understand that some values are valid in the error, sorting out the valid vs invalid values from the error message can be a pain.

Printing only the invalid values makes the error much more clear.

Signed-off-by: Mingjian Liu mingjianliu@pinterest.com

[dweitzman]

The change looks good to me. Maybe the commit/PR description could be more descriptive? As an example:

vtgate: list only the problem values when insert fails due to vindex.Verify

Before this change a batch insert failure due to a vindex Verify error would include all the values given for the vindex column for all rows inserted: both valid and invalid.

That can be confusing when many or most values are valid. Even if you understand that some values are valid in the error, sorting out the valid vs invalid values from the error message can be a pain.

Printing only the invalid values makes the error much more clear.

[harshit-gangal]

LGTM

[mingjianliu]

The change looks good to me. Maybe the commit/PR description could be more descriptive? As an example:

vtgate: list only the problem values when insert fails due to vindex.Verify
Before this change a batch insert failure due to a vindex Verify error would include all the values given for the vindex column for all rows inserted: both valid and invalid.
That can be confusing when many or most values are valid. Even if you understand that some values are valid in the error, sorting out the valid vs invalid values from the error message can be a pain.
Printing only the invalid values makes the error much more clear.

Thank you so much for that! I just copied your description this time. Learned from it!

[deepthi]

@dweitzman @harshit-gangal this PR has been approved but not merged. Is there a reason it has been kept open?