msyavuz commented on code in PR #43453:
URL: https://github.com/apache/superset/pull/43453#discussion_r3882404155
##########
superset-frontend/src/components/Datasource/components/DatasourceEditor/DatasourceEditor.tsx:
##########
@@ -827,6 +828,76 @@ 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. Parens inside a
+// single-quoted string literal (with '' as an escaped quote) are ignored so
+// they don't desync the depth count.
+export const isCountExpression = (expression?: string): boolean => {
+ const trimmed = expression?.trim();
+ if (!trimmed || !/^count\s*\(/i.test(trimmed) || !trimmed.endsWith(')')) {
+ return false;
+ }
+ let depth = 0;
+ let inString = false;
+ for (let i = trimmed.indexOf('('); i < trimmed.length; i += 1) {
+ const char = trimmed[i];
+ if (inString) {
+ if (char === "'" && trimmed[i + 1] === "'") {
+ i += 1;
+ } else if (char === "'") {
+ inString = false;
+ }
+ } else if (char === "'") {
Review Comment:
The scan only tracks single quotes, so a `'` inside a double-quoted
identifier desyncs the depth count — `COUNT("x'") / COUNT("y'")` returns `true`
and warns on a ratio.
##########
superset-frontend/src/components/Datasource/components/DatasourceEditor/DatasourceEditor.tsx:
##########
@@ -827,6 +828,76 @@ 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();
Review Comment:
`trim()` makes `" .0%"` warn about a misleadingly large number, but
`getNumberFormatter` throws on the untrimmed stored value and renders the
Invalid format fallback instead — intentional that the two disagree?
--
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]
