On Wednesday, April 21, 2021 1:24 PM, Peter Smith <smithpb2250@gmail.com> Wrote
>I tried playing a bit with your psql patch V5 and I did not find any
>problems - it seemed to work as advertised.
>
>Below are a few code review comments.
Thanks for you review. I've updated the patch to V6 according to your comments.
>1. Patch applies with whitespace warnings.
Fixed.
>2. Unrelated "code tidy" fixes maybe should be another patch?
Agreed. Will post this modification on another thread.
>3. Unnecessary NULL check?
Agreed. NULL check removed.
>4. Memory not freed in multiple places?
oops. Memory free added.
>5. strncmp replacement?
Agreed. Thanks for your advice. Since this modification has little relation with my patch here.
I will merge this with comment(2) and push this on another patch.
>6. byte_length == 0?
>The byte_length was not being checked before, so why is the check needed now?
We need to make sure the empty input to be case sensitive as before(HEAD).
For example
CREATE TABLE onetab1 (f1 int);
update onetab1 SET [tab]
Without the check of "byte_length == 0", pg_strdup_keyword_case will make the column name "f1" to be upper case "F1".
Namely, the output will be " update onetab1 SET F1" which is not so good.
I added some tab tests for this empty input case, too.
>7. test typo "ralation"
>8. test typo "case-insensitiveq"
Thanks, typo fixed.
Any further comment is very welcome.
Regards,
Tang