codeant-ai-for-open-source[bot] commented on code in PR #42296: URL: https://github.com/apache/superset/pull/42296#discussion_r3626625933
########## superset-frontend/plugins/plugin-chart-echarts/src/utils/xAxisDrillByFilter.ts: ########## @@ -0,0 +1,137 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import { + BinaryQueryObjectFilterClause, + QueryFormColumn, + TimeGranularity, +} from '@superset-ui/core'; + +const DAY_MS = 24 * 60 * 60 * 1000; + +/** + * Format a Date as a naive ISO datetime string (UTC, no timezone suffix), + * the format Superset's time range parser expects, e.g. "2021-01-01T00:00:00". + */ +export const formatNaiveDateTime = (date: Date): string => + date.toISOString().slice(0, 19); + +/** + * Given the start (label) of a time bucket and its time grain, return the + * [since, until) range covering the bucket, using calendar-aware UTC + * arithmetic. Week-ending grains are labeled by the last day of the bucket, + * so their range extends backwards from the label. Returns undefined for + * unknown grains. + */ +export const getTimeBucketRange = ( + bucketLabel: Date, + grain: TimeGranularity, +): { since: Date; until: Date } | undefined => { + const until = new Date(bucketLabel.getTime()); + switch (grain) { + case TimeGranularity.SECOND: + until.setUTCSeconds(until.getUTCSeconds() + 1); + break; + case TimeGranularity.MINUTE: + until.setUTCMinutes(until.getUTCMinutes() + 1); + break; + case TimeGranularity.FIVE_MINUTES: + until.setUTCMinutes(until.getUTCMinutes() + 5); + break; + case TimeGranularity.TEN_MINUTES: + until.setUTCMinutes(until.getUTCMinutes() + 10); + break; + case TimeGranularity.FIFTEEN_MINUTES: + until.setUTCMinutes(until.getUTCMinutes() + 15); + break; + case TimeGranularity.THIRTY_MINUTES: + until.setUTCMinutes(until.getUTCMinutes() + 30); + break; + case TimeGranularity.HOUR: + until.setUTCHours(until.getUTCHours() + 1); + break; + case TimeGranularity.DATE: + case TimeGranularity.DAY: + until.setUTCDate(until.getUTCDate() + 1); + break; + case TimeGranularity.WEEK: + case TimeGranularity.WEEK_STARTING_SUNDAY: + case TimeGranularity.WEEK_STARTING_MONDAY: + until.setUTCDate(until.getUTCDate() + 7); + break; + case TimeGranularity.WEEK_ENDING_SATURDAY: + case TimeGranularity.WEEK_ENDING_SUNDAY: + // These buckets are labeled with their last day: the bucket spans + // the 6 days before the label plus the label day itself. + until.setUTCDate(until.getUTCDate() + 1); + return { since: new Date(bucketLabel.getTime() - 6 * DAY_MS), until }; + case TimeGranularity.MONTH: + until.setUTCMonth(until.getUTCMonth() + 1); + break; + case TimeGranularity.QUARTER: + until.setUTCMonth(until.getUTCMonth() + 3); + break; + case TimeGranularity.YEAR: + until.setUTCFullYear(until.getUTCFullYear() + 1); + break; + default: + return undefined; + } + return { since: new Date(bucketLabel.getTime()), until }; +}; + +/** + * Build a drill-by filter clause matching the clicked value on a temporal + * x-axis. When a known time grain is active, the clause is a TEMPORAL_RANGE + * covering the clicked bucket; otherwise it falls back to an exact match on + * the timestamp. + */ +export const getTemporalXAxisDrillByFilter = ( + col: QueryFormColumn, + value: unknown, + grain?: TimeGranularity, + formattedVal?: string, +): BinaryQueryObjectFilterClause | undefined => { + if ( + !col || + (typeof value !== 'number' && + typeof value !== 'string' && + !(value instanceof Date)) + ) { + return undefined; + } + const bucketLabel = value instanceof Date ? value : new Date(value); + if (Number.isNaN(bucketLabel.getTime())) { + return undefined; + } + const range = grain ? getTimeBucketRange(bucketLabel, grain) : undefined; + if (!range) { + return { + col, + op: '==', + val: formatNaiveDateTime(bucketLabel), + formattedVal, + }; Review Comment: **Suggestion:** The exact-match fallback truncates timestamps to whole seconds, so temporal values containing milliseconds/microseconds can fail to match the clicked row. Preserve full timestamp precision for `==` filters instead of slicing off fractional seconds. [logic error] <details> <summary><b>Severity Level:</b> Major ⚠️</summary> ```mdx - ❌ Drill By on raw timestamps can return empty results. - ⚠️ Users drilling into high-frequency time data misfiltered. - ⚠️ Temporal x-axis drill behavior inconsistent with displayed data. ``` </details> <details> <summary><b>Steps of Reproduction ✅ </b></summary> ```mdx 1. Create a Time Series / ECharts Timeseries chart using a temporal column with sub-second precision and no explicit time grain (formData.time_grain_sqla unset), so the x-axis uses raw timestamps (caller: superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/EchartsTimeseries.tsx). 2. Open the chart in a dashboard and right-click a point whose underlying timestamp includes fractional seconds (e.g. 2024-01-01T00:00:00.123). 3. The chart context menu (superset-frontend/src/components/Chart/ChartContextMenu/ChartContextMenu.tsx) receives drill-by filters including an x-axis filter built via getTemporalXAxisDrillByFilter() (superset-frontend/plugins/plugin-chart-echarts/src/utils/xAxisDrillByFilter.ts:104-137) with grain undefined. 4. In getTemporalXAxisDrillByFilter(), range is falsy so the fallback branch executes (lines 124-129), setting val to formatNaiveDateTime(bucketLabel), which truncates milliseconds to whole seconds (line 31). The backend equality filter compares '2024-01-01T00:00:00' to the row value '2024-01-01T00:00:00.123', the comparison fails, and the Drill By result shows no rows for the clicked point. ``` </details> [](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=c570c531c5f44c988631506c633c8390&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=c570c531c5f44c988631506c633c8390&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) *(Use Cmd/Ctrl + Click for best experience)* <details> <summary><b>Prompt for AI Agent 🤖 </b></summary> ```mdx This is a comment left during a code review. **Path:** superset-frontend/plugins/plugin-chart-echarts/src/utils/xAxisDrillByFilter.ts **Line:** 124:129 **Comment:** *Logic Error: The exact-match fallback truncates timestamps to whole seconds, so temporal values containing milliseconds/microseconds can fail to match the clicked row. Preserve full timestamp precision for `==` filters instead of slicing off fractional seconds. 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%2F42296&comment_hash=e160df88edab92ab6544603890b2bba23ddc7259b549bb31a9ea71c798b287e7&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42296&comment_hash=e160df88edab92ab6544603890b2bba23ddc7259b549bb31a9ea71c798b287e7&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: [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]
