bito-code-review[bot] commented on code in PR #43000:
URL: https://github.com/apache/superset/pull/43000#discussion_r3803375514
##########
superset/db_engine_specs/clickhouse.py:
##########
@@ -161,6 +162,11 @@ def is_read_limit_error(cls, ex: Exception) -> bool:
types.DECIMAL(),
GenericDataType.NUMERIC,
),
+ (
+ re.compile(r".*Float.*", re.IGNORECASE),
+ types.Float(),
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Duplicate type mapping patterns in specs</b></div>
<div id="fix">
Detected syntactic duplication of type mapping and datetime conversion logic
across two database engine specification files. The patterns for Float,
DateTime, and Date type mappings along with the epoch_to_dttm and convert_dttm
methods are duplicated in clickhouse.py (lines 167-193) and databend.py (lines
121-147). Consider creating a shared base class or utility module to
consolidate this logic and reduce maintenance burden.
</div>
</div>
<small><i>Code Review Run #8374aa</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
tests/unit_tests/db_engine_specs/test_clickhouse.py:
##########
@@ -193,6 +198,19 @@ def test_connect_convert_dttm(
),
("Float32", Float, None, GenericDataType.NUMERIC, False),
("Float64", Float, None, GenericDataType.NUMERIC, False),
+ # The base spec's float pattern is anchored (`^float`), so wrapped
float
+ # types only resolve as NUMERIC thanks to the ClickHouse-specific
+ # `.*Float.*` mapping. File upload creates `Nullable(Float64)` columns,
+ # which would otherwise be typed as STRING in Superset.
+ ("Nullable(Float32)", Float, None, GenericDataType.NUMERIC, False),
+ ("Nullable(Float64)", Float, None, GenericDataType.NUMERIC, False),
+ (
+ "LowCardinality(Nullable(Float64))",
+ Float,
+ None,
+ GenericDataType.NUMERIC,
+ False,
+ ),
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Duplicate test data tuples across files</b></div>
<div id="fix">
Detected syntactic duplication in test data setup across two test files. The
test data tuples for numeric types (Decimal variants), boolean types, and date
types are duplicated across test_clickhouse.py (lines 213-223) and
test_databend.py (lines 138-148). Consider extracting this test data into a
shared fixture to improve maintainability and reduce code duplication.
</div>
</div>
<small><i>Code Review Run #8374aa</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
