Skip to content

vef: add stateful encoder to Field/Item#66

Merged
malone-at-work merged 6 commits intomainfrom
maloney/vdf/2
Mar 3, 2026
Merged

vef: add stateful encoder to Field/Item#66
malone-at-work merged 6 commits intomainfrom
maloney/vdf/2

Conversation

@malone-at-work
Copy link
Copy Markdown
Member

@malone-at-work malone-at-work commented Feb 27, 2026

TypeEncoder is a per-Field or Item object that holds encoding state across rows. Previously, every encode call allocated a new output buffer from mem_root; now a single scratch buffer (sized to persisted_length) is allocated once and reused for the lifetime of the Field's TABLE or the Item's query execution.

For VDF-based encode ops, the VDF call structures (vef_invalue_t, vef_vdf_args_t, vef_vdf_result_t) are pre-filled at construction time, so only the per-row fields (input pointer/length, result type) are touched on each call. A rarely-used overflow path handles the case where VDF output exceeds the pre-allocated buffer size.

TypeEncoder is lazily created via GetTypeEncoderFor and cached on the Field or Item. Item::cleanup() nulls the pointer between prepared-statement re-executions so a fresh encoder is allocated on the new thd->mem_root next execution.

The public encode API in util.h is simplified: callers no longer pass MEM_ROOT, field_name, or TypeContext — the encoder resolves these from the Field or Item itself.

@malone-at-work malone-at-work changed the title ef: add stateful TypeEncoder to Field and Item ef: add stateful TypeEncoder to Field/Item Feb 27, 2026
@malone-at-work malone-at-work changed the title ef: add stateful TypeEncoder to Field/Item ef: add stateful encoder to Field/Item Feb 27, 2026
@malone-at-work malone-at-work changed the title ef: add stateful encoder to Field/Item vef: add stateful encoder to Field/Item Feb 27, 2026
@malone-at-work malone-at-work force-pushed the maloney/vdf/2 branch 3 times, most recently from d1f7997 to b00a94c Compare March 2, 2026 01:42
@malone-at-work
Copy link
Copy Markdown
Member Author

I added another commit this morning, because for temp tables memory needs to be allocated on the TableShare's mem root not on the Table's.

Copy link
Copy Markdown
Member

@tomas-villagesql tomas-villagesql left a comment

Choose a reason for hiding this comment

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

approved, but would update one test as commented

PREPARE stmt FROM 'SELECT id FROM t1 WHERE val = ? ORDER BY id';

SET @p = '(1.0,2.0)';
EXECUTE stmt USING @p;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I would make these without decimal, as the test would work equally well if everything was encoded as a string. I.e make the strings not match, but complex match

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is the same feedback I always give, but missed in my own PR, thanks!

Comment thread villagesql/types/type_encoder.cc Outdated
Comment thread villagesql/types/type_encoder.cc Outdated
Comment thread villagesql/types/type_encoder.cc Outdated
Comment thread sql/field.h

private:
const villagesql::TypeContext *custom_type{nullptr};
villagesql::TypeEncoder *type_encoder_{nullptr};
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is unfortunate, because we will also need a decoder, and maybe other pointers. It would be nice to put them into a container, or ideally with the TypeContext, but I think that is a much larger change. Should we put a TODO here (villagesql, not beta).

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes, let's clean up in a future PR. It cannot go in the TypeContext, but maybe it can be packaged with it.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't see a TODO

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Now you do! ;-)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Item

Comment thread villagesql/types/util.cc Outdated
TypeEncoder ia a per-Field or Item object that holds encoding state
across rows. Previously, every encode call allocated a new output buffer
from mem_root; now a single scratch buffer (sized to persisted_length)
is allocated once and reused for the lifetime of the Field's TABLE or
the Item's query execution.

For VDF-based encode ops, the VDF call structures (vef_invalue_t,
vef_vdf_args_t, vef_vdf_result_t) are pre-filled at construction time,
so only the per-row fields (input pointer/length, result type) are
touched on each call. A rarely-used overflow path handles the case where
VDF output exceeds the pre-allocated buffer size.

TypeEncoder is lazily created via GetTypeEncoderFor and cached on the
Field or Item. Item::cleanup() nulls the pointer between
prepared-statement re-executions so a fresh encoder is allocated on the
new thd->mem_root next execution.

The public encode API in util.h is simplified: callers no longer pass
MEM_ROOT, field_name, or TypeContext — the encoder resolves these from
the Field or Item itself.
Comment thread villagesql/types/util.cc Outdated
Comment thread villagesql/types/type_encoder.cc Outdated
Comment thread sql/field.h

private:
const villagesql::TypeContext *custom_type{nullptr};
villagesql::TypeEncoder *type_encoder_{nullptr};
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't see a TODO

Comment thread sql/field.h

private:
const villagesql::TypeContext *custom_type{nullptr};
villagesql::TypeEncoder *type_encoder_{nullptr};
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Item

@malone-at-work malone-at-work merged commit e3780a9 into main Mar 3, 2026
3 of 4 checks passed
@malone-at-work malone-at-work deleted the maloney/vdf/2 branch March 3, 2026 19:45
@github-actions github-actions Bot locked and limited conversation to collaborators Mar 3, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants