rusackas commented on PR #43017: URL: https://github.com/apache/superset/pull/43017#issuecomment-5442135014
Nice catch on the N+1 here, and the regression tests actually asserting `find_by_id` is never called are a good way to pin the behavior down. One thing worth checking before merge: `datasets_by_id` ends up keyed by `dataset.id` (an int), but the lookups use the raw `datasetId` pulled straight out of `json_metadata`, which `native_filter_cache.py` types as `int | str` elsewhere in the codebase. `find_by_ids` will still resolve a string id fine since it converts before querying, but `datasets_by_id.get(dataset_id)` would miss on an int/str key mismatch and silently drop that target's `datasetUuid`. Worth normalizing the collected ids to int before building the dict? -- 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]
