+ * Note the constrain buf->position <= buf->limit may be broken + * at initialization. Make sure that the constrain is satisfied + * when consume chars.
Also, this comment might be better at the top the whole function?
The constraint is broken in the if branch, so I think put this comment in the branch
is more precise.
The check if (iter->buf != iter->fetch_datum_iterator->buf) is what we need to do for the compressed case. Could we use this directly instead of having a separate state variable iter->compressed, with a macro like this? #define TOAST_ITER_COMPRESSED(iter) \ (iter->buf != iter->fetch_datum_iterator->buf)
The logic of the macro may be hard to understand, so I think it's ok to just check the compressed state variable.
+ * If "ctrlc" field in iterator is equal to INVALID_CTRLC, it means that + * the field is invalid and need to read the control byte from the + * source buffer in the next iteration, see pglz_decompress_iterate(). + */ +#define INVALID_CTRLC 8
I think the macro might be better placed in pg_lzcompress.h, and for consistency used in pglz_decompress(). Then the comment can be shorter and more general. With my additional comment in init_detoast_iterator(), hopefully it will be clear to readers.
The main role of this macro is to explain the iterator's "ctrlc" state, IMO it's reasonable to put
the macro and definition of de-TOAST iterator together.
Thanks for your suggestion, I have updated the patch.
