korbit-ai[bot] commented on code in PR #32235:
URL: https://github.com/apache/superset/pull/32235#discussion_r1952831571
##########
superset/migrations/shared/native_filters.py:
##########
@@ -85,7 +84,7 @@ def convert_filter_scopes_to_native_filters( # pylint:
disable=invalid-name,too
fltr: dict[str, Any] = {
"cascadeParentIds": [],
- "id": f"NATIVE_FILTER-{shortid.generate()}",
+ "id": f"NATIVE_FILTER-{short_id}",
Review Comment:
### Reused filter ID across iterations <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The same short_id is reused for all filters within the iteration loop,
leading to duplicate filter IDs.
###### Why this matters
Multiple filters will have the same ID, causing filter operations to
malfunction and potentially break dashboard functionality.
###### Suggested change ∙ *Feature Preview*
Generate a new unique ID for each filter. Example:
```python
for field, filter_scope in filter_scope_by_key_and_field[key].items():
short_id = str(uuid.uuid4()) # Generate new ID for each filter
fltr: dict[str, Any] = {
"cascadeParentIds": [],
"id": f"NATIVE_FILTER-{short_id}",
```
</details>
<sub>
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/086a73d8-7fac-4ae5-9021-0d366d4b9175?suggestedFixEnabled=true)
💬 Chat with Korbit by mentioning @korbit-ai.
</sub>
<!--- korbi internal id:ea463fca-7eb2-407d-a0bc-f455c9126d97 -->
##########
superset/migrations/shared/native_filters.py:
##########
@@ -49,7 +48,7 @@
:see: convert_filter_scopes
"""
- shortid = ShortId()
+ short_id = f"{shortid()}"[:9]
Review Comment:
### Unsafe UUID truncation <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The string slicing of the new UUID to 9 characters without additional
uniqueness guarantees could lead to potential ID collisions.
###### Why this matters
Truncating UUIDs arbitrarily can significantly reduce the uniqueness
guarantee and lead to duplicate IDs being generated for native filters, causing
unexpected behavior in filter operations.
###### Suggested change ∙ *Feature Preview*
Use a full UUID or a more robust method to generate shorter unique IDs.
Example:
```python
# Option 1: Use full UUID
short_id = str(uuid.uuid4())
# Option 2: Use base64 encoding of UUID to get shorter but still unique IDs
import base64
import uuid
short_id =
base64.urlsafe_b64encode(uuid.uuid4().bytes).decode('ascii').rstrip('=')[:11]
```
</details>
<sub>
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/3bd27ff9-dda4-434c-92c5-8daefd725640?suggestedFixEnabled=true)
💬 Chat with Korbit by mentioning @korbit-ai.
</sub>
<!--- korbi internal id:a438aa0b-6436-402f-88aa-45eb74e6c42b -->
--
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]
