Skip to content

Commit

Permalink
fix(taps): Support non-nullable but not required SQL columns (#2225)
Browse files Browse the repository at this point in the history
Closes #2224
  • Loading branch information
edgarrmondragon authored Feb 9, 2024
1 parent b7b8d15 commit 89d6afc
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 5 deletions.
3 changes: 2 additions & 1 deletion singer_sdk/connectors/sql.py
Original file line number Diff line number Diff line change
Expand Up @@ -464,7 +464,8 @@ def discover_catalog_entry(
th.Property(
name=column_name,
wrapped=th.CustomType(jsonschema_type),
required=not is_nullable,
nullable=is_nullable,
required=column_name in key_properties if key_properties else False,
),
)
schema = table_schema.to_dict()
Expand Down
8 changes: 6 additions & 2 deletions singer_sdk/typing.py
Original file line number Diff line number Diff line change
Expand Up @@ -517,7 +517,7 @@ class Property(JSONTypeHelper[T], t.Generic[T]):
"""Generic Property. Should be nested within a `PropertiesList`."""

# TODO: Make some of these arguments keyword-only. This is a breaking change.
def __init__(
def __init__( # noqa: PLR0913
self,
name: str,
wrapped: JSONTypeHelper[T] | type[JSONTypeHelper[T]],
Expand All @@ -527,6 +527,8 @@ def __init__(
secret: bool | None = False, # noqa: FBT002
allowed_values: list[T] | None = None,
examples: list[T] | None = None,
*,
nullable: bool | None = None,
) -> None:
"""Initialize Property object.
Expand All @@ -547,6 +549,7 @@ def __init__(
are permitted. This will define the type as an 'enum'.
examples: Optional. A list of one or more sample values. These may be
displayed to the user as hints of the expected format of inputs.
nullable: If True, the property may be null.
"""
self.name = name
self.wrapped = wrapped
Expand All @@ -556,6 +559,7 @@ def __init__(
self.secret = secret
self.allowed_values = allowed_values or None
self.examples = examples or None
self.nullable = nullable

@property
def type_dict(self) -> dict: # type: ignore[override]
Expand Down Expand Up @@ -585,7 +589,7 @@ def to_dict(self) -> dict:
A JSON Schema dictionary describing the object.
"""
type_dict = self.type_dict
if self.optional:
if self.nullable or self.optional:
type_dict = append_type(type_dict, "null")
if self.default is not None:
type_dict.update({"default": self.default})
Expand Down
9 changes: 8 additions & 1 deletion tests/samples/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,14 @@ def _sqlite_sample_db(sqlite_connector):
for t in range(3):
conn.execute(sa.text(f"DROP TABLE IF EXISTS t{t}"))
conn.execute(
sa.text(f"CREATE TABLE t{t} (c1 int PRIMARY KEY, c2 varchar(10))"),
sa.text(
f"""
CREATE TABLE t{t} (
c1 int PRIMARY KEY NOT NULL,
c2 varchar(10) NOT NULL
)
"""
),
)
for x in range(100):
conn.execute(
Expand Down
4 changes: 3 additions & 1 deletion tests/samples/test_tap_sqlite.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,8 @@ def test_sqlite_discovery(sqlite_sample_tap: SQLTap):

assert stream.metadata.root.table_key_properties == ["c1"]
assert stream.primary_keys == ["c1"]
assert stream.schema["properties"]["c1"] == {"type": ["integer"]}
assert stream.schema["required"] == ["c1"]


def test_sqlite_input_catalog(sqlite_sample_tap: SQLTap):
Expand All @@ -90,7 +92,7 @@ def test_sqlite_input_catalog(sqlite_sample_tap: SQLTap):

for schema in [stream.schema, stream.stream_maps[0].transformed_schema]:
assert len(schema["properties"]) == 2
assert schema["properties"]["c1"] == {"type": ["integer", "null"]}
assert schema["properties"]["c1"] == {"type": ["integer"]}
assert schema["properties"]["c2"] == {"type": ["string", "null"]}
assert stream.name == stream.tap_stream_id == "main-t1"

Expand Down

0 comments on commit 89d6afc

Please sign in to comment.