Allow to log (debug) queries in text form to dedicated log file#419
Draft
Allow to log (debug) queries in text form to dedicated log file#419
Conversation
kofrezo
changed the title
Introduce sql_generator_v2.py that rewrites the query generation using
Django ORM instead of raw SQL string concatenation. This provides better
security through parameterized queries and leverages Django's query
optimization.
Key changes:
- New query_builder.py facade for switching between v1/v2 implementations
- New sql_generator_v2.py with Django ORM-based query generation
- SQL_GENERATOR_VERSION setting to toggle implementations ('v1' or 'v2')
- query_executer.py updated to handle both SQL strings and QuerySets
- Comprehensive unit tests comparing v1 and v2 results
The v2 implementation supports all filter types (BaseFilter, Regexp,
comparison, containment, logical) and attribute types (special, standard,
relation, reverse, supernet, domain) with related_via traversal.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
The error "can't adapt type 'OuterRef'" occurred because: 1. OuterRef(OuterRef(...)) is invalid - Django's OuterRef doesn't nest 2. OuterRef cannot be passed as a parameter to RawSQL Fixed by restructuring _build_supernet_lookup and _build_supernet_related_query to reference the outer query's server table directly in the SQL string using "server"."server_id" instead of trying to use nested OuterRef expressions. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Remove defer('intern_ip') for v2 QuerySets to prevent N+1 queries.
The issue: defer() on a v2 QuerySet excludes intern_ip from the SELECT,
but QueryMaterializer later accesses intern_ip, causing Django to fetch
it individually for each server (thousands of extra queries).
The v1 raw SQL already includes intern_ip in the SELECT clause, so
defer() has no effect there. Removing defer() for v2 matches this
behavior and eliminates the N+1 problem.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
The issue: OuterRef('server_id') inside a nested Subquery incorrectly
references the immediate outer query's server_id (ServerAttribute table)
instead of the outer Server table's server_id.
For example, external_domain servers with responsible_admin configured
as "related via" were missing because the relation lookup was correlating
with the wrong table.
Fixed by using RawSQL with direct reference to "server"."server_id"
for both relation and reverse related_via types, similar to the
supernet case. This ensures proper correlation with the outer Server query.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
The query "route_network=foe-aw-int" was 14x slower (5.7s vs 400ms) because
the hostname filter was applied AFTER a correlated subquery, causing it to
be re-evaluated for each row in the outer query.
Before (slow - nested structure):
EXISTS (
SELECT FROM server U0
WHERE U0.server_id IN (correlated supernet lookup) -- per-row!
AND U0.hostname = 'foe-aw-int'
)
After (fast - matches v1 structure):
EXISTS (
SELECT FROM server supernet
JOIN inet tables
WHERE inet_containment
AND supernet.server_id IN (
SELECT server_id FROM server WHERE hostname = 'foe-aw-int' -- once!
)
)
The key optimization: the hostname filter is now a NON-CORRELATED subquery
that PostgreSQL can evaluate once and cache, rather than per-row.
Also added _build_hostname_filter_sql() helper to convert Q objects to SQL
for embedding in the optimized query structure.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
Add a method that takes the internal representation of a Query (dict) and builds the text query from it. This will be handy to log queries for debugging.
Add a configurable logger for logging Servershell and API queries to a dedicated file.
kofrezo
force-pushed
the
dk_log_queries
branch
from
01a95d8 to
f641d3d
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.