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

Reduce duplicated code involved in running a query #683

Merged
merged 2 commits into from
Jun 7, 2023

Conversation

andyleiserson
Copy link
Collaborator

In particular, this makes there be only one copy of the code that processes the input reports for an IPA query.

In particular, this makes there be only one copy of the code that
processes the input reports for an IPA query.
@andyleiserson
Copy link
Collaborator Author

This is scaled back a bit from what I had originally tried to do. I tried to make the query processing generic over the return type of the query, i.e., a query would return Vec<O: Serializable> instead of Box<dyn ProtocolResult>. But I ran into some issues doing that and in the interest of getting what we need now done, removed those changes.

while let Some(data) = input.next().await {
input_vec.extend(IPAInputRow::<F, MatchKey, BreakdownKey>::from_byte_slice(
&data.unwrap(),
));
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The big win is that there's only one copy of this code that does input.align and the subsequent processing of the inputs. It's only a few lines right now, but it's about to get a bit more involved.

&'a PrssEndpoint,
&'a Gateway,
ByteArrStream,
) -> Pin<Box<dyn Future<Output = QueryResult> + Send + 'a>>
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This box is needed only because having two type parameters for the function and the returned future prevents describing the HRTB for the 'a lifetime.

Copy link
Member

Choose a reason for hiding this comment

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

I'm still amazed at how you can operate with lifetime traits. I like rust, but hate having to deal with lifetimes. It's as though the language was designed to suck you in and then trap you in a Kafkaesque nightmare filled with unbalanced quotes.

&'a PrssEndpoint,
&'a Gateway,
ByteArrStream,
) -> Pin<Box<dyn Future<Output = QueryResult> + Send + 'a>>
Copy link
Member

Choose a reason for hiding this comment

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

I'm still amazed at how you can operate with lifetime traits. I like rust, but hate having to deal with lifetimes. It's as though the language was designed to suck you in and then trap you in a Kafkaesque nightmare filled with unbalanced quotes.

config: QueryConfig,
gateway: Gateway,
input: ByteArrStream,
) -> JoinHandle<QueryResult> {
match (config.query_type, config.field_type) {
Copy link
Member

Choose a reason for hiding this comment

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

This is a bit repetitious, but I think that this is a reasonable amount of repetition to manage.

field: FieldType,
impl<F, C, S, SB> IpaQuery<F, C, S>
where
C: UpgradableContext + Send,
Copy link
Collaborator

Choose a reason for hiding this comment

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

🙈

@akoshelev akoshelev merged commit 4164900 into private-attribution:main Jun 7, 2023
@andyleiserson andyleiserson deleted the runner branch June 8, 2023 00:15
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.

3 participants