fix(click-to-expand): fix Neo4j Browser regression bugs from scope refactoring#116
Merged
fix(click-to-expand): fix Neo4j Browser regression bugs from scope refactoring#116
Conversation
- Cargo.toml: 0.6.1 → 0.6.2-dev - CHANGELOG.md: [Unreleased] → [0.6.2-dev] - Unreleased - STATUS.md: version line → v0.6.2-dev - README.md: Replace v0.6.1 What's New with v0.6.2-dev scope redesign summary - Unit tests: 534 → 1,032 (actual current count) - Pytest: '3000+ 95+%' → 3,026 passing / 13 pre-existing failures - LDBC: remove inflated 15/41 claim → accurate 14/37 (38%) - Remove 'both production-ready' Bolt/HTTP overclaim - Version: v0.6.1 → v0.6.2-dev Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Focus on Neo4j Browser and Graph-Notebook support, improved WITH clause correctness, and reliable query results. Remove internal architecture/refactoring details. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Switch all demo tables from Memory to MergeTree so data survives container restarts across both neo4j-browser and graph-notebook demos - Remove 001-init.sh mounts: ClickHouse auto-runs init-db.sql from docker-entrypoint-initdb.d, making the shell wrapper redundant - Switch clickgraph service to image: genezhang/clickgraph:v0.6.2-dev in both demos (was building locally) - Remove data-loading logic from setup_demo_data.sh (now verification only); setup.sh no longer duplicates the entrypoint data load Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…pruned MATCH queries Four bugs fixed to restore Neo4j Browser click-to-expand functionality: Bug 1 (filter_tagging crash): When child plan is Empty (TypeInference pruned all types), FilterTagging's extract_filters fails with no table context. Wrap extract_filters in match block and propagate Empty on error. Bug 2a (VLP CTE empty placeholder - core fix): TypeInference Phase 1 updates plan_ctx with inferred labels for multi-type VLP endpoint, but the GraphNode was already computed BEFORE the labels were set. Re-run infer_labels_recursive on the right GraphNode after multi-type VLP detection so it gets proper ViewScan and node_types populated, preventing Phase 2 from generating empty UNION branches for Post→AUTHORED→Post (which don't exist). Bug 2b (type mismatch in VLP+WITH JOIN): Wrap the node id column in toString() when joining WITH CTEs to VLP CTEs, matching the String type of start_id/end_id stored in VLP CTEs. Bug 2c (extract_node_labels polymorphic): When a node has multiple inferred types (node_types.len() > 1), return all types for path enumeration, not just the primary label. Enables correct CTE generation for polymorphic VLP endpoints (e.g. o:Post|User in Post expansion). Bug 3 (empty SQL for impossible MATCH queries): is_return_only_query() incorrectly classified pruned MATCH queries as RETURN-only because it recursed through GraphJoins with empty joins. A pruned MATCH query like 'MATCH (a:Post)-[r]->(b:Post)' produces GraphJoins(Projection(Empty), joins=[]) after TypeInference removes all invalid paths. Removing GraphJoins from is_return_only_query() ensures such queries trigger the early-exit path that returns SELECT 1 WHERE false, producing empty results instead of bare 'LIMIT 10' syntax errors. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…1 behavior
Amend: distinguish pure RETURN queries from TypeInference-pruned MATCH queries
in is_return_only_query(). Both produce GraphJoins(Projection(Empty), joins=[])
after planning, but differ in Projection items:
- Pure RETURN: items contain literals (e.g. Literal(1) AS val) → return-only
- Pruned MATCH: items contain graph aliases (TableAlias("r")) → NOT return-only
Check projection items for TableAlias expressions to classify correctly.
This prevents 'RETURN 1 AS val' from generating SELECT 1 WHERE false.
Also includes Cargo.lock and README updates for v0.6.2-dev.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Browser probes (OPTIONS, HEAD, non-upgrade GET) hitting the Bolt port were logged as ERROR-level messages, polluting logs. Fix by: - Detecting all common HTTP verbs (OPTIONS/HEAD/PUT/DELETE) as HTTP probes - Downgrading WebSocket handshake failures from ERROR to DEBUG (expected) - Dropping non-GET HTTP probes silently at DEBUG level These are normal browser CORS preflight and health-check requests, not bugs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
genezhang
changed the title
There was a problem hiding this comment.
Pull request overview
This pull request fixes 5 critical regression bugs introduced by the foundational variable scope resolution redesign (PR #115), plus infrastructure improvements for the Neo4j Browser demo targeting v0.6.2-dev.
Changes:
- Bug Fixes: Fixes type inference crashes, VLP multi-type endpoint handling, JOIN type mismatches, polymorphic label extraction, and RETURN-only query detection
- Logging Improvements: Reduces HTTP probe noise on the Bolt port from ERROR to DEBUG level
- Demo Infrastructure: Migrates demo tables to MergeTree ENGINE for data persistence, removes duplicate data loading logic, and uses official Docker image
Reviewed changes
Copilot reviewed 15 out of 17 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
src/query_planner/analyzer/filter_tagging.rs |
Wraps extract_filters call in error handling to prevent crashes when child plan is Empty and has no table context |
src/query_planner/analyzer/type_inference.rs |
Re-runs inference on right node after multi-type VLP label detection so it gets proper ViewScan and node_types |
src/render_plan/plan_builder_utils.rs |
Adds toString() wrappers to JOIN conditions between WITH CTEs and VLP CTEs to resolve UInt64 vs String type mismatches |
src/render_plan/cte_extraction.rs |
Returns all inferred types when node_types.len() > 1 instead of only the primary label for correct polymorphic path enumeration |
src/render_plan/plan_builder.rs |
Distinguishes pruned MATCH queries from RETURN-only queries by checking for TableAlias items in Projection |
src/server/mod.rs |
Detects HTTP probes (OPTIONS, HEAD, etc.) on Bolt port and logs them at DEBUG level instead of ERROR to reduce noise |
demos/neo4j-browser/init-db.sql |
Changes all table ENGINE from Memory to MergeTree for data persistence across container restarts |
demos/neo4j-browser/docker-compose.yml |
Uses official Docker image genezhang/clickgraph:v0.6.2-dev and removes 001-init.sh mount |
demos/neo4j-browser/setup.sh |
Removes duplicate data loading call (data now loaded automatically via init-db.sql) |
demos/neo4j-browser/setup_demo_data.sh |
Converts from data loader to data verification script |
demos/graph-notebook/docker-compose.yml |
Uses official Docker image and removes 001-init.sh mount |
| Version files | Updates version from 0.6.1 to 0.6.2-dev across Cargo.toml, STATUS.md, README.md, and CHANGELOG.md |
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
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.
Summary
Fixes multiple regression bugs in Neo4j Browser click-to-expand functionality introduced by the foundational variable scope resolution redesign (PR #115). Also includes demo infrastructure improvements for v0.6.2-dev.
Bug Fixes
Bug 1 — filter_tagging crash on Empty child plan
When TypeInference prunes all types from a MATCH pattern, the
filter_taggingpass crashed trying to extract filters with no table context. Now wraps in a match block and propagatesEmptyon error.Bug 2a — VLP multi-type inference (core fix)
TypeInference Phase 1 computed the right
GraphNodebefore updatingplan_ctxwith inferred labels for multi-type VLP endpoints. Phase 2 then generated an empty UNION branch (WHERE 0=1). Fix: re-runinfer_labels_recursiveon the right node after detection so it gets properViewScanandnode_types.Bug 2b — VLP+WITH type mismatch
JOIN condition between WITH CTEs and VLP CTEs failed due to type mismatch (
UInt64vsString). Fix: wrap node id columns intoString()in both JOIN condition locations.Bug 2c — extract_node_labels not polymorphic
extract_node_labelsreturned only the primary label whennode_types.len() > 1, causing incorrect path enumeration. Fix: return all types when multiple are present.Bug 3 — empty SQL for impossible MATCH
is_return_only_query()incorrectly treated pruned MATCH queries as RETURN-only (both produceGraphJoins(Projection(Empty))after TypeInference). Fix: distinguish by checking Projection items — pruned MATCH hasTableAliasitems; pureRETURN 1has onlyLiteralitems.Noise fix — HTTP probes on Bolt port
Neo4j Browser sends OPTIONS/GET probes to the Bolt port. These fell through to the TCP Bolt handler causing ERROR-level log spam. Now detected and dropped at DEBUG level.
Demo Infrastructure
MergeTreeENGINE (data persists across container restarts)setup.sh(init-db.sql is the single entrypoint)clickgraphservice →image: genezhang/clickgraph:v0.6.2-dev(official image)Verification