Skip to content

Commit

Permalink
feat(FIR-42592): allow agg index without key columns (#150)
Browse files Browse the repository at this point in the history
  • Loading branch information
ptiurin authored Jan 15, 2025
1 parent c53bc05 commit b78b9c0
Show file tree
Hide file tree
Showing 5 changed files with 28 additions and 13 deletions.
3 changes: 3 additions & 0 deletions .changes/unreleased/Changed-20250115-095239.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
kind: Changed
body: Allow aggregating indexes without key columns to be specified.
time: 2025-01-15T09:52:39.233654Z
6 changes: 4 additions & 2 deletions dbt/adapters/firebolt/connections.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
)
from firebolt.client import DEFAULT_API_URL
from firebolt.client.auth import Auth, ClientCredentials, UsernamePassword
from firebolt.common._types import ExtendedType
from firebolt.db import ARRAY, DECIMAL
from firebolt.db import connect as sdk_connect
from firebolt.db.connection import Connection as SDKConnection
Expand Down Expand Up @@ -191,8 +192,8 @@ def cancel(self, connection: Connection) -> None:
raise NotImplementedError('`cancel` is not implemented for this adapter!')

@classmethod
def data_type_code_to_name( # type: ignore[override] # FIR-29423
cls, type_code: Union[type, ARRAY, DECIMAL]
def data_type_code_to_name(
cls, type_code: Union[type, ExtendedType] # type: ignore[override] # FIR-29423
) -> str:
"""
Convert a Firebolt data type code to a string representing the data type.
Expand All @@ -218,6 +219,7 @@ def data_type_code_to_name( # type: ignore[override] # FIR-29423
return 'bytea'
else:
return 'text'
return 'text'


def _determine_auth(credentials: FireboltCredentials) -> Auth:
Expand Down
5 changes: 2 additions & 3 deletions dbt/adapters/firebolt/impl.py
Original file line number Diff line number Diff line change
Expand Up @@ -120,13 +120,12 @@ def parse(cls, raw_index: Optional[Mapping]) -> Optional['FireboltIndexConfig']:
'for join indexes.'
)
if index_config.index_type.upper() == 'AGGREGATING' and not (
index_config.key_columns and index_config.aggregation
index_config.aggregation
):
raise CompilationError(
'Invalid aggregating index definition:\n'
f' Got: {index_config}.\n'
' key_columns and aggregation must be specified '
'for aggregating indexes.'
' aggregation must be specified for aggregating indexes.'
)
return index_config
except ValidationError as exc:
Expand Down
14 changes: 8 additions & 6 deletions dbt/include/firebolt/macros/adapters.sql
Original file line number Diff line number Diff line change
Expand Up @@ -75,18 +75,20 @@
create_statement (str): either "CREATE JOIN INDEX" or
"CREATE AND GENERATE AGGREGATING INDEX"
spine_col ([str]):
if agg index, key columns
if agg index, key columns - can be none
if join index, join column
other_col ([str]):
if agg index, aggregating columns
if join index, dimension column
#}
{{ create_statement }} "{{ index_name }}" ON {{ relation }} (
{% if spine_col is iterable and spine_col is not string -%}
{{ spine_col | join(', ') }},
{% else -%}
{{ spine_col }},
{% endif -%}
{% if spine_col -%}
{% if spine_col is iterable and spine_col is not string -%}
{{ spine_col | join(', ') }},
{% else -%}
{{ spine_col }},
{% endif -%}
{%- endif -%}
{% if other_col is iterable and other_col is not string -%}
{{ other_col | join(', ') }}
{%- else -%}
Expand Down
13 changes: 11 additions & 2 deletions tests/unit/test_index.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,17 @@ def test_firebolt_index_config_parse():
assert index_config.dimension_column == '"dimension"'


def test_firebolt_index_config_no_key_columns():
raw_index = {
'index_type': 'aggregating',
'aggregation': 'COUNT(*)',
}
index_config = FireboltIndexConfig.parse(raw_index)
assert index_config.index_type == 'aggregating'
assert index_config.key_columns is None
assert index_config.aggregation == 'COUNT(*)'


@pytest.mark.parametrize(
'raw_index',
[
Expand All @@ -84,8 +95,6 @@ def test_firebolt_index_config_parse():
},
# missing dimension column
{'index_type': 'join', 'join_columns': ['column1']},
# missing aggregation
{'index_type': 'aggregating', 'key_columns': ['key1']},
# missing aggregation but containing extra columns
{
'index_type': 'aggregating',
Expand Down

0 comments on commit b78b9c0

Please sign in to comment.