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

Tweak compilation cache for SQL queries #8066

Merged
merged 13 commits into from
Dec 5, 2024
Merged

Tweak compilation cache for SQL queries #8066

merged 13 commits into from
Dec 5, 2024

Conversation

aljazerzen
Copy link
Contributor

@aljazerzen aljazerzen commented Dec 3, 2024

  • we had QueryUnit.cachable set to False for sql-over-edgedb-protocol,
  • we were using the hash of the whole NormalizedSource as cache key, which includes extracted constants, which defeats (most) of the purpose of caching

Commits:

  • fix NormalizedSource.variables
  • compute cache_key from pg_query_fingerprint
  • Revert "compute cache_key from pg_query_fingerprint"
  • pgsql Source.cache_key
  • more tests

@aljazerzen aljazerzen changed the title pg parser Tweak compilation cache for SQL queries Dec 3, 2024
@msullivan
Copy link
Member

Any idea what's going on with the test failure?

@aljazerzen
Copy link
Contributor Author

We were rejecting multi-statement queries in execute, after trying to fetch type descriptors from PostgreSQL.

Comment on lines +405 to +416
@contextlib.contextmanager
def assertChange(
self, measure_fn: typing.Callable[[], int | float],
expected_change: int | float
):
before = measure_fn()
try:
yield
finally:
after = measure_fn()
change = after - before
self.assertEqual(expected_change, change)
Copy link
Member

Choose a reason for hiding this comment

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

I like this

@aljazerzen aljazerzen merged commit 57deeed into master Dec 5, 2024
24 checks passed
@aljazerzen aljazerzen deleted the pg-parser branch December 5, 2024 17:06
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