msyavuz commented on code in PR #43453:
URL: https://github.com/apache/superset/pull/43453#discussion_r3861228809


##########
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\(/i.test(trimmed) || !trimmed.endsWith(')')) {

Review Comment:
   `/^count\(/` requires no space, so `COUNT (*)` with `.0%` still renders 
silently — worth `count\s*\(`? False negative only, so nothing misfires.



##########
superset-frontend/src/components/Datasource/components/DatasourceEditor/DatasourceEditor.tsx:
##########
@@ -30,6 +30,7 @@ import { connect, ConnectedProps } from 'react-redux';
 import type { AnyAction } from 'redux';
 import type { ThunkDispatch } from 'redux-thunk';
 import { Radio } from '@superset-ui/core/components/Radio';
+import { formatSpecifier } from 'd3-format';

Review Comment:
   This is the first runtime `d3-format` import under `superset-frontend/src` 
(the other two are type-only) and the package is only a `@types` entry in 
`superset-frontend/package.json`, so it resolves via workspace hoisting — 
declare it, or re-export `formatSpecifier` from `superset-ui-core`?



-- 
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]

Reply via email to