EnxDev commented on code in PR #43453:
URL: https://github.com/apache/superset/pull/43453#discussion_r3882296829
##########
superset-frontend/packages/superset-ui-core/src/number-format/index.ts:
##########
@@ -19,6 +19,8 @@
export { default as NumberFormats } from './NumberFormats';
export { default as NumberFormatter, PREVIEW_VALUE } from './NumberFormatter';
+export { formatSpecifier } from 'd3-format';
+export type { FormatLocaleDefinition } from 'd3-format';
Review Comment:
Fixed in f74b8768b9 — promoted `@types/d3-format` to `dependencies` in
`packages/superset-ui-core/package.json` since it's now part of a public
re-export.
##########
superset-frontend/package.json:
##########
@@ -146,7 +146,6 @@
"@superset-ui/plugin-chart-world-map":
"file:./plugins/plugin-chart-world-map",
"@superset-ui/preset-chart-deckgl": "file:./plugins/preset-chart-deckgl",
"@superset-ui/switchboard": "file:./packages/superset-ui-switchboard",
- "@types/d3-format": "^3.0.1",
Review Comment:
Fixed in f74b8768b9 — regenerated the lockfile (`npm install
--package-lock-only`), which drops that stale root-level entry.
##########
superset-frontend/src/components/Datasource/components/DatasourceEditor/DatasourceEditor.tsx:
##########
@@ -827,6 +828,64 @@ function EditorsSelector({
const ResultTable =
extensionsRegistry.get('sqleditor.extension.resultTable') ?? FilterableTable;
+// D3's '%' and 'p' types both multiply by 100; parsed via d3-format's own
+// grammar so garbage like "foo%" is rejected rather than matched by suffix.
+export const isPercentD3Format = (d3format?: string): boolean => {
+ const trimmed = d3format?.trim();
+ if (!trimmed) {
+ return false;
+ }
+ try {
+ const { type } = formatSpecifier(trimmed);
+ return type === '%' || type === 'p';
+ } catch {
+ return false;
+ }
+};
+
+// Matches the outermost COUNT(...) call's parens by depth, so a ratio like
+// `COUNT(*) / COUNT(*)` isn't misclassified but a nested call like
+// `COUNT(DISTINCT COALESCE(a, b))` is still recognized.
+export const isCountExpression = (expression?: string): boolean => {
+ const trimmed = expression?.trim();
+ if (!trimmed || !/^count\s*\(/i.test(trimmed) || !trimmed.endsWith(')')) {
+ return false;
+ }
+ let depth = 0;
+ for (let i = trimmed.indexOf('('); i < trimmed.length; i += 1) {
Review Comment:
Good catch, fixed in f74b8768b9 — the scan now tracks whether it's inside a
single-quoted string literal (honoring `''` as an escaped quote) and ignores
parens while inside one. Added a regression test for `COUNT(CASE WHEN x = '('
THEN 1 END)`.
--
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]
