Skip to content

fix: Ignore arrow conversion when transformation is not used#313

Merged
piket merged 6 commits into
masterfrom
refactor-optional-arrow
Nov 21, 2025
Merged

fix: Ignore arrow conversion when transformation is not used#313
piket merged 6 commits into
masterfrom
refactor-optional-arrow

Conversation

@piket
Copy link
Copy Markdown
Collaborator

@piket piket commented Nov 5, 2025

What this PR does / why we need it:

Currently, results from an online store are always transformed into arrow arrays for use with the transformation service even if no transformation is requested. Later those arrow arrays are then transformed back into the proto values for the response. This adds unnecessary complexity and computation for the vast majority of use cases.

Which issue(s) this PR fixes:

This change checks whether transformations are needed and if not completely skips the arrow transformations.

Misc

Ran the Benchmark tests locally to generate these results:

BenchmarkTransposeFeatureRowsIntoColumnsWithArrowConversion 	12		70	  16736909 ns/op
BenchmarkTransposeFeatureRowsIntoColumnsWithoutArrowConversion 	12 		98	  11914213 ns/op
BenchmarkFullLoopArrowConversion 								12    	60	  18241852 ns/op

@piket piket force-pushed the refactor-optional-arrow branch from 2dbdfe3 to d319b29 Compare November 5, 2025 21:22
@piket piket force-pushed the refactor-optional-arrow branch from 614319c to ac3ee38 Compare November 12, 2025 22:21
@piket piket force-pushed the refactor-optional-arrow branch from ac3ee38 to a94424e Compare November 12, 2025 22:30
@piket piket force-pushed the refactor-optional-arrow branch from 83f94ef to 4ed695c Compare November 18, 2025 19:38
Copy link
Copy Markdown
Collaborator

@EXPEbdodla EXPEbdodla left a comment

Choose a reason for hiding this comment

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

LGTM. Unit test failures seems intermittent.

@piket piket merged commit 1fa0360 into master Nov 21, 2025
54 of 59 checks passed
@piket piket deleted the refactor-optional-arrow branch November 21, 2025 18:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants