Skip to content

Commit dc81cfe

Browse files
cowork-bot: emit valid SQL for mixed-type columns and empty/nested-only roots
Type inference now collapses a column whose values mix incompatible types (e.g. a string and an integer) to TEXT instead of keeping the first-seen numeric type. Previously this produced INSERT statements with a quoted string literal in a numeric column, which Postgres/MySQL reject as invalid SQL. A NULL value is treated as "unset" so a column that first sees NULL can still take a concrete type when a non-NULL value arrives (e.g. [null, 42] -> INTEGER), and an all-NULL column resolves to TEXT. convert()/generate_schema() no longer emit an invalid `CREATE TABLE "x" ();` or `INSERT INTO "x" () VALUES ();` for an empty object or a root whose only content is nested arrays (flatten). They now emit only the valid child tables, or an explicit comment when nothing can be generated, instead of a silent green no-op. - src/json2sql/converter.py: add _infer_type/_merge_type; skip empty root tables - tests/test_edge_cases.py: correct the two tests that encoded the old buggy widening behavior - tests/test_type_inference.py: regression tests for mixed-type, NULL, and empty/nested-only-root cases across all three dialects - .github/workflows/cowork-auto-pr.yml: seed server-side PR opener
1 parent d2fd340 commit dc81cfe

4 files changed

Lines changed: 259 additions & 35 deletions

File tree

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
# Seeded by the repo-improver-rotation Cowork job into cowork/improve-* branches.
2+
# Opens a PR automatically when such a branch is pushed (sandbox cannot reach
3+
# the GitHub API directly; this runs server-side with the repo's GITHUB_TOKEN).
4+
name: cowork-auto-pr
5+
on:
6+
push:
7+
branches: ['cowork/improve-**']
8+
permissions:
9+
contents: read
10+
pull-requests: write
11+
jobs:
12+
ensure-pr:
13+
runs-on: ubuntu-latest
14+
steps:
15+
# gh pr create requires a local git checkout to diff head against base;
16+
# without this step every run failed with "not a git repository" and no
17+
# PR was ever opened (fleet-wide defect: 11/11 seeded copies lacked it).
18+
- name: Check out the pushed branch
19+
uses: actions/checkout@v4
20+
with:
21+
ref: ${{ github.ref_name }}
22+
fetch-depth: 0
23+
- name: Open PR for this branch if none exists
24+
env:
25+
GH_TOKEN: ${{ github.token }}
26+
run: |
27+
set -eu
28+
existing=$(gh pr list --repo "$GITHUB_REPOSITORY" --head "$GITHUB_REF_NAME" --state open --json number --jq 'length')
29+
if [ "$existing" = "0" ]; then
30+
gh pr create --repo "$GITHUB_REPOSITORY" \
31+
--head "$GITHUB_REF_NAME" \
32+
--title "cowork-bot: automated improvements ($GITHUB_REF_NAME)" \
33+
--body "Automated improvement PR from the Cowork repo-improver rotation (one coherent senior-dev improvement per run; see individual commit messages). Subsequent runs push additional commits to this PR rather than opening new ones."
34+
else
35+
echo "Open PR already exists for $GITHUB_REF_NAME — nothing to do."
36+
fi

src/json2sql/converter.py

Lines changed: 71 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,14 @@ def convert(self, json_text: str, table_name: str = "data") -> str:
4747
insert_sql(name, list(columns.keys()), rows, self.dialect)
4848
)
4949

50-
return "\n\n".join(statements)
50+
result = "\n\n".join(s for s in statements if s)
51+
# An empty object / nested-only root legitimately produces no SQL; say
52+
# so explicitly instead of returning "" (avoids a silent green no-op).
53+
return (
54+
result
55+
if result
56+
else "-- No columns to generate (empty or nested-only object)."
57+
)
5158

5259
def generate_schema(self, json_text: str, table_name: str = "data") -> str:
5360
"""Generate only CREATE TABLE statements from JSON data."""
@@ -69,14 +76,17 @@ def generate_schema(self, json_text: str, table_name: str = "data") -> str:
6976
else:
7077
columns = {"value": "TEXT"}
7178

