gpdb icon indicating copy to clipboard operation
gpdb copied to clipboard

[WIP] Improve gp_hyperloglog codes and add tests for it.

Open higuoxing opened this issue 2 years ago • 1 comments

This patch helps remove redundant base64 encoding and decoding functions in gp_hyperloglog.c and add test cases for gp_hyperloglog.

  1. Postgres upstream provides the implementation of base64 encoding and decoding utilities. The difference between Postgres's base64 and the original base64 is that the original encoding function obeys the 76-character per-line rule (it inserts a \n to the encoded string every 76-character). I think it doesn't matter whether to start a new line every 76-characters and it's good to remove redundant codes.

  2. Test cases for gp_hyperloglog are added in this patch (I don't know why there isn't any test for it).

Here are some reminders before you submit the pull request

  • [x] Add tests for the change
  • [x] Pass make installcheck

higuoxing avatar Mar 24 '22 04:03 higuoxing

Looks Good. Why WIP?

kainwen avatar Mar 30 '22 13:03 kainwen

Hi, interesting PR, any plan to make progress?

kainwen avatar Sep 22 '22 03:09 kainwen

Hi, interesting PR, any plan to make progress?

Yes. My concern is that whether it's ok to print out the base64 string without obeying the 76 characters/line rule. Both of them are legal.

image https://en.wikipedia.org/wiki/Base64

higuoxing avatar Sep 22 '22 07:09 higuoxing