Skip to content

Commit 3727e7a

Browse files
committed
fix: harden JOIN semantics — NULL key exclusion, type coercion, column validation, AS alias support, doc fixes
1 parent d9543d9 commit 3727e7a

5 files changed

Lines changed: 268 additions & 7 deletions

File tree

docs/SQL_SPEC.md

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -112,8 +112,8 @@ SELECT [DISTINCT] columns FROM table
112112
#### JOIN (two tables)
113113

114114
```
115-
SELECT qualified_columns FROM table [alias]
116-
[INNER] JOIN table [alias] ON condition { AND condition }
115+
SELECT qualified_columns FROM table [ [AS] alias ]
116+
[ INNER | LEFT [OUTER] ] JOIN table [ [AS] alias ] ON condition { AND condition }
117117
[WHERE conditions]
118118
[ORDER BY qualified_column [ASC|DESC]]
119119
[LIMIT n]
@@ -462,7 +462,7 @@ For UPDATE with WHERE:
462462
| Invalid LIMIT (non-integer) | `ValueError` | `LIMIT must be an integer` |
463463
| INSERT into headless sheet | `ValueError` | `Cannot insert into sheet without headers` |
464464
| Empty IN clause | `ValueError` | `IN clause cannot be empty` |
465-
| Unsupported JOIN variant | `ValueError` | `RIGHT JOIN is not supported...` |
465+
| Unsupported JOIN variant | `ValueError` | `Unsupported SQL syntax: {type} JOIN` |
466466

467467
---
468468

@@ -500,9 +500,9 @@ column_list = column { "," column } ;
500500
value_list = value { "," value } ;
501501
column_def = column [ type_name ] ;
502502
assignment = column "=" value ;
503-
qualified_col = [ table_or_alias "." ] column ;
504-
table_ref = table [ alias ] ;
505-
join_clause = [ "INNER" | "LEFT" ] "JOIN" table_ref "ON" join_cond { "AND" join_cond } ;
503+
qualified_col = table_or_alias "." column ;
504+
table_ref = table [ [ "AS" ] alias ] ;
505+
join_clause = [ "INNER" | "LEFT" [ "OUTER" ] ] "JOIN" table_ref "ON" join_cond { "AND" join_cond } ;
506506
join_cond = qualified_col "=" qualified_col ;
507507
alias = identifier ;
508508

src/excel_dbapi/executor.py

Lines changed: 66 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -308,6 +308,70 @@ def _execute_join_select(
308308
left_sources = {str(from_source["table"]), str(from_source["ref"])}
309309
on_clauses = join_spec["on"]["clauses"]
310310

311+
# --- Column existence validation ---
312+
# Build a mapping: source_ref -> set of valid column names
313+
left_ref = str(from_source["ref"])
314+
left_table_name = str(from_source["table"])
315+
right_ref = str(right_source["ref"])
316+
right_table_name = str(right_source["table"])
317+
source_headers: dict[str, set[str]] = {}
318+
for src in (left_ref, left_table_name):
319+
source_headers[src] = set(left_headers)
320+
for src in (right_ref, right_table_name):
321+
source_headers[src] = set(right_headers)
322+
323+
def _validate_column_ref(source: str, name: str, context: str) -> None:
324+
valid = source_headers.get(source)
325+
if valid is None:
326+
raise ValueError(f"Unknown source reference: {source}")
327+
if name not in valid:
328+
raise ValueError(
329+
f"Unknown column: {source}.{name}. "
330+
f"Available columns in '{source}': {sorted(valid)}"
331+
)
332+
333+
# Validate SELECT columns
334+
for column in parsed["columns"]:
335+
if isinstance(column, dict) and column.get("type") == "column":
336+
_validate_column_ref(str(column["source"]), str(column["name"]), "SELECT")
337+
338+
# Validate ON columns
339+
for clause in on_clauses:
340+
for side in ("left", "right"):
341+
col = clause[side]
342+
_validate_column_ref(str(col["source"]), str(col["name"]), "ON")
343+
344+
# Validate WHERE columns
345+
where_raw = parsed.get("where")
346+
if where_raw:
347+
for condition in where_raw.get("conditions", []):
348+
col_ref = str(condition.get("column", ""))
349+
if "." in col_ref:
350+
src, col_name = col_ref.split(".", 1)
351+
_validate_column_ref(src, col_name, "WHERE")
352+
353+
# Validate ORDER BY column
354+
order_by_raw = parsed.get("order_by")
355+
if order_by_raw:
356+
col_ref = str(order_by_raw["column"])
357+
if "." in col_ref:
358+
src, col_name = col_ref.split(".", 1)
359+
_validate_column_ref(src, col_name, "ORDER BY")
360+
361+
# Sentinel: each NULL key gets a unique object so NULL != NULL per SQL standard.
362+
_null_sentinel_counter = 0
363+
364+
def _normalize_key_value(val: Any) -> Any:
365+
"""Coerce key values for consistent hash matching."""
366+
if val is None:
367+
nonlocal _null_sentinel_counter
368+
_null_sentinel_counter += 1
369+
return object() # unique, never equals another
370+
num = self._to_number(val)
371+
if num is not None:
372+
return num
373+
return val
374+
311375
def build_key(
312376
row_ns: dict[str, Any],
313377
is_left_side: bool,
@@ -318,7 +382,8 @@ def build_key(
318382
right_col = clause["right"]
319383
left_is_left = str(left_col["source"]) in left_sources
320384
selected = left_col if left_is_left == is_left_side else right_col
321-
key_parts.append(self._resolve_join_column(row_ns, selected))
385+
raw_val = self._resolve_join_column(row_ns, selected)
386+
key_parts.append(_normalize_key_value(raw_val))
322387
return tuple(key_parts)
323388

324389
right_hash: dict[tuple[Any, ...], list[dict[str, Any]]] = {}

src/excel_dbapi/parser.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -683,6 +683,11 @@ def _parse_select(
683683
token_index = from_index + 2
684684
if token_index < len(tokens):
685685
maybe_alias = tokens[token_index]
686+
if maybe_alias.upper() == "AS":
687+
token_index += 1
688+
if token_index >= len(tokens):
689+
raise ValueError("Expected alias after AS")
690+
maybe_alias = tokens[token_index]
686691
if not _is_reserved_select_token(maybe_alias):
687692
from_entry["alias"] = maybe_alias
688693
from_entry["ref"] = maybe_alias
@@ -724,6 +729,11 @@ def _parse_select(
724729
}
725730
if token_index < len(tokens):
726731
maybe_alias = tokens[token_index]
732+
if maybe_alias.upper() == "AS":
733+
token_index += 1
734+
if token_index >= len(tokens):
735+
raise ValueError("Expected alias after AS")
736+
maybe_alias = tokens[token_index]
727737
if not _is_reserved_select_token(maybe_alias):
728738
join_source["alias"] = maybe_alias
729739
join_source["ref"] = maybe_alias

tests/test_executor.py

Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -542,3 +542,157 @@ def test_executor_join_missing_right_sheet(tmp_path: Path):
542542
)
543543
with pytest.raises(ValueError, match="not found"):
544544
SharedExecutor(engine).execute(parsed)
545+
546+
547+
def test_executor_join_null_keys_do_not_match(tmp_path: Path):
548+
"""NULL join keys must not match per SQL standard: NULL != NULL."""
549+
file_path = tmp_path / "join_null_keys.xlsx"
550+
workbook = Workbook()
551+
left = workbook.active
552+
assert left is not None
553+
left.title = "left_t"
554+
left.append(["id", "name"])
555+
left.append([1, "Alice"])
556+
left.append([None, "Bob"]) # NULL key
557+
left.append([3, "Charlie"])
558+
559+
right = workbook.create_sheet("right_t")
560+
right.append(["id", "role"])
561+
right.append([1, "admin"])
562+
right.append([None, "ghost"]) # NULL key
563+
right.append([3, "editor"])
564+
workbook.save(file_path)
565+
566+
engine = OpenpyxlBackend(str(file_path))
567+
# INNER JOIN: NULL keys should NOT match
568+
parsed = parse_sql(
569+
"SELECT a.id, a.name, b.role FROM left_t a INNER JOIN right_t b ON a.id = b.id"
570+
)
571+
results = SharedExecutor(engine).execute(parsed)
572+
# Bob should NOT appear — NULL != NULL
573+
assert results.rows == [(1, "Alice", "admin"), (3, "Charlie", "editor")]
574+
575+
576+
def test_executor_left_join_null_keys_unmatched(tmp_path: Path):
577+
"""LEFT JOIN with NULL keys: left row with NULL key gets NULL-filled right columns."""
578+
file_path = tmp_path / "left_join_null_keys.xlsx"
579+
workbook = Workbook()
580+
left = workbook.active
581+
assert left is not None
582+
left.title = "left_t"
583+
left.append(["id", "name"])
584+
left.append([1, "Alice"])
585+
left.append([None, "Bob"])
586+
587+
right = workbook.create_sheet("right_t")
588+
right.append(["id", "role"])
589+
right.append([1, "admin"])
590+
right.append([None, "ghost"])
591+
workbook.save(file_path)
592+
593+
engine = OpenpyxlBackend(str(file_path))
594+
parsed = parse_sql(
595+
"SELECT a.id, a.name, b.role FROM left_t a LEFT JOIN right_t b ON a.id = b.id"
596+
)
597+
results = SharedExecutor(engine).execute(parsed)
598+
# Bob (NULL key) has no match → right columns are None
599+
assert results.rows == [(1, "Alice", "admin"), (None, "Bob", None)]
600+
601+
602+
def test_executor_join_numeric_string_coercion(tmp_path: Path):
603+
"""Join key '1' (string) should match 1 (int) via numeric coercion."""
604+
file_path = tmp_path / "join_coercion.xlsx"
605+
workbook = Workbook()
606+
left = workbook.active
607+
assert left is not None
608+
left.title = "left_t"
609+
left.append(["id", "name"])
610+
left.append(["1", "Alice"]) # string '1'
611+
left.append(["2", "Bob"])
612+
613+
right = workbook.create_sheet("right_t")
614+
right.append(["id", "role"])
615+
right.append([1, "admin"]) # int 1
616+
right.append([2, "editor"])
617+
workbook.save(file_path)
618+
619+
engine = OpenpyxlBackend(str(file_path))
620+
parsed = parse_sql(
621+
"SELECT a.id, a.name, b.role FROM left_t a INNER JOIN right_t b ON a.id = b.id"
622+
)
623+
results = SharedExecutor(engine).execute(parsed)
624+
assert len(results.rows) == 2
625+
assert results.rows[0][1] == "Alice"
626+
assert results.rows[0][2] == "admin"
627+
628+
629+
def test_executor_join_unknown_select_column(tmp_path: Path):
630+
"""SELECT referencing non-existent column in JOIN query raises ValueError."""
631+
file_path = tmp_path / "join_bad_col.xlsx"
632+
workbook = Workbook()
633+
left = workbook.active
634+
assert left is not None
635+
left.title = "users"
636+
left.append(["id", "name"])
637+
left.append([1, "Alice"])
638+
639+
right = workbook.create_sheet("admins")
640+
right.append(["id", "role"])
641+
right.append([1, "admin"])
642+
workbook.save(file_path)
643+
644+
engine = OpenpyxlBackend(str(file_path))
645+
# a.nonexistent doesn't exist in users sheet
646+
parsed = parse_sql(
647+
"SELECT a.nonexistent FROM users a INNER JOIN admins b ON a.id = b.id"
648+
)
649+
with pytest.raises(ValueError, match="Unknown column"):
650+
SharedExecutor(engine).execute(parsed)
651+
652+
653+
def test_executor_join_unknown_on_column(tmp_path: Path):
654+
"""ON referencing non-existent column raises ValueError."""
655+
file_path = tmp_path / "join_bad_on.xlsx"
656+
workbook = Workbook()
657+
left = workbook.active
658+
assert left is not None
659+
left.title = "users"
660+
left.append(["id", "name"])
661+
left.append([1, "Alice"])
662+
663+
right = workbook.create_sheet("admins")
664+
right.append(["id", "role"])
665+
right.append([1, "admin"])
666+
workbook.save(file_path)
667+
668+
engine = OpenpyxlBackend(str(file_path))
669+
parsed = parse_sql(
670+
"SELECT a.id, b.id FROM users a INNER JOIN admins b ON a.id = b.nonexistent"
671+
)
672+
with pytest.raises(ValueError, match="Unknown column"):
673+
SharedExecutor(engine).execute(parsed)
674+
675+
676+
def test_executor_join_with_as_alias(tmp_path: Path):
677+
"""JOIN with AS keyword in alias (SQLAlchemy compatibility)."""
678+
file_path = tmp_path / "join_as_alias.xlsx"
679+
workbook = Workbook()
680+
left = workbook.active
681+
assert left is not None
682+
left.title = "users"
683+
left.append(["id", "name"])
684+
left.append([1, "Alice"])
685+
left.append([2, "Bob"])
686+
687+
right = workbook.create_sheet("admins")
688+
right.append(["id", "role"])
689+
right.append([1, "admin"])
690+
workbook.save(file_path)
691+
692+
engine = OpenpyxlBackend(str(file_path))
693+
# Use 'AS' keyword in both FROM and JOIN aliases
694+
parsed = parse_sql(
695+
"SELECT a.id, a.name, b.role FROM users AS a INNER JOIN admins AS b ON a.id = b.id"
696+
)
697+
results = SharedExecutor(engine).execute(parsed)
698+
assert results.rows == [(1, "Alice", "admin")]

tests/test_parser.py

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -518,3 +518,35 @@ def test_parse_join_rejects_having():
518518
def test_parse_join_rejects_distinct():
519519
with pytest.raises(ValueError, match="DISTINCT is not supported with JOIN"):
520520
parse_sql("SELECT DISTINCT a.id FROM t1 a JOIN t2 b ON a.id = b.id")
521+
522+
523+
def test_parse_join_with_as_alias_from():
524+
"""FROM table AS alias should be accepted."""
525+
result = parse_sql("SELECT a.id, b.id FROM t1 AS a JOIN t2 AS b ON a.id = b.id")
526+
assert result["from"]["alias"] == "a"
527+
assert result["from"]["ref"] == "a"
528+
assert result["joins"][0]["source"]["alias"] == "b"
529+
assert result["joins"][0]["source"]["ref"] == "b"
530+
531+
532+
def test_parse_join_with_as_alias_mixed():
533+
"""Mix of AS alias and bare alias should be accepted."""
534+
result = parse_sql("SELECT a.id, b.id FROM t1 AS a JOIN t2 b ON a.id = b.id")
535+
assert result["from"]["alias"] == "a"
536+
assert result["joins"][0]["source"]["alias"] == "b"
537+
538+
539+
def test_parse_from_as_alias_single_table():
540+
"""FROM table AS alias without JOIN should be accepted."""
541+
result = parse_sql("SELECT id FROM users AS u")
542+
assert result["from"]["alias"] == "u"
543+
assert result["from"]["ref"] == "u"
544+
545+
546+
def test_parse_left_outer_join_with_as():
547+
"""LEFT OUTER JOIN with AS alias should be accepted."""
548+
result = parse_sql(
549+
"SELECT a.id, b.id FROM t1 AS a LEFT OUTER JOIN t2 AS b ON a.id = b.id"
550+
)
551+
assert result["joins"][0]["type"] == "LEFT"
552+
assert result["joins"][0]["source"]["alias"] == "b"

0 commit comments

Comments
 (0)