Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support new agate Integer data_type in adapter code #9004

Merged
merged 3 commits into from
Nov 7, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changes/unreleased/Fixes-20231031-144837.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
kind: Fixes
body: Fix exception running empty seed file
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR and this changelog indicate we're solving multiple issues, can we add a second changelog to cover new data_type?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't solve multiple issues, it's all the same issue. The problem with the empty seed file was happening because of the lack of support for the new Integer data type.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, you mean additional info in the existing changelog? Sure.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yup! not questioning the PR just want to make sure our changelogs cover everything that's being changed

time: 2023-10-31T14:48:37.774871-04:00
custom:
Author: gshank
Issue: "8895"
13 changes: 13 additions & 0 deletions core/dbt/adapters/base/impl.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@
get_column_value_uncased,
merge_tables,
table_from_rows,
Integer,
)
from dbt.clients.jinja import MacroGenerator
from dbt.contracts.graph.manifest import Manifest, MacroManifest
Expand Down Expand Up @@ -962,6 +963,17 @@
"""
raise NotImplementedError("`convert_number_type` is not implemented for this adapter!")

@classmethod
def convert_integer_type(cls, agate_table: agate.Table, col_idx: int) -> str:
"""Return the type in the database that best maps to the agate.Number
type for the given agate table and column index.

:param agate_table: The table
:param col_idx: The index into the agate table for the column.
:return: The name of the type in the database
"""
return "integer"

Check warning on line 975 in core/dbt/adapters/base/impl.py

View check run for this annotation

Codecov / codecov/patch

core/dbt/adapters/base/impl.py#L975

Added line #L975 was not covered by tests

@classmethod
@abc.abstractmethod
def convert_boolean_type(cls, agate_table: agate.Table, col_idx: int) -> str:
Expand Down Expand Up @@ -1019,6 +1031,7 @@
def convert_agate_type(cls, agate_table: agate.Table, col_idx: int) -> Optional[str]:
agate_type: Type = agate_table.column_types[col_idx]
conversions: List[Tuple[Type, Callable[..., str]]] = [
(Integer, cls.convert_integer_type),
(agate.Text, cls.convert_text_type),
(agate.Number, cls.convert_number_type),
(agate.Boolean, cls.convert_boolean_type),
Expand Down
4 changes: 4 additions & 0 deletions core/dbt/adapters/sql/impl.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,10 @@ def convert_number_type(cls, agate_table: agate.Table, col_idx: int) -> str:
decimals = agate_table.aggregate(agate.MaxPrecision(col_idx)) # type: ignore[attr-defined]
return "float8" if decimals else "integer"

@classmethod
def convert_integer_type(cls, agate_table: agate.Table, col_idx: int) -> str:
return "integer"

@classmethod
def convert_boolean_type(cls, agate_table: agate.Table, col_idx: int) -> str:
return "boolean"
Expand Down
23 changes: 23 additions & 0 deletions tests/functional/seeds/test_seeds.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import pytest
from dbt.tests.util import run_dbt

# This is for seed tests on dbt-core code, not adapter code


class TestEmptySeed:
@pytest.fixture(scope="class")
def project_config_update(self):
return {
"seeds": {
"quote_columns": False,
},
}

@pytest.fixture(scope="class")
def seeds(self):
return {"empty_with_header.csv": "a,b,c"}

def test_empty_seeds(self, project):
# Should create an empty table and not fail
results = run_dbt(["seed"])
assert len(results) == 1
3 changes: 3 additions & 0 deletions tests/unit/mock_adapter.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,9 @@ def convert_text_type(self, *args, **kwargs):
def convert_number_type(self, *args, **kwargs):
return self.responder.convert_number_type(*args, **kwargs)

def convert_integer_type(self, *args, **kwargs):
return self.responder.convert_integer_type(*args, **kwargs)

def convert_boolean_type(self, *args, **kwargs):
return self.responder.convert_boolean_type(*args, **kwargs)

Expand Down
Loading