codeant-ai-for-open-source[bot] commented on code in PR #37517:
URL: https://github.com/apache/superset/pull/37517#discussion_r2740831126
##########
superset-frontend/src/explore/reducers/exploreReducer.js:
##########
@@ -124,7 +124,15 @@ export default function exploreReducer(state = {}, action)
{
},
[actions.SET_FIELD_VALUE]() {
const { controlName, value, validationErrors } = action;
- let new_form_data = { ...state.form_data, [controlName]: value };
+ // Normalize server_page_length to number if it's a non-empty string
representing an integer
+ const normalizedValue =
+ controlName === 'server_page_length' &&
+ typeof value === 'string' &&
+ value.trim() !== '' &&
+ Number.isInteger(Number(value))
+ ? Number(value)
Review Comment:
**Suggestion:** The normalization only accepts integer strings via
Number.isInteger(Number(value)), which will leave valid numeric strings with
decimals (e.g., "99.5") as strings and still trigger backend comparison errors;
change the check to accept any finite numeric string and convert using
Number(value.trim()) so numeric floats are normalized too. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Table chart pagination can fail on decimal input.
- ⚠️ Explore form_data may contain string page sizes.
- ⚠️ Backend comparisons may raise TypeError.
```
</details>
```suggestion
// Normalize server_page_length to number if it's a non-empty string
representing a numeric value
const normalizedValue =
controlName === 'server_page_length' &&
typeof value === 'string' &&
value.trim() !== '' &&
Number.isFinite(Number(value.trim()))
? Number(value.trim())
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open Explore and edit a table-style chart that exposes the
server_page_length control
in the UI.
The frontend control dispatches a SET_FIELD_VALUE action handled in
superset-frontend/src/explore/reducers/exploreReducer.js at the handler
[actions.SET_FIELD_VALUE]()
(see lines ~125-135 in the reducer). The reducer receives the action in
the function
starting
at line 125 and destructures action as `const { controlName, value,
validationErrors }
= action;`.
2. In the UI, type a decimal numeric value like "99.5" into the freeForm
SelectControl for
server_page_length
and confirm (this triggers the same SET_FIELD_VALUE action above). The
reducer then
reaches the
normalization block at the lines shown in this suggestion (~lines
127-134).
3. The current code checks Number.isInteger(Number(value)) (line ~132). For
"99.5"
Number("99.5") is 99.5,
Number.isInteger(99.5) is false, so normalizedValue remains the original
string "99.5".
Immediately after (line ~135) the reducer writes new_form_data with the
unconverted
string:
`let new_form_data = { ...state.form_data, [controlName]: normalizedValue
};`.
4. The unchanged string "99.5" is sent to the backend as part of form_data
when the chart
issues its query
(this behavior is the explored flow described in the PR). The backend
expects an
integer/number and,
as reported in the PR, can raise a Python TypeError ("'<' not supported
between
instances of 'str' and 'int'").
This reproduces the failure when decimals are submitted: the frontend
leaves "99.5" as
a string which
can trigger the backend comparison error.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/src/explore/reducers/exploreReducer.js
**Line:** 127:133
**Comment:**
*Logic Error: The normalization only accepts integer strings via
Number.isInteger(Number(value)), which will leave valid numeric strings with
decimals (e.g., "99.5") as strings and still trigger backend comparison errors;
change the check to accept any finite numeric string and convert using
Number(value.trim()) so numeric floats are normalized too.
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.
```
</details>
--
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]