72-
statements = [create_table_sql(table_name, columns, self.dialect)]
79+
statements = []
80+
if columns:
81+
statements.append(create_table_sql(table_name, columns, self.dialect))
7382

7483
# Process extra tables from flattening
7584
self._process_flatten(objects, table_name)
7685
for name, cols, _ in self._extra_tables:
7786
statements.append(create_table_sql(name, cols, self.dialect))
7887

79-
return "\n\n".join(statements)
88+
result = "\n\n".join(statements)
89+
return result if result else "-- No columns to generate."
8090

8191
def _convert_objects(self, objects: list[dict], table_name: str) -> str:
8292
"""Convert a list of JSON objects to SQL."""
@@ -117,6 +127,11 @@ def _convert_objects(self, objects: list[dict], table_name: str) -> str:
117127
row.append(format_value(raw, self.dialect))
118128
rows.append(row)
119129

130+
if not columns:
131+
# No scalar columns to emit (e.g. an empty object, or a root whose
132+
# only content is nested arrays that became child tables). Emit
133+
# nothing here rather than an invalid `CREATE TABLE "x" ();`.
134+
return ""
120135
parts = [create_table_sql(table_name, columns, self.dialect)]
121136
if rows:
122137
parts.append(
@@ -133,19 +148,50 @@ def _convert_primitives(self, values: list, table_name: str) -> str:
133148
parts.append(insert_sql(table_name, ["value"], rows, self.dialect))
134149
return "\n\n".join(parts)
135150

151+
def _infer_type(self, value: object) -> str | None:
152+
"""Return the SQL type for ``value``, or ``None`` when it is NULL.
153+
154+
A NULL value is treated as *unset* rather than as TEXT so a column
155+
that first sees NULL can still take a concrete type when a non-NULL
156+
value arrives later (e.g. ``[null, 42]`` -> INTEGER), and an
157+
all-NULL column resolves to TEXT.
158+
"""
159+
if value is None:
160+
return None
161+
return sql_type_for(value, self.dialect)
162+
163+
def _merge_type(self, current: str | None, inferred: str) -> str:
164+
"""Merge a previously inferred column type with a new value's type.
165+
166+
Returns ``TEXT`` whenever the two types are incompatible, because TEXT
167+
is the only column type that is valid across Postgres/MySQL/SQLite.
168+
This guarantees the generated INSERTs are executable for every dialect
169+
instead of silently emitting a quoted string literal into a numeric
170+
column (invalid SQL on Postgres/MySQL).
171+
"""
172+
if current is None:
173+
return inferred
174+
if current == inferred:
175+
return current
176+
return "TEXT"
177+
136178
def _infer_columns(self, objects: list[dict]) -> dict[str, str]:
137-
"""Infer column names and types from a list of objects."""
138-
columns: dict[str, str] = {}
179+
"""Infer column names and types from a list of objects.
180+
181+
Type inference is order-independent and always yields SQL that is
182+
valid for every supported dialect: once a column has seen values of
183+
two incompatible types it collapses to TEXT.
184+
"""
185+
columns: dict[str, str | None] = {}
139186
for obj in objects:
140187
for key, value in obj.items():
188+
inferred = self._infer_type(value)
141189
if key not in columns:
142-
columns[key] = sql_type_for(value, self.dialect)
143-
elif value is not None:
144-
# Upgrade type if we find a non-null value
145-
inferred = sql_type_for(value, self.dialect)
146-
if columns[key] == "TEXT" and inferred != "TEXT":
147-
columns[key] = inferred
148-
return columns
190+
columns[key] = inferred
191+
elif inferred is not None:
192+
columns[key] = self._merge_type(columns[key], inferred)
193+
# Resolve columns that never received a concrete value (all NULL) to TEXT.
194+
return {k: (v if v is not None else "TEXT") for k, v in columns.items()}
149195

150196
def _infer_columns_flattened(
151197
self, objects: list[dict], table_name: str
@@ -157,20 +203,21 @@ def _infer_columns_flattened(
157203
and flat_map maps flattened column name -> (parent_key, sub_key) for
158204
resolving values during row construction.
159205
"""
160-
columns: dict[str, str] = {}
206+
columns: dict[str, str | None] = {}
161207
flat_map: dict[str, tuple[str, str]] = {}
162208
for obj in objects:
163209
for key, value in obj.items():
164210
if isinstance(value, dict) and self.flatten:
165211
for sub_key, sub_value in value.items():
166212
flat_key = f"{key}_{sub_key}"
213+
inferred = self._infer_type(sub_value)
167214
if flat_key not in columns:
168-
columns[flat_key] = sql_type_for(sub_value, self.dialect)
215+
columns[flat_key] = inferred
169216
flat_map[flat_key] = (key, sub_key)
170-
elif sub_value is not None:
171-
inferred = sql_type_for(sub_value, self.dialect)
172-
if columns[flat_key] == "TEXT" and inferred != "TEXT":
173-
columns[flat_key] = inferred
217+
elif inferred is not None:
218+
columns[flat_key] = self._merge_type(
219+
columns[flat_key], inferred
220+
)
174221
elif (
175222
isinstance(value, list)
176223
and value
@@ -180,13 +227,13 @@ def _infer_columns_flattened(
180227
# Skip - goes to separate table
181228
pass
182229
else:
230+
inferred = self._infer_type(value)
183231
if key not in columns:
184-
columns[key] = sql_type_for(value, self.dialect)
185-
elif value is not None:
186-
inferred = sql_type_for(value, self.dialect)
187-
if columns[key] == "TEXT" and inferred != "TEXT":
188-
columns[key] = inferred
189-
return columns, flat_map
232+
columns[key] = inferred
233+
elif inferred is not None:
234+
columns[key] = self._merge_type(columns[key], inferred)
235+
resolved = {k: (v if v is not None else "TEXT") for k, v in columns.items()}
236+
return resolved, flat_map
190237

191238
def _flatten_nested(
192239
self,

tests/test_edge_cases.py

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -53,38 +53,46 @@ def test_flatten_mixed_nested_and_simple_values(self):
5353
assert "'missing'" in result or "NULL" in result
5454

5555
def test_flatten_type_upgrade_for_flattened_column(self):
56-
"""flattened column type upgrades from TEXT to more specific type (line 165)."""
56+
"""Flattened column mixing a string and int collapses to TEXT (line 165)."""
5757
from json2sql.converter import JSONToSQLConverter
5858

5959
converter = JSONToSQLConverter(flatten=True)
6060
data = [
61-
{"id": 1, "meta": {"count": "five"}}, # string TEXT
62-
{"id": 2, "meta": {"count": 42}}, # integer → upgrade
61+
{"id": 1, "meta": {"count": "five"}}, # string -> TEXT
62+
{"id": 2, "meta": {"count": 42}}, # integer -> mixed -> TEXT
6363
]
6464
result = converter.convert(json.dumps(data))
6565
assert "meta_count" in result
66-
# After upgrade, type should be INTEGER not TEXT
67-
assert "INTEGER" in result
66+
# The flattened meta_count column saw a string AND an integer, so it must
67+
# be TEXT (not widened to INTEGER) to keep the generated SQL valid.
68+
assert "TEXT" in result
6869
assert "'five'" in result
6970
assert "42" in result
7071

7172
def test_type_upgrade_for_top_level_column(self):
72-
"""Top-level column type upgrades from TEXT to more specific type (line 175)."""
73+
"""Mixed-type column collapses to TEXT so the generated SQL is valid.
74+
75+
A column that mixes a string and an integer must become TEXT (not
76+
INTEGER): otherwise the string literal would be inserted into a numeric
77+
column and rejected by Postgres/MySQL. See ``converter._merge_type``.
78+
"""
7379
from json2sql.converter import JSONToSQLConverter
7480

7581
data = [
76-
{"key": "hello"}, # string TEXT
77-
{"key": 100}, # integer → upgrade to INTEGER
82+
{"key": "hello"}, # string -> TEXT
83+
{"key": 100}, # integer -> would be INTEGER, but mixed -> TEXT
7884
]
7985
# Non-flatten path (_infer_columns)
8086
result = JSONToSQLConverter().convert(json.dumps(data))
81-
assert "INTEGER" in result
87+
assert "TEXT" in result
88+
assert "INTEGER" not in result # must NOT widen to a numeric type
8289
assert "'hello'" in result
8390
assert "100" in result
8491

85-
# Flatten path (_infer_columns_flattened else branch, line 175)
92+
# Flatten path (_infer_columns_flattened else branch)
8693
result2 = JSONToSQLConverter(flatten=True).convert(json.dumps(data))
87-
assert "INTEGER" in result2
94+
assert "TEXT" in result2
95+
assert "INTEGER" not in result2
8896
assert "'hello'" in result2
8997
assert "100" in result2
9098

tests/test_type_inference.py

Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,133 @@
1+
"""Regression tests for type inference and valid-SQL edge cases.
2+
3+
These guard the two correctness fixes in converter.py:
4+
5+
1. Mixed-type columns collapse to TEXT so the generated INSERTs are valid for
6+
every dialect (Postgres/MySQL would reject a quoted string literal placed
7+
into a numeric column).
8+
2. Empty / nested-only roots emit valid SQL (or an explicit comment) instead of
9+
an invalid ``CREATE TABLE "x" ();`` / ``INSERT INTO "x" () VALUES ();``.
10+
"""
11+
12+
from __future__ import annotations
13+
14+
import json
15+
import re
16+
17+
import pytest
18+
19+
from json2sql.converter import JSONToSQLConverter
20+
from json2sql.dialects import Dialect
21+
22+
NUMERIC = {"INT", "INTEGER", "DOUBLE PRECISION", "DOUBLE", "REAL"}
23+
24+
25+
def column_type(out: str, col: str, dialect: Dialect) -> str | None:
26+
"""Extract the SQL type declared for ``col`` from a CREATE TABLE in ``out``."""
27+
qcol = f"`{col}`" if dialect == Dialect.MYSQL else f'"{col}"'
28+
m = re.search(re.escape(qcol) + r"\s+(\w+(?:\([^)]*\))?)", out)
29+
return m.group(1) if m else None
30+
31+
32+
@pytest.mark.parametrize("dialect", [Dialect.POSTGRES, Dialect.MYSQL, Dialect.SQLITE])
33+
def test_mixed_string_then_int_collapses_to_text(dialect):
34+
"""A string followed by an int must become TEXT, not a numeric type."""
35+
out = JSONToSQLConverter(dialect=dialect).convert(
36+
json.dumps([{"k": "hello"}, {"k": 100}])
37+
)
38+
assert column_type(out, "k", dialect) == "TEXT"
39+
assert "'hello'" in out and "100" in out
40+
41+
42+
@pytest.mark.parametrize("dialect", [Dialect.POSTGRES, Dialect.MYSQL, Dialect.SQLITE])
43+
def test_mixed_int_then_string_collapses_to_text(dialect):
44+
"""An int followed by a string must also become TEXT (order independence)."""
45+
out = JSONToSQLConverter(dialect=dialect).convert(
46+
json.dumps([{"k": 1}, {"k": "hello"}])
47+
)
48+
assert column_type(out, "k", dialect) == "TEXT"
49+
50+
51+
def test_mixed_int_then_float_collapses_to_text():
52+
"""Two incompatible concrete types also collapse to TEXT."""
53+
out = JSONToSQLConverter().convert(json.dumps([{"k": 1}, {"k": 2.5}]))
54+
assert column_type(out, "k", Dialect.POSTGRES) == "TEXT"
55+
56+
57+
@pytest.mark.parametrize("dialect", [Dialect.POSTGRES, Dialect.MYSQL, Dialect.SQLITE])
58+
def test_null_then_int_upgrades_to_int(dialect):
59+
"""A NULL first does not pin the column to TEXT; a later int wins."""
60+
out = JSONToSQLConverter(dialect=dialect).convert(
61+
json.dumps([{"k": None}, {"k": 42}])
62+
)
63+
assert column_type(out, "k", dialect) in NUMERIC
64+
65+
66+
@pytest.mark.parametrize("dialect", [Dialect.POSTGRES, Dialect.MYSQL, Dialect.SQLITE])
67+
def test_int_then_null_stays_int(dialect):
68+
"""A trailing NULL must not downgrade an established concrete type."""
69+
out = JSONToSQLConverter(dialect=dialect).convert(
70+
json.dumps([{"k": 7}, {"k": None}])
71+
)
72+
assert column_type(out, "k", dialect) in NUMERIC
73+
74+
75+
@pytest.mark.parametrize("dialect", [Dialect.POSTGRES, Dialect.MYSQL, Dialect.SQLITE])
76+
def test_all_null_column_defaults_to_text(dialect):
77+
"""An all-NULL column resolves to TEXT rather than an unset/empty type."""
78+
out = JSONToSQLConverter(dialect=dialect).convert(
79+
json.dumps([{"k": None}, {"k": None}])
80+
)
81+
assert column_type(out, "k", dialect) == "TEXT"
82+
83+
84+
@pytest.mark.parametrize("dialect", [Dialect.POSTGRES, Dialect.MYSQL, Dialect.SQLITE])
85+
def test_null_then_string_becomes_text(dialect):
86+
"""NULL first, then a string -> a text column (not an unset numeric type).
87+
88+
A NULL is "unset", so the later string wins; MySQL renders this as
89+
VARCHAR(255), Postgres/SQLite as TEXT. The key invariant is that it is a
90+
string type, never INTEGER/INT.
91+
"""
92+
out = JSONToSQLConverter(dialect=dialect).convert(
93+
json.dumps([{"k": None}, {"k": "x"}])
94+
)
95+
assert column_type(out, "k", dialect) in {"TEXT", "VARCHAR(255)"}
96+
97+
98+
def test_empty_object_root_emits_no_invalid_sql():
99+
"""An empty object must not produce ``CREATE TABLE "data" ();``."""
100+
out = JSONToSQLConverter().convert(json.dumps({}))
101+
assert "CREATE TABLE" not in out
102+
assert "INSERT INTO" not in out
103+
# An explicit, observable comment instead of a silent empty string.
104+
assert out.startswith("-- ")
105+
106+
107+
def test_generate_schema_empty_object_emits_comment():
108+
"""generate_schema on an empty object returns a comment, not invalid SQL."""
109+
out = JSONToSQLConverter().generate_schema(json.dumps({}))
110+
assert "CREATE TABLE" not in out
111+
assert out.startswith("-- ")
112+
113+
114+
@pytest.mark.parametrize("dialect", [Dialect.POSTGRES, Dialect.MYSQL, Dialect.SQLITE])
115+
def test_nested_array_only_root_keeps_child_tables_only(dialect):
116+
"""Flatten of a root with only a nested array emits child table, no bad root."""
117+
out = JSONToSQLConverter(dialect=dialect, flatten=True).convert(
118+
json.dumps({"items": [{"x": 1}, {"x": 2}]})
119+
)
120+
assert "data_items" in out
121+
assert column_type(out, "x", dialect) in NUMERIC
122+
# The bogus empty root CREATE/INSERT must be absent.
123+
assert 'CREATE TABLE "data" (\n\n);' not in out
124+
assert "INSERT INTO \"data\" ()" not in out
125+
126+
127+
def test_convert_never_emits_empty_column_list():
128+
"""No generated statement may contain a zero-column table or insert."""
129+
out = JSONToSQLConverter(flatten=True).convert(
130+
json.dumps([{"a": 1, "b": [{"c": 2}]}, {"a": 3}])
131+
)
132+
assert "();" not in out
133+
assert "INSERT INTO" in out

0 commit comments

Comments
 (0)