Skip to content

refactor: backend execute_query/3 returns Adaptor.QueryResult#3317

Merged
Ziinc merged 2 commits intoLogflare:mainfrom
msmithstubbs:refactor/adaptor-query-result
Mar 31, 2026
Merged

refactor: backend execute_query/3 returns Adaptor.QueryResult#3317
Ziinc merged 2 commits intoLogflare:mainfrom
msmithstubbs:refactor/adaptor-query-result

Conversation

@msmithstubbs
Copy link
Copy Markdown
Contributor

Add Logflare.Backends.Adaptor.QueryResult struct to provide a consistent response shape for execute_query

Part of #3307

@msmithstubbs msmithstubbs force-pushed the refactor/adaptor-query-result branch from 45082bb to 148ccae Compare March 27, 2026 00:53
@msmithstubbs msmithstubbs marked this pull request as ready for review March 27, 2026 01:06
@msmithstubbs msmithstubbs marked this pull request as draft March 27, 2026 04:50
@msmithstubbs msmithstubbs marked this pull request as ready for review March 27, 2026 06:05
iex> execute_query(souce_backend, from(s in "log_event_..."))
{:ok, [%{...}]}
iex> execute_query(source_backend, from(s in "log_event_..."))
{:ok, %Logflare.Backends.Adaptor.QueryResult{rows: [%{...}]}}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Doesn't need to be fully qualified since it is example docstring, not a doctest

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done.

@type t :: t(map())
@type t(meta) :: %__MODULE__{
rows: [term()],
meta: meta
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We lost the benefits of having it as a struct if we were to use a generic meta key. Would prefer the keys to be directly on the struct so that we don't need to hunt for the keys in the adaptor modules.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I've flattened all the keys into the struct. Keys that are adaptor specific, such as bq_params and total_bytes_processed, are set to :not_supported by default.

@msmithstubbs msmithstubbs force-pushed the refactor/adaptor-query-result branch 2 times, most recently from d7225ee to d6c836b Compare March 30, 2026 23:44
@msmithstubbs msmithstubbs requested a review from Ziinc March 30, 2026 23:49
@msmithstubbs msmithstubbs force-pushed the refactor/adaptor-query-result branch from d6c836b to a7a73ea Compare March 31, 2026 00:02
@msmithstubbs msmithstubbs force-pushed the refactor/adaptor-query-result branch from a7a73ea to 7107b8b Compare March 31, 2026 00:14
@Ziinc Ziinc merged commit dddb54d into Logflare:main Mar 31, 2026
12 checks passed
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