Skip to content

Commit 6f76ef6

Browse files
authored
Harden security, improve reliability, release beta 5 (#3)
Transform layer overhaul: - Replace 500+ line if-else dispatch chains with table-driven lookup - Unify three parallel variable tracking systems into single API - Consolidate SQL generation from 3 paths down to 2 - Remove ~400 lines of redundant variable tracking code - Remove ~310 lines of dead code and unused declarations Security and reliability: - Parameterized queries prevent SQL injection in MERGE operations - Dynamic buffers prevent overflow in JSON serialization - Proper deep-copy and cleanup for path element properties - Consistent string escaping for SQL literals - Dynamic pending_prop_joins eliminates silent query truncation New infrastructure: - sql_builder module for structured query construction - query_dispatch with priority-based pattern registry - json_builder for graph algorithm output - transform_variables with scope management and visibility tracking Bug fixes: - Fix varlen relationship use-after-free - Fix WITH+MATCH variable chaining - Fix path variable lookup in RETURN - Fix UNION queries returning only second branch - Fix NULL value handling (return JSON null, not string "NULL") Tests: 749 C + 160 Python passing Version: 0.1.0-beta.5
1 parent e99c5a5 commit 6f76ef6

File tree

115 files changed

+14515
-3362
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

115 files changed

+14515
-3362
lines changed
Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
---
2+
id: 001-keep-append-sql-for-expression
3+
level: adr
4+
title: "Keep append_sql for expression transformation"
5+
number: 1
6+
short_code: "GQLITE-A-0001"
7+
created_at: 2025-12-27T17:12:02.977994+00:00
8+
updated_at: 2025-12-27T17:12:02.977994+00:00
9+
decision_date:
10+
decision_maker:
11+
parent:
12+
archived: true
13+
14+
tags:
15+
- "#adr"
16+
- "#phase/draft"
17+
18+
19+
exit_criteria_met: false
20+
strategy_id: NULL
21+
initiative_id: NULL
22+
---
23+
24+
# ADR-1: Keep append_sql for expression transformation
25+
26+
## Context
27+
28+
During the Unified SQL Builder initiative (GQLITE-I-0025), we migrated READ query clauses (MATCH, WITH, UNWIND, RETURN) to use the structured `sql_builder` API. This raised the question of whether expression transformation should also be migrated.
29+
30+
Expression transformation currently uses `append_sql()` with 399 calls across:
31+
- `transform_expr_ops.c` - 78 calls (binary ops, comparisons)
32+
- `transform_return.c` - 74 calls (return expressions)
33+
- `transform_expr_predicate.c` - 41 calls (IS NULL, LIKE, IN)
34+
- `transform_func_*.c` - ~170 calls (function implementations)
35+
36+
## Decision
37+
38+
**Keep `append_sql()` for expression transformation.** Do not migrate expressions to the unified builder API.
39+
40+
## Rationale
41+
42+
Expression transformation is fundamentally different from clause assembly:
43+
44+
1. **Linear string building**: Expressions are built left-to-right as SQL fragments. There's no clause reordering needed (unlike SELECT/FROM/WHERE which must be assembled in specific order).
45+
46+
2. **No deferred assembly**: Expression output is immediate - `1 + 2` becomes `1 + 2`. There's no need to buffer parts for later assembly.
47+
48+
3. **Nested recursion**: Expressions recurse deeply (`transform_expression``transform_binary_op``transform_expression`). Each level appends to the same buffer, which `append_sql()` handles naturally.
49+
50+
4. **Different abstraction level**: The unified builder operates at clause level (SELECT, FROM, JOIN). Expressions are sub-clause components that get passed TO the builder via `sql_select(builder, expr_string, alias)`.
51+
52+
## Consequences
53+
54+
### Positive
55+
- No unnecessary refactoring of working code
56+
- Expression transformation remains simple and direct
57+
- Clear separation: builder for clauses, append_sql for expressions
58+
59+
### Negative
60+
- Two patterns coexist in codebase (acceptable given different purposes)
61+
62+
### Neutral
63+
- 399 append_sql calls remain (appropriate usage)
Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
---
2+
id: 001-keep-finalize-sql-generation-as
3+
level: adr
4+
title: "Keep finalize_sql_generation as final assembly step"
5+
number: 1
6+
short_code: "GQLITE-A-0002"
7+
created_at: 2025-12-27T17:12:39.126579+00:00
8+
updated_at: 2025-12-27T17:12:39.126579+00:00
9+
decision_date:
10+
decision_maker:
11+
parent:
12+
archived: true
13+
14+
tags:
15+
- "#adr"
16+
- "#phase/draft"
17+
18+
19+
exit_criteria_met: false
20+
strategy_id: NULL
21+
initiative_id: NULL
22+
---
23+
24+
# ADR-1: Keep finalize_sql_generation as final assembly step
25+
26+
*This template includes sections for various types of architectural decisions. Delete sections that don't apply to your specific use case.*
27+
28+
## Context **[REQUIRED]**
29+
30+
{What is the issue that we're seeing that is motivating this decision or change?}
31+
32+
## Decision **[REQUIRED]**
33+
34+
{What is the change that we're proposing or have agreed to implement?}
35+
36+
## Alternatives Analysis **[CONDITIONAL: Complex Decision]**
37+
38+
{Delete if there's only one obvious solution}
39+
40+
| Option | Pros | Cons | Risk Level | Implementation Cost |
41+
|--------|------|------|------------|-------------------|
42+
| {Option 1} | {Benefits} | {Drawbacks} | {Low/Medium/High} | {Estimate} |
43+
| {Option 2} | {Benefits} | {Drawbacks} | {Low/Medium/High} | {Estimate} |
44+
| {Option 3} | {Benefits} | {Drawbacks} | {Low/Medium/High} | {Estimate} |
45+
46+
## Rationale **[REQUIRED]**
47+
48+
{Why did we choose this option over alternatives?}
49+
50+
## Consequences **[REQUIRED]**
51+
52+
### Positive
53+
- {Benefit 1}
54+
- {Benefit 2}
55+
56+
### Negative
57+
- {Cost or drawback 1}
58+
- {Cost or drawback 2}
59+
60+
### Neutral
61+
- {Neutral consequence 1}
62+
63+
## Review Schedule **[CONDITIONAL: Temporary Decision]**
64+
65+
{Delete if decision is permanent}
66+
67+
### Review Triggers
68+
- {Condition that would trigger review 1}
69+
- {Condition that would trigger review 2}
70+
71+
### Scheduled Review
72+
- **Next Review Date**: {Date}
73+
- **Review Criteria**: {What to evaluate}
74+
- **Sunset Date**: {When this decision expires if not renewed}

.metis/backlog/bugs/GQLITE-T-0042.md renamed to .metis/archived/backlog/bugs/GQLITE-T-0042.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ created_at: 2025-12-26T02:47:07.078313+00:00
77
updated_at: 2025-12-26T02:55:00.051195+00:00
88
parent:
99
blocked_by: []
10-
archived: false
10+
archived: true
1111

1212
tags:
1313
- "#task"
@@ -86,6 +86,8 @@ MATCH (n:Test) WITH n.category AS cat, sum(n.value) AS total RETURN cat, total
8686

8787
## Acceptance Criteria
8888

89+
## Acceptance Criteria
90+
8991
- [ ] `WHERE age > 27` correctly filters when age is an integer property
9092
- [ ] `WITH n.category AS cat, sum(n.value) AS total` groups by category correctly
9193
- [ ] Python binding tests `test_with_where` and `test_with_aggregation` pass

.metis/backlog/bugs/GQLITE-T-0043.md renamed to .metis/archived/backlog/bugs/GQLITE-T-0043.md

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,15 @@ level: task
44
title: "Fix OPTIONAL MATCH - returns cartesian product instead of left join"
55
short_code: "GQLITE-T-0043"
66
created_at: 2025-12-26T03:08:11.344863+00:00
7-
updated_at: 2025-12-26T03:08:11.344863+00:00
7+
updated_at: 2025-12-27T19:22:49.095227+00:00
88
parent:
99
blocked_by: []
10-
archived: false
10+
archived: true
1111

1212
tags:
1313
- "#task"
14-
- "#phase/backlog"
1514
- "#bug"
15+
- "#phase/completed"
1616

1717

1818
exit_criteria_met: false
@@ -64,6 +64,14 @@ RETURN a.name, friend.name
6464
- **Benefits of Fixing**: {What improves after refactoring}
6565
- **Risk Assessment**: {Risks of not addressing this}
6666

67+
## Acceptance Criteria
68+
69+
## Acceptance Criteria
70+
71+
## Acceptance Criteria
72+
73+
## Acceptance Criteria
74+
6775
## Acceptance Criteria **[REQUIRED]**
6876

6977
- [ ] {Specific, testable requirement 1}

.metis/backlog/bugs/GQLITE-T-0044.md renamed to .metis/archived/backlog/bugs/GQLITE-T-0044.md

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,15 @@ level: task
44
title: "Fix UNWIND+CREATE - only creates one node instead of iterating"
55
short_code: "GQLITE-T-0044"
66
created_at: 2025-12-26T03:16:29.344348+00:00
7-
updated_at: 2025-12-26T03:21:24.361246+00:00
7+
updated_at: 2025-12-27T19:21:45.298697+00:00
88
parent:
99
blocked_by: []
10-
archived: false
10+
archived: true
1111

1212
tags:
1313
- "#task"
1414
- "#bug"
15-
- "#phase/todo"
15+
- "#phase/completed"
1616

1717

1818
exit_criteria_met: false
@@ -53,6 +53,12 @@ The UNWIND transformation creates a CTE with the list values, but the subsequent
5353

5454
## Acceptance Criteria
5555

56+
## Acceptance Criteria
57+
58+
## Acceptance Criteria
59+
60+
## Acceptance Criteria
61+
5662
## Acceptance Criteria
5763
- [ ] `UNWIND [1,2,3] AS x CREATE (:Node {val: x})` creates 3 nodes
5864
- [ ] Python test `test_unwind_with_create` passes

.metis/backlog/bugs/GQLITE-T-0045.md renamed to .metis/archived/backlog/bugs/GQLITE-T-0045.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ created_at: 2025-12-26T03:16:29.456577+00:00
77
updated_at: 2025-12-26T03:30:02.446840+00:00
88
parent:
99
blocked_by: []
10-
archived: false
10+
archived: true
1111

1212
tags:
1313
- "#task"
@@ -56,6 +56,8 @@ The parser doesn't recognize array index access syntax (`identifier[expr]`) when
5656

5757
## Acceptance Criteria
5858

59+
## Acceptance Criteria
60+
5961
## Acceptance Criteria
6062
- [ ] `items[0]` syntax works after WITH projection
6163
- [ ] `items[idx]` works with variable index

.metis/backlog/features/GQLITE-T-0009.md renamed to .metis/archived/backlog/features/GQLITE-T-0009.md

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ created_at: 2025-12-24T01:49:49.503861+00:00
77
updated_at: 2025-12-24T01:49:49.503861+00:00
88
parent:
99
blocked_by: []
10-
archived: false
10+
archived: true
1111

1212
tags:
1313
- "#task"
@@ -36,6 +36,8 @@ Implement CALL clause for invoking stored procedures and built-in database funct
3636

3737
## Acceptance Criteria
3838

39+
## Acceptance Criteria
40+
3941
- [ ] `CALL db.labels()` returns all labels in the database
4042
- [ ] `CALL db.relationshipTypes()` returns all relationship types
4143
- [ ] `CALL db.propertyKeys()` returns all property keys
@@ -75,6 +77,31 @@ Consider allowing user-defined procedures via:
7577
- SQL functions registered in SQLite
7678
- Plugin system for custom procedures
7779

78-
## Status Updates
80+
## Decision (Dec 2025)
81+
82+
**Status**: Won't implement - using language bindings instead.
83+
84+
### Rationale
85+
86+
The CALL procedure syntax exists in Neo4j primarily for:
87+
- Schema introspection (`db.labels()`, `db.propertyKeys()`)
88+
- Admin operations (`dbms.killQuery()`)
89+
- APOC extensions (bulk operations, import/export)
90+
91+
For GraphQLite, these use cases are better served by the language bindings:
92+
93+
| Use Case | CALL Approach | Bindings Approach |
94+
|----------|---------------|-------------------|
95+
| List labels | `CALL db.labels()` | `graph.get_labels()` |
96+
| Schema info | `CALL db.schema.visualization()` | `graph.schema()` |
97+
| Admin ops | `CALL dbms.*` | Direct SQLite access |
98+
99+
**Benefits of bindings approach**:
100+
- No grammar/parser changes needed
101+
- Type-safe in Python/Rust
102+
- Can return rich objects, not just tabular data
103+
- Simpler implementation
104+
105+
**Tradeoff**: Neo4j tooling compatibility lost. Acceptable since GraphQLite is embedded, not a server with external GUI tools connecting to it.
79106

80-
*To be added during implementation*
107+
Revisit if users specifically request Neo4j-compatible introspection queries.

.metis/backlog/features/GQLITE-T-0026.md renamed to .metis/archived/backlog/features/GQLITE-T-0026.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ created_at: 2025-12-24T22:50:16.585069+00:00
77
updated_at: 2025-12-25T16:35:31.911289+00:00
88
parent:
99
blocked_by: []
10-
archived: false
10+
archived: true
1111

1212
tags:
1313
- "#task"
@@ -85,6 +85,8 @@ RETURN louvain(resolution) -- resolution parameter
8585

8686
## Acceptance Criteria
8787

88+
## Acceptance Criteria
89+
8890
## Acceptance Criteria **[REQUIRED]**
8991

9092
- [ ] {Specific, testable requirement 1}

.metis/backlog/features/GQLITE-T-0027.md renamed to .metis/archived/backlog/features/GQLITE-T-0027.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ created_at: 2025-12-24T22:50:16.659637+00:00
77
updated_at: 2025-12-25T16:53:35.321316+00:00
88
parent:
99
blocked_by: []
10-
archived: false
10+
archived: true
1111

1212
tags:
1313
- "#task"
@@ -80,6 +80,8 @@ RETURN triangleCount()
8080

8181
## Acceptance Criteria
8282

83+
## Acceptance Criteria
84+
8385
## Acceptance Criteria **[REQUIRED]**
8486

8587
- [ ] {Specific, testable requirement 1}

.metis/backlog/features/GQLITE-T-0028.md renamed to .metis/archived/backlog/features/GQLITE-T-0028.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ created_at: 2025-12-24T22:50:16.735482+00:00
77
updated_at: 2025-12-25T17:00:35.061052+00:00
88
parent:
99
blocked_by: []
10-
archived: false
10+
archived: true
1111

1212
tags:
1313
- "#task"
@@ -83,6 +83,8 @@ RETURN astar(source, target, heuristic_func)
8383

8484
## Acceptance Criteria
8585

86+
## Acceptance Criteria
87+
8688
## Acceptance Criteria **[REQUIRED]**
8789

8890
- [ ] {Specific, testable requirement 1}

0 commit comments

Comments
 (0)