Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[query] Move LoweredTableReaderCoercer into ExecuteContext #14696

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ehigham
Copy link
Member

@ehigham ehigham commented Sep 20, 2024

Refactored table reader coercion and caching mechanism.

What changed?

  • Removed shouldCacheQueryInfo method from Backend class
  • Introduced CoercerCache in ExecuteContext
  • Refactored LoweredTableReader.makeCoercer to return a function instead of a class
  • Removed local caching in GenericTableValue and LoweredTableReader

Why make this change?

This change aims to optimize table reader coercion by:

  • Centralizing caching logic in ExecuteContext
  • Allowing more flexible caching strategies across different backend implementations

@ehigham ehigham force-pushed the ehigham/http-like-rpc branch from f2c39a6 to 2f3f122 Compare October 1, 2024 19:45
@ehigham ehigham force-pushed the ehigham/ctx-coercer-cache branch from 16d54bd to 82cd91b Compare October 1, 2024 19:45
@ehigham ehigham force-pushed the ehigham/http-like-rpc branch from 2f3f122 to b02c83b Compare October 1, 2024 20:03
@ehigham ehigham force-pushed the ehigham/ctx-coercer-cache branch from 82cd91b to 63191ba Compare October 1, 2024 20:03
@ehigham ehigham force-pushed the ehigham/http-like-rpc branch from b02c83b to 20e6520 Compare October 8, 2024 19:19
@ehigham ehigham force-pushed the ehigham/ctx-coercer-cache branch from 63191ba to dded098 Compare October 8, 2024 19:19
@ehigham ehigham force-pushed the ehigham/http-like-rpc branch from 20e6520 to e66b858 Compare October 8, 2024 20:30
@ehigham ehigham force-pushed the ehigham/ctx-coercer-cache branch from dded098 to 4f3a6e3 Compare October 8, 2024 20:30
@ehigham ehigham force-pushed the ehigham/http-like-rpc branch from e66b858 to 49bdf73 Compare October 16, 2024 20:02
@ehigham ehigham force-pushed the ehigham/ctx-coercer-cache branch from 4f3a6e3 to db7595e Compare October 16, 2024 20:02
@ehigham ehigham force-pushed the ehigham/http-like-rpc branch from 49bdf73 to 0ce4a09 Compare October 16, 2024 21:30
@ehigham ehigham force-pushed the ehigham/ctx-coercer-cache branch from db7595e to 9615ece Compare October 16, 2024 21:30
@ehigham ehigham force-pushed the ehigham/http-like-rpc branch from 0ce4a09 to 3a671ee Compare October 17, 2024 15:00
@ehigham ehigham force-pushed the ehigham/ctx-coercer-cache branch from 9615ece to 1951717 Compare October 17, 2024 15:00
@ehigham ehigham force-pushed the ehigham/http-like-rpc branch from 3a671ee to ee01140 Compare October 21, 2024 15:24
@ehigham ehigham force-pushed the ehigham/ctx-coercer-cache branch from 1951717 to 3f89fe2 Compare October 21, 2024 15:25
@ehigham ehigham force-pushed the ehigham/http-like-rpc branch from ee01140 to 22c3b9b Compare October 21, 2024 18:50
@ehigham ehigham force-pushed the ehigham/ctx-coercer-cache branch from 3f89fe2 to 1d6e424 Compare October 21, 2024 18:50
@ehigham ehigham force-pushed the ehigham/ctx-persisted-ir branch from 0ab8b01 to 9d5cf6c Compare January 31, 2025 19:12
@ehigham ehigham force-pushed the ehigham/ctx-coercer-cache branch from cf11bbe to aa8477e Compare January 31, 2025 19:12
@ehigham ehigham force-pushed the ehigham/ctx-persisted-ir branch from 9d5cf6c to dde4ab0 Compare January 31, 2025 19:27
@ehigham ehigham force-pushed the ehigham/ctx-coercer-cache branch from aa8477e to 490b5a6 Compare January 31, 2025 19:27
@ehigham ehigham force-pushed the ehigham/ctx-persisted-ir branch from dde4ab0 to 071d613 Compare January 31, 2025 21:21
@ehigham ehigham force-pushed the ehigham/ctx-coercer-cache branch from 490b5a6 to fcdedec Compare January 31, 2025 21:21
@ehigham ehigham force-pushed the ehigham/ctx-persisted-ir branch from 071d613 to ea9666d Compare February 3, 2025 17:20
@ehigham ehigham force-pushed the ehigham/ctx-coercer-cache branch from fcdedec to b0c0615 Compare February 3, 2025 17:20
@ehigham ehigham force-pushed the ehigham/ctx-persisted-ir branch from ea9666d to 7d6b30f Compare February 3, 2025 18:00
@ehigham ehigham force-pushed the ehigham/ctx-coercer-cache branch from b0c0615 to 1fbfbbd Compare February 3, 2025 18:01
@ehigham ehigham force-pushed the ehigham/ctx-persisted-ir branch from 7d6b30f to bd3988a Compare February 3, 2025 20:51
@ehigham ehigham force-pushed the ehigham/ctx-coercer-cache branch from 1fbfbbd to d7d8ab1 Compare February 3, 2025 20:51
@ehigham ehigham force-pushed the ehigham/ctx-persisted-ir branch from bd3988a to 9397c47 Compare February 4, 2025 22:44
@ehigham ehigham force-pushed the ehigham/ctx-coercer-cache branch from d7d8ab1 to 8f40db1 Compare February 4, 2025 22:44
@ehigham ehigham force-pushed the ehigham/ctx-persisted-ir branch from 9397c47 to aaf021a Compare February 6, 2025 18:44
@ehigham ehigham force-pushed the ehigham/ctx-coercer-cache branch from 8f40db1 to c2882b7 Compare February 6, 2025 18:44
@ehigham ehigham force-pushed the ehigham/ctx-persisted-ir branch from aaf021a to d8a765e Compare February 20, 2025 16:50
@ehigham ehigham force-pushed the ehigham/ctx-coercer-cache branch from c2882b7 to e226050 Compare February 20, 2025 16:50
@ehigham ehigham force-pushed the ehigham/ctx-persisted-ir branch from d8a765e to fe61d6d Compare February 20, 2025 17:28
@ehigham ehigham force-pushed the ehigham/ctx-coercer-cache branch from e226050 to 4566500 Compare February 20, 2025 17:28
@ehigham ehigham force-pushed the ehigham/ctx-coercer-cache branch from 4566500 to e218589 Compare February 27, 2025 22:16
@ehigham ehigham force-pushed the ehigham/ctx-persisted-ir branch from fe61d6d to 2c24fd9 Compare February 27, 2025 22:17
Base automatically changed from ehigham/ctx-persisted-ir to main February 28, 2025 20:06
@ehigham ehigham force-pushed the ehigham/ctx-coercer-cache branch from e218589 to 0c246e1 Compare March 3, 2025 17:55
Copy link
Collaborator

@patrick-schultz patrick-schultz left a comment

Choose a reason for hiding this comment

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

Nice change. While I was looking into what LoweredTableReaderCoercer does, I noticed it's only used in the VCF and Plink readers. That makes me think we could get rid of LoweredTableReaderCoercer and GenericTableValue completely if we refactored those readers to create TableStages directly, probably using registered functions to do the actual parsing logic. But thats just something to think about for the future.

@ehigham ehigham force-pushed the ehigham/ctx-coercer-cache branch from 0c246e1 to 52480a9 Compare March 19, 2025 19:13
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