From dc81cfe612c29fe880c59e011c9eb63e2b60a3ec Mon Sep 17 00:00:00 2001 From: Jaixii Date: Fri, 17 Jul 2026 14:22:06 -0400 Subject: [PATCH] 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 --- .github/workflows/cowork-auto-pr.yml | 36 ++++++++ src/json2sql/converter.py | 95 ++++++++++++++----- tests/test_edge_cases.py | 30 +++--- tests/test_type_inference.py | 133 +++++++++++++++++++++++++++ 4 files changed, 259 insertions(+), 35 deletions(-) create mode 100644 .github/workflows/cowork-auto-pr.yml create mode 100644 tests/test_type_inference.py diff --git a/.github/workflows/cowork-auto-pr.yml b/.github/workflows/cowork-auto-pr.yml new file mode 100644 index 0000000..b27f04e --- /dev/null +++ b/.github/workflows/cowork-auto-pr.yml @@ -0,0 +1,36 @@ +# Seeded by the repo-improver-rotation Cowork job into cowork/improve-* branches. +# Opens a PR automatically when such a branch is pushed (sandbox cannot reach +# the GitHub API directly; this runs server-side with the repo's GITHUB_TOKEN). +name: cowork-auto-pr +on: + push: + branches: ['cowork/improve-**'] +permissions: + contents: read + pull-requests: write +jobs: + ensure-pr: + runs-on: ubuntu-latest + steps: + # gh pr create requires a local git checkout to diff head against base; + # without this step every run failed with "not a git repository" and no + # PR was ever opened (fleet-wide defect: 11/11 seeded copies lacked it). + - name: Check out the pushed branch + uses: actions/checkout@v4 + with: + ref: ${{ github.ref_name }} + fetch-depth: 0 + - name: Open PR for this branch if none exists + env: + GH_TOKEN: ${{ github.token }} + run: | + set -eu + existing=$(gh pr list --repo "$GITHUB_REPOSITORY" --head "$GITHUB_REF_NAME" --state open --json number --jq 'length') + if [ "$existing" = "0" ]; then + gh pr create --repo "$GITHUB_REPOSITORY" \ + --head "$GITHUB_REF_NAME" \ + --title "cowork-bot: automated improvements ($GITHUB_REF_NAME)" \ + --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." + else + echo "Open PR already exists for $GITHUB_REF_NAME — nothing to do." + fi diff --git a/src/json2sql/converter.py b/src/json2sql/converter.py index 5f41303..a326f8c 100644 --- a/src/json2sql/converter.py +++ b/src/json2sql/converter.py @@ -47,7 +47,14 @@ def convert(self, json_text: str, table_name: str = "data") -> str: insert_sql(name, list(columns.keys()), rows, self.dialect) ) - return "\n\n".join(statements) + result = "\n\n".join(s for s in statements if s) + # An empty object / nested-only root legitimately produces no SQL; say + # so explicitly instead of returning "" (avoids a silent green no-op). + return ( + result + if result + else "-- No columns to generate (empty or nested-only object)." + ) def generate_schema(self, json_text: str, table_name: str = "data") -> str: """Generate only CREATE TABLE statements from JSON data.""" @@ -69,14 +76,17 @@ def generate_schema(self, json_text: str, table_name: str = "data") -> str: else: columns = {"value": "TEXT"} - statements = [create_table_sql(table_name, columns, self.dialect)] + statements = [] + if columns: + statements.append(create_table_sql(table_name, columns, self.dialect)) # Process extra tables from flattening self._process_flatten(objects, table_name) for name, cols, _ in self._extra_tables: statements.append(create_table_sql(name, cols, self.dialect)) - return "\n\n".join(statements) + result = "\n\n".join(statements) + return result if result else "-- No columns to generate." def _convert_objects(self, objects: list[dict], table_name: str) -> str: """Convert a list of JSON objects to SQL.""" @@ -117,6 +127,11 @@ def _convert_objects(self, objects: list[dict], table_name: str) -> str: row.append(format_value(raw, self.dialect)) rows.append(row) + if not columns: + # No scalar columns to emit (e.g. an empty object, or a root whose + # only content is nested arrays that became child tables). Emit + # nothing here rather than an invalid `CREATE TABLE "x" ();`. + return "" parts = [create_table_sql(table_name, columns, self.dialect)] if rows: parts.append( @@ -133,19 +148,50 @@ def _convert_primitives(self, values: list, table_name: str) -> str: parts.append(insert_sql(table_name, ["value"], rows, self.dialect)) return "\n\n".join(parts) + def _infer_type(self, value: object) -> str | None: + """Return the SQL type for ``value``, or ``None`` when it is NULL. + + A NULL value is treated as *unset* rather than as TEXT so a column + that first sees NULL can still take a concrete type when a non-NULL + value arrives later (e.g. ``[null, 42]`` -> INTEGER), and an + all-NULL column resolves to TEXT. + """ + if value is None: + return None + return sql_type_for(value, self.dialect) + + def _merge_type(self, current: str | None, inferred: str) -> str: + """Merge a previously inferred column type with a new value's type. + + Returns ``TEXT`` whenever the two types are incompatible, because TEXT + is the only column type that is valid across Postgres/MySQL/SQLite. + This guarantees the generated INSERTs are executable for every dialect + instead of silently emitting a quoted string literal into a numeric + column (invalid SQL on Postgres/MySQL). + """ + if current is None: + return inferred + if current == inferred: + return current + return "TEXT" + def _infer_columns(self, objects: list[dict]) -> dict[str, str]: - """Infer column names and types from a list of objects.""" - columns: dict[str, str] = {} + """Infer column names and types from a list of objects. + + Type inference is order-independent and always yields SQL that is + valid for every supported dialect: once a column has seen values of + two incompatible types it collapses to TEXT. + """ + columns: dict[str, str | None] = {} for obj in objects: for key, value in obj.items(): + inferred = self._infer_type(value) if key not in columns: - columns[key] = sql_type_for(value, self.dialect) - elif value is not None: - # Upgrade type if we find a non-null value - inferred = sql_type_for(value, self.dialect) - if columns[key] == "TEXT" and inferred != "TEXT": - columns[key] = inferred - return columns + columns[key] = inferred + elif inferred is not None: + columns[key] = self._merge_type(columns[key], inferred) + # Resolve columns that never received a concrete value (all NULL) to TEXT. + return {k: (v if v is not None else "TEXT") for k, v in columns.items()} def _infer_columns_flattened( self, objects: list[dict], table_name: str @@ -157,20 +203,21 @@ def _infer_columns_flattened( and flat_map maps flattened column name -> (parent_key, sub_key) for resolving values during row construction. """ - columns: dict[str, str] = {} + columns: dict[str, str | None] = {} flat_map: dict[str, tuple[str, str]] = {} for obj in objects: for key, value in obj.items(): if isinstance(value, dict) and self.flatten: for sub_key, sub_value in value.items(): flat_key = f"{key}_{sub_key}" + inferred = self._infer_type(sub_value) if flat_key not in columns: - columns[flat_key] = sql_type_for(sub_value, self.dialect) + columns[flat_key] = inferred flat_map[flat_key] = (key, sub_key) - elif sub_value is not None: - inferred = sql_type_for(sub_value, self.dialect) - if columns[flat_key] == "TEXT" and inferred != "TEXT": - columns[flat_key] = inferred + elif inferred is not None: + columns[flat_key] = self._merge_type( + columns[flat_key], inferred + ) elif ( isinstance(value, list) and value @@ -180,13 +227,13 @@ def _infer_columns_flattened( # Skip - goes to separate table pass else: + inferred = self._infer_type(value) if key not in columns: - columns[key] = sql_type_for(value, self.dialect) - elif value is not None: - inferred = sql_type_for(value, self.dialect) - if columns[key] == "TEXT" and inferred != "TEXT": - columns[key] = inferred - return columns, flat_map + columns[key] = inferred + elif inferred is not None: + columns[key] = self._merge_type(columns[key], inferred) + resolved = {k: (v if v is not None else "TEXT") for k, v in columns.items()} + return resolved, flat_map def _flatten_nested( self, diff --git a/tests/test_edge_cases.py b/tests/test_edge_cases.py index 0468300..5ac5b73 100644 --- a/tests/test_edge_cases.py +++ b/tests/test_edge_cases.py @@ -53,38 +53,46 @@ def test_flatten_mixed_nested_and_simple_values(self): assert "'missing'" in result or "NULL" in result def test_flatten_type_upgrade_for_flattened_column(self): - """flattened column type upgrades from TEXT to more specific type (line 165).""" + """Flattened column mixing a string and int collapses to TEXT (line 165).""" from json2sql.converter import JSONToSQLConverter converter = JSONToSQLConverter(flatten=True) data = [ - {"id": 1, "meta": {"count": "five"}}, # string → TEXT - {"id": 2, "meta": {"count": 42}}, # integer → upgrade + {"id": 1, "meta": {"count": "five"}}, # string -> TEXT + {"id": 2, "meta": {"count": 42}}, # integer -> mixed -> TEXT ] result = converter.convert(json.dumps(data)) assert "meta_count" in result - # After upgrade, type should be INTEGER not TEXT - assert "INTEGER" in result + # The flattened meta_count column saw a string AND an integer, so it must + # be TEXT (not widened to INTEGER) to keep the generated SQL valid. + assert "TEXT" in result assert "'five'" in result assert "42" in result def test_type_upgrade_for_top_level_column(self): - """Top-level column type upgrades from TEXT to more specific type (line 175).""" + """Mixed-type column collapses to TEXT so the generated SQL is valid. + + A column that mixes a string and an integer must become TEXT (not + INTEGER): otherwise the string literal would be inserted into a numeric + column and rejected by Postgres/MySQL. See ``converter._merge_type``. + """ from json2sql.converter import JSONToSQLConverter data = [ - {"key": "hello"}, # string → TEXT - {"key": 100}, # integer → upgrade to INTEGER + {"key": "hello"}, # string -> TEXT + {"key": 100}, # integer -> would be INTEGER, but mixed -> TEXT ] # Non-flatten path (_infer_columns) result = JSONToSQLConverter().convert(json.dumps(data)) - assert "INTEGER" in result + assert "TEXT" in result + assert "INTEGER" not in result # must NOT widen to a numeric type assert "'hello'" in result assert "100" in result - # Flatten path (_infer_columns_flattened else branch, line 175) + # Flatten path (_infer_columns_flattened else branch) result2 = JSONToSQLConverter(flatten=True).convert(json.dumps(data)) - assert "INTEGER" in result2 + assert "TEXT" in result2 + assert "INTEGER" not in result2 assert "'hello'" in result2 assert "100" in result2 diff --git a/tests/test_type_inference.py b/tests/test_type_inference.py new file mode 100644 index 0000000..cf6d693 --- /dev/null +++ b/tests/test_type_inference.py @@ -0,0 +1,133 @@ +"""Regression tests for type inference and valid-SQL edge cases. + +These guard the two correctness fixes in converter.py: + +1. Mixed-type columns collapse to TEXT so the generated INSERTs are valid for + every dialect (Postgres/MySQL would reject a quoted string literal placed + into a numeric column). +2. Empty / nested-only roots emit valid SQL (or an explicit comment) instead of + an invalid ``CREATE TABLE "x" ();`` / ``INSERT INTO "x" () VALUES ();``. +""" + +from __future__ import annotations + +import json +import re + +import pytest + +from json2sql.converter import JSONToSQLConverter +from json2sql.dialects import Dialect + +NUMERIC = {"INT", "INTEGER", "DOUBLE PRECISION", "DOUBLE", "REAL"} + + +def column_type(out: str, col: str, dialect: Dialect) -> str | None: + """Extract the SQL type declared for ``col`` from a CREATE TABLE in ``out``.""" + qcol = f"`{col}`" if dialect == Dialect.MYSQL else f'"{col}"' + m = re.search(re.escape(qcol) + r"\s+(\w+(?:\([^)]*\))?)", out) + return m.group(1) if m else None + + +@pytest.mark.parametrize("dialect", [Dialect.POSTGRES, Dialect.MYSQL, Dialect.SQLITE]) +def test_mixed_string_then_int_collapses_to_text(dialect): + """A string followed by an int must become TEXT, not a numeric type.""" + out = JSONToSQLConverter(dialect=dialect).convert( + json.dumps([{"k": "hello"}, {"k": 100}]) + ) + assert column_type(out, "k", dialect) == "TEXT" + assert "'hello'" in out and "100" in out + + +@pytest.mark.parametrize("dialect", [Dialect.POSTGRES, Dialect.MYSQL, Dialect.SQLITE]) +def test_mixed_int_then_string_collapses_to_text(dialect): + """An int followed by a string must also become TEXT (order independence).""" + out = JSONToSQLConverter(dialect=dialect).convert( + json.dumps([{"k": 1}, {"k": "hello"}]) + ) + assert column_type(out, "k", dialect) == "TEXT" + + +def test_mixed_int_then_float_collapses_to_text(): + """Two incompatible concrete types also collapse to TEXT.""" + out = JSONToSQLConverter().convert(json.dumps([{"k": 1}, {"k": 2.5}])) + assert column_type(out, "k", Dialect.POSTGRES) == "TEXT" + + +@pytest.mark.parametrize("dialect", [Dialect.POSTGRES, Dialect.MYSQL, Dialect.SQLITE]) +def test_null_then_int_upgrades_to_int(dialect): + """A NULL first does not pin the column to TEXT; a later int wins.""" + out = JSONToSQLConverter(dialect=dialect).convert( + json.dumps([{"k": None}, {"k": 42}]) + ) + assert column_type(out, "k", dialect) in NUMERIC + + +@pytest.mark.parametrize("dialect", [Dialect.POSTGRES, Dialect.MYSQL, Dialect.SQLITE]) +def test_int_then_null_stays_int(dialect): + """A trailing NULL must not downgrade an established concrete type.""" + out = JSONToSQLConverter(dialect=dialect).convert( + json.dumps([{"k": 7}, {"k": None}]) + ) + assert column_type(out, "k", dialect) in NUMERIC + + +@pytest.mark.parametrize("dialect", [Dialect.POSTGRES, Dialect.MYSQL, Dialect.SQLITE]) +def test_all_null_column_defaults_to_text(dialect): + """An all-NULL column resolves to TEXT rather than an unset/empty type.""" + out = JSONToSQLConverter(dialect=dialect).convert( + json.dumps([{"k": None}, {"k": None}]) + ) + assert column_type(out, "k", dialect) == "TEXT" + + +@pytest.mark.parametrize("dialect", [Dialect.POSTGRES, Dialect.MYSQL, Dialect.SQLITE]) +def test_null_then_string_becomes_text(dialect): + """NULL first, then a string -> a text column (not an unset numeric type). + + A NULL is "unset", so the later string wins; MySQL renders this as + VARCHAR(255), Postgres/SQLite as TEXT. The key invariant is that it is a + string type, never INTEGER/INT. + """ + out = JSONToSQLConverter(dialect=dialect).convert( + json.dumps([{"k": None}, {"k": "x"}]) + ) + assert column_type(out, "k", dialect) in {"TEXT", "VARCHAR(255)"} + + +def test_empty_object_root_emits_no_invalid_sql(): + """An empty object must not produce ``CREATE TABLE "data" ();``.""" + out = JSONToSQLConverter().convert(json.dumps({})) + assert "CREATE TABLE" not in out + assert "INSERT INTO" not in out + # An explicit, observable comment instead of a silent empty string. + assert out.startswith("-- ") + + +def test_generate_schema_empty_object_emits_comment(): + """generate_schema on an empty object returns a comment, not invalid SQL.""" + out = JSONToSQLConverter().generate_schema(json.dumps({})) + assert "CREATE TABLE" not in out + assert out.startswith("-- ") + + +@pytest.mark.parametrize("dialect", [Dialect.POSTGRES, Dialect.MYSQL, Dialect.SQLITE]) +def test_nested_array_only_root_keeps_child_tables_only(dialect): + """Flatten of a root with only a nested array emits child table, no bad root.""" + out = JSONToSQLConverter(dialect=dialect, flatten=True).convert( + json.dumps({"items": [{"x": 1}, {"x": 2}]}) + ) + assert "data_items" in out + assert column_type(out, "x", dialect) in NUMERIC + # The bogus empty root CREATE/INSERT must be absent. + assert 'CREATE TABLE "data" (\n\n);' not in out + assert "INSERT INTO \"data\" ()" not in out + + +def test_convert_never_emits_empty_column_list(): + """No generated statement may contain a zero-column table or insert.""" + out = JSONToSQLConverter(flatten=True).convert( + json.dumps([{"a": 1, "b": [{"c": 2}]}, {"a": 3}]) + ) + assert "();" not in out + assert "INSERT INTO" in out