You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "I3eka (via GitHub)" <gi...@apache.org> on 2026/08/13 15:58:30 UTC
[PR] fix(mcp): defer unknown numeric types to compile [superset]
I3eka opened a new pull request, #43121:
URL: https://github.com/apache/superset/pull/43121
# fix(mcp): defer unknown numeric types to compile
### SUMMARY
Do not reject numeric aggregations solely because a virtual dataset column reports empty or `UNKNOWN` type metadata. The existing compile check remains the authoritative validation.
Base: `apache/superset:master` at `e2bb33b1da17c16a51e54d84bcf496516d67a713`.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Not applicable; backend behavior only.
### TESTING INSTRUCTIONS
`pytest -q tests/unit_tests/mcp_service/chart/validation/test_column_name_normalization.py`
Result: 49 passed.
The changed files also pass the repository pre-commit hooks.
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(mcp): defer unknown numeric types to compile [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #43121:
URL: https://github.com/apache/superset/pull/43121#issuecomment-5282960471
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(mcp): defer unknown numeric types to compile [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #43121:
URL: https://github.com/apache/superset/pull/43121#issuecomment-5283050992
<!-- Bito Reply -->
The flagged issue is correct. The current allowlist in `dataset_validator.py` uses a hardcoded list of strings, which fails to account for common SQL numeric type variations like `BIGINT`, `REAL`, or parameterized types like `DECIMAL(10,2)`.
To resolve this, you should replace the exact-match check with a more robust detection method, such as checking if the type string contains common numeric keywords or using a helper function that normalizes the type name.
Here is a concise implementation using a helper function:
```python
# In superset/mcp_service/chart/validation/dataset_validator.py
def _is_numeric_type(type_str: str) -> bool:
numeric_keywords = ["INT", "FLOAT", "DOUBLE", "DECIMAL", "NUMERIC", "REAL", "NUMBER"]
return any(keyword in type_str.upper() for keyword in numeric_keywords)
# Replace the list check with:
# and not _is_numeric_type(col_info.get("type", ""))
```
There are no other comments on this PR. Would you like me to proceed with implementing this change in the file?
**superset/mcp_service/chart/validation/dataset_validator.py**
```
def _is_numeric_type(type_str: str) -> bool:
numeric_keywords = ["INT", "FLOAT", "DOUBLE", "DECIMAL", "NUMERIC", "REAL", "NUMBER"]
return any(keyword in type_str.upper() for keyword in numeric_keywords)
```
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(mcp): defer unknown numeric types to compile [superset]
Posted by "I3eka (via GitHub)" <gi...@apache.org>.
I3eka closed pull request #43121: fix(mcp): defer unknown numeric types to compile
URL: https://github.com/apache/superset/pull/43121
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(mcp): defer unknown numeric types to compile [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #43121:
URL: https://github.com/apache/superset/pull/43121#discussion_r3776987729
##########
superset/mcp_service/chart/validation/dataset_validator.py:
##########
@@ -593,7 +593,15 @@ def _validate_aggregations(
col_ref.aggregate in numeric_aggs
and not col_info.get("is_numeric", False)
and col_info.get("type", "").upper()
- not in ["INTEGER", "FLOAT", "DOUBLE", "DECIMAL", "NUMERIC"]
+ not in [
+ "",
+ "UNKNOWN",
+ "INTEGER",
+ "FLOAT",
+ "DOUBLE",
+ "DECIMAL",
+ "NUMERIC",
+ ]
Review Comment:
**Suggestion:** The exact-match allowlist omits numeric type spellings such as `BIGINT`, `SMALLINT`, `TINYINT`, `REAL`, `NUMBER`, `DOUBLE PRECISION`, and parameterized types such as `DECIMAL(10,2)`. When metadata sets `is_numeric` to false for these valid numeric columns, `SUM`/`AVG`/other numeric aggregates are incorrectly rejected before compilation. Reuse the same token-based numeric detection used by the histogram validator or otherwise normalize these type names consistently. [type error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ SUM and AVG fail for valid backend numeric types.
- ⚠️ Affected charts never reach database compilation.
- ⚠️ Numeric metadata inconsistencies cause false validation errors.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=c640fe60aecf4d6ba078e451995cf12c&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=c640fe60aecf4d6ba078e451995cf12c&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/mcp_service/chart/validation/dataset_validator.py
**Line:** 596:604
**Comment:**
*Type Error: The exact-match allowlist omits numeric type spellings such as `BIGINT`, `SMALLINT`, `TINYINT`, `REAL`, `NUMBER`, `DOUBLE PRECISION`, and parameterized types such as `DECIMAL(10,2)`. When metadata sets `is_numeric` to false for these valid numeric columns, `SUM`/`AVG`/other numeric aggregates are incorrectly rejected before compilation. Reuse the same token-based numeric detection used by the histogram validator or otherwise normalize these type names consistently.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43121&comment_hash=eed60afdb94ae12aa63b185167e42c5049ba634e8d6975694c9497b7ef82651e&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43121&comment_hash=eed60afdb94ae12aa63b185167e42c5049ba634e8d6975694c9497b7ef82651e&reaction=dislike'>👎</a>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org