Fix cluster hint overriding partition hint on bigquery #3497
Conversation
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
docs | c0fcefd | Commit Preview URL Branch Preview URL |
Dec 18 2025, 02:23 PM |
| assert expected_clause in sql_partitioned | ||
|
|
||
|
|
||
| def test_adapter_hints_comprehensive_single_column() -> None: |
There was a problem hiding this comment.
A unit test is good. But this particular scenario likely already worked before the fix because bigquery_adapter() below creates a new column_hints = {} each time.
It's best to add also a unit test for the actual bug: a single call with both partition and cluster on the same column.
| destinations_configs(default_sql_configs=True, subset=["bigquery"]), | ||
| ids=lambda x: x.name, | ||
| ) | ||
| def test_adapter_hints_clustering_and_partitioning( |
|
|
||
| if isinstance(partition, str): | ||
| column_hints[partition] = {"name": partition, PARTITION_HINT: True} # type: ignore[typeddict-unknown-key] | ||
| column_hints.setdefault(partition, {"name": partition})[PARTITION_HINT] = True # type: ignore[typeddict-unknown-key] |
There was a problem hiding this comment.
This fix is correct.
Here's also a chance to improve code quality: this pattern of setting column hints repeats 4 times. Let's extract this pattern into a helper function (e.g. _set_column_hint()) for maintainability. Most likely it could be used in other adapters, but this is a for a follow-up PR. For now I'd keep _set_column_hint in this module.
Fixes a bug where applying both a partition hint and a cluster hint on the same column would result in the partition hint being silently dropped.
Closes #3471