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

Protocol v1 #18

Closed
wants to merge 6 commits into from
Closed

Protocol v1 #18

wants to merge 6 commits into from

Conversation

bgaidioz
Copy link
Contributor

@bgaidioz bgaidioz commented Jan 9, 2025

No description provided.

val rows2 = collectRows(blockingStub.executeTable(req2))
rows2.size shouldBe 50

// This confirms the narrower qualifier is applied on top of the already-complete cache,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

How does the test confirms the predicate was applied "on top of the already-complete cache"?

if (validCandidates.isEmpty) {
None
} else {
// 3) Pick the cache with the fewest `cd.quals.size`
Copy link
Contributor Author

Choose a reason for hiding this comment

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

All candidates are valid at that point. If picking the one that had the smallest quals.size, it's going to require more filtering. We likely want the smallest cache, and apply the minimum number of predicates on top, meaning the one with the largest number of qualifiers.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Or is there a way to pick the smallest cache (in number of rows)? That would be a decent criteria:

A query like lineitem WHERE date > X AND status = Y and country = Z has three predicates and can be served by caches:

  • WHERE date > X,
  • WHERE status = Y and country = Z

It's not obvious the one with more predicates is better. Perhaps the date predicate was returning very few rows.

columnFiltered.flatMap { ce =>
QualSelectivityAnalyzer
.differenceIfMoreSelective(ce.definition.quals, requestedQuals)
.map { diff => (ce, diff) }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

At that level we don't know if predicates were applied by the target DAS. We can't assume applying the diff qualifiers is enough.

* predicates being considered false when not supported (in cache
filtering),
* edited cache logic to accept a cache that implements the exact same
predicates,
* edited `MessageAdapter` related code because it appeared to be a
singleton reused for all tasks,
* missing filter on table name,
* cleared two warning stack traces indicating bad practices or missing
calls to `close`,
* made server read its parameters from config,
* handle new cache and max-size related message fields in `Query`,
* added test framework Postgres DAS.
@miguelbranco80 miguelbranco80 deleted the protocol-v1 branch February 18, 2025 10:52
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