Skip to content

Commit 1a2bffb

Browse files
authored
Add comprehensive plan to fix remaining 819 skipped tests (#15)
1 parent 8a5092d commit 1a2bffb

File tree

11 files changed

+1447
-196
lines changed

11 files changed

+1447
-196
lines changed

PLAN.md

Lines changed: 276 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,276 @@
1+
# Comprehensive Plan: Fix Remaining Tests
2+
3+
## Current Status
4+
- **Tests passing:** 6,005 (88.0%)
5+
- **Tests skipped:** 819 (12.0%)
6+
- Parser failures: 173 tests
7+
- Explain mismatches: 331 tests
8+
- Other (metadata skip/explain=false): ~315 tests
9+
10+
## Phase 1: Parser Fixes (High Impact)
11+
12+
### 1.1 `view()` Table Function (~50 tests)
13+
**Problem:** The `view(SELECT ...)` table function with inline subquery fails to parse.
14+
```sql
15+
SELECT * FROM view(SELECT 1 as id);
16+
```
17+
**Files:** `parser/parser.go` (parseTableExpression, parseFunctionCall)
18+
**Solution:** When parsing a function call and the function name is `view`, check if the first argument starts with SELECT/WITH and parse it as a subquery instead of expression list.
19+
20+
### 1.2 Complex Type Casts with Named Parameters (~30 tests)
21+
**Problem:** `::Tuple(a UInt32, b String)` with named fields fails
22+
```sql
23+
SELECT tuple(42, 42)::Tuple(a UInt32, b UInt32);
24+
```
25+
**Files:** `parser/expression.go` (parseDataType)
26+
**Solution:** Extend parseDataType to handle named parameters in type constructors like `Tuple(name Type, ...)`.
27+
28+
### 1.3 DESCRIBE on Table Functions (~20 tests)
29+
**Problem:** `desc format()`, `desc url()`, `desc s3Cluster()` fail
30+
```sql
31+
desc format(CSV, '"value"');
32+
```
33+
**Files:** `parser/parser.go` (parseDescribe)
34+
**Solution:** Handle table function after DESC/DESCRIBE by calling parseTableExpression.
35+
36+
### 1.4 INSERT INTO FUNCTION (~15 tests)
37+
**Problem:** INSERT INTO FUNCTION with file paths and settings fails
38+
```sql
39+
insert into function file(02458_data.jsonl) select * settings engine_file_truncate_on_insert=1;
40+
```
41+
**Files:** `parser/parser.go` (parseInsert)
42+
**Solution:** Handle TABLE FUNCTION keyword and parse function call with settings.
43+
44+
### 1.5 CREATE USER / FUNCTION / DICTIONARY (~10 tests)
45+
**Problem:** These CREATE variants are not supported
46+
```sql
47+
CREATE USER test_user GRANTEES ...;
48+
CREATE DICTIONARY d0 (c1 UInt64) PRIMARY KEY c1;
49+
```
50+
**Files:** `parser/parser.go` (parseCreate)
51+
**Solution:** Add cases for USER, FUNCTION, DICTIONARY in parseCreate switch.
52+
53+
### 1.6 SHOW SETTINGS (~5 tests)
54+
**Problem:** SHOW SETTINGS LIKE syntax not supported
55+
```sql
56+
show settings like 'send_timeout';
57+
```
58+
**Files:** `parser/parser.go` (parseShow)
59+
**Solution:** Handle SETTINGS keyword after SHOW.
60+
61+
### 1.7 PASTE JOIN (~3 tests)
62+
**Problem:** PASTE JOIN is not recognized
63+
```sql
64+
SELECT * FROM t1 PASTE JOIN t2;
65+
```
66+
**Files:** `parser/parser.go` (parseTableExpression or join parsing)
67+
**Solution:** Add PASTE as a valid join type.
68+
69+
### 1.8 `any()` Subquery Syntax (~2 tests)
70+
**Problem:** `== any (SELECT ...)` syntax not supported
71+
```sql
72+
select 1 == any (select number from numbers(10));
73+
```
74+
**Files:** `parser/expression.go`
75+
**Solution:** Handle `any(subquery)` as a special expression form after comparison operators.
76+
77+
---
78+
79+
## Phase 2: Explain Layer Fixes (Medium Impact)
80+
81+
### 2.1 INDEX Clause in CREATE TABLE (~50 tests)
82+
**Problem:** INDEX definitions are skipped but should produce explain output
83+
```sql
84+
CREATE TABLE t (x UInt8, INDEX i x TYPE hypothesis GRANULARITY 100);
85+
```
86+
**Files:** `parser/parser.go` (parseCreateTable), `internal/explain/statements.go`
87+
**Solution:**
88+
1. Parse INDEX into an ast.IndexDefinition struct
89+
2. Add explain output for index definitions
90+
91+
### 2.2 SETTINGS Inside Function Arguments (~40 tests)
92+
**Problem:** SETTINGS in table functions should create a Set child
93+
```sql
94+
SELECT * FROM icebergS3(s3_conn, SETTINGS key='value');
95+
```
96+
**Files:** `parser/expression.go` (parseFunctionCall), `internal/explain/functions.go`
97+
**Solution:** Capture SETTINGS as a Set node attached to the function call, output in explain.
98+
99+
### 2.3 WITH FILL Clause (~30 tests)
100+
**Problem:** ORDER BY ... WITH FILL is not captured
101+
```sql
102+
SELECT nan ORDER BY 1 WITH FILL;
103+
```
104+
**Files:** `parser/parser.go` (parseOrderByItem), `internal/explain/select.go`
105+
**Solution:** Add WithFill field to OrderItem, parse WITH FILL, output in explain.
106+
107+
### 2.4 Column CODEC Clause (~20 tests)
108+
**Problem:** CODEC(GCD, LZ4) in columns not captured
109+
```sql
110+
CREATE TABLE t (col UInt32 CODEC(GCD, LZ4));
111+
```
112+
**Files:** `parser/parser.go` (parseColumnDeclaration), `internal/explain/statements.go`
113+
**Solution:** Parse CODEC clause into ColumnDeclaration, output in explain.
114+
115+
### 2.5 Column EPHEMERAL Modifier (~15 tests)
116+
**Problem:** EPHEMERAL keyword not captured
117+
```sql
118+
CREATE TABLE t (a Int EPHEMERAL);
119+
```
120+
**Files:** `parser/parser.go` (parseColumnDeclaration)
121+
**Solution:** Add Ephemeral field to ColumnDeclaration, parse and explain.
122+
123+
### 2.6 CREATE TABLE ... AS function() (~15 tests)
124+
**Problem:** CREATE TABLE AS s3Cluster(...) should have Function child
125+
```sql
126+
CREATE TABLE test AS s3Cluster('cluster', 'url');
127+
```
128+
**Files:** `parser/parser.go` (parseCreateTable), `internal/explain/statements.go`
129+
**Solution:** Parse AS clause when followed by function call, store as TableFunction field.
130+
131+
### 2.7 WithElement Wrapper for CTEs (~20 tests)
132+
**Problem:** Some CTEs need WithElement wrapper in output
133+
```sql
134+
WITH sub AS (SELECT ...) SELECT ...;
135+
```
136+
**Files:** `internal/explain/select.go`
137+
**Solution:** Output WithElement wrapper when appropriate for CTE definitions.
138+
139+
### 2.8 Float Scientific Notation (~15 tests)
140+
**Problem:** Very small/large floats should use scientific notation
141+
```sql
142+
SELECT 2.2250738585072014e-308;
143+
```
144+
**Files:** `internal/explain/format.go`
145+
**Solution:** Format floats using scientific notation when appropriate.
146+
147+
### 2.9 Negative Literals in Arrays (~10 tests)
148+
**Problem:** Arrays with negatives may output Function instead of Literal
149+
```sql
150+
SELECT [-10000, 5750];
151+
```
152+
**Files:** `internal/explain/expressions.go`
153+
**Solution:** Properly detect and format negative integer literals in arrays.
154+
155+
### 2.10 Parameterized View Placeholders (~10 tests)
156+
**Problem:** `{name:Type}` parameters in views
157+
```sql
158+
create view v as select number where number%2={parity:Int8};
159+
```
160+
**Files:** `internal/explain/expressions.go`
161+
**Solution:** Output Parameter nodes correctly with type info.
162+
163+
### 2.11 Column TTL (~10 tests)
164+
**Problem:** TTL expression on columns not captured
165+
```sql
166+
CREATE TABLE t (c Int TTL expr());
167+
```
168+
**Files:** `parser/parser.go` (parseColumnDeclaration)
169+
**Solution:** Parse TTL clause into ColumnDeclaration.
170+
171+
---
172+
173+
## Phase 3: Lower Priority Fixes
174+
175+
### 3.1 GROUPING SETS (~5 tests)
176+
```sql
177+
SELECT ... GROUP BY GROUPING SETS ((a), (b));
178+
```
179+
180+
### 3.2 QUALIFY Clause (~5 tests)
181+
```sql
182+
SELECT x QUALIFY row_number() OVER () = 1;
183+
```
184+
185+
### 3.3 INTO OUTFILE TRUNCATE (~3 tests)
186+
```sql
187+
SELECT 1 INTO OUTFILE '/dev/null' TRUNCATE FORMAT Npy;
188+
```
189+
190+
### 3.4 INTERVAL with Dynamic Type (~3 tests)
191+
```sql
192+
SELECT INTERVAL c0::Dynamic DAY;
193+
```
194+
195+
### 3.5 ALTER TABLE with Multiple Operations (~3 tests)
196+
```sql
197+
ALTER TABLE t (DELETE WHERE ...), (UPDATE ... WHERE ...);
198+
```
199+
200+
### 3.6 EXPLAIN SYNTAX for SYSTEM commands (~2 tests)
201+
```sql
202+
explain syntax system drop schema cache for hdfs;
203+
```
204+
205+
---
206+
207+
## Implementation Order (Recommended)
208+
209+
1. **Week 1: Parser Fundamentals**
210+
- 1.2 Complex Type Casts (unlocks many tests)
211+
- 1.1 view() Table Function (high impact)
212+
- 1.3 DESCRIBE on Table Functions
213+
214+
2. **Week 2: Parser Completeness**
215+
- 1.4 INSERT INTO FUNCTION
216+
- 1.5 CREATE USER/FUNCTION/DICTIONARY
217+
- 1.6 SHOW SETTINGS
218+
- 1.7 PASTE JOIN
219+
- 1.8 any() Subquery
220+
221+
3. **Week 3: Explain Layer - CREATE TABLE**
222+
- 2.1 INDEX Clause
223+
- 2.4 CODEC Clause
224+
- 2.5 EPHEMERAL Modifier
225+
- 2.6 CREATE TABLE AS function()
226+
- 2.11 Column TTL
227+
228+
4. **Week 4: Explain Layer - SELECT**
229+
- 2.2 SETTINGS in Functions
230+
- 2.3 WITH FILL
231+
- 2.7 WithElement for CTEs
232+
- 2.10 Parameterized View Placeholders
233+
234+
5. **Week 5: Explain Layer - Formatting**
235+
- 2.8 Float Scientific Notation
236+
- 2.9 Negative Literals in Arrays
237+
238+
6. **Week 6: Remaining Items**
239+
- Phase 3 lower priority items
240+
241+
---
242+
243+
## Estimated Impact
244+
245+
| Phase | Tests Fixed | New Pass Rate |
246+
|-------|-------------|---------------|
247+
| 1.1-1.4 | ~115 | ~90% |
248+
| 1.5-1.8 | ~20 | ~90.5% |
249+
| 2.1-2.6 | ~140 | ~93% |
250+
| 2.7-2.11 | ~65 | ~94% |
251+
| Phase 3 | ~20 | ~94.5% |
252+
253+
---
254+
255+
## Files to Modify
256+
257+
### Parser Layer
258+
- `parser/parser.go` - Main parser (CREATE, INSERT, DESCRIBE, SHOW, joins)
259+
- `parser/expression.go` - Expression parsing (type casts, functions, special syntax)
260+
- `ast/ast.go` - AST node definitions (IndexDefinition, new fields)
261+
262+
### Explain Layer
263+
- `internal/explain/statements.go` - CREATE TABLE explain
264+
- `internal/explain/select.go` - SELECT explain (WITH FILL, CTEs)
265+
- `internal/explain/functions.go` - Function explain (SETTINGS)
266+
- `internal/explain/expressions.go` - Expression explain (literals, parameters)
267+
- `internal/explain/format.go` - Output formatting (scientific notation)
268+
269+
---
270+
271+
## Testing Strategy
272+
273+
1. Run tests frequently: `go test ./parser -timeout 5s`
274+
2. After each fix, verify no regressions: compare PASS count
275+
3. Check specific test cases: `go test ./parser -v -run "TestParser/test_name"`
276+
4. Monitor for infinite loops (timeout protection already in place)

0 commit comments

Comments
 (0)