EnxDev commented on code in PR #43453:
URL: https://github.com/apache/superset/pull/43453#discussion_r3882949220
##########
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:
Fixed in 89879e1f8f — the scan now tracks whichever quote character opened
the string (single or double), instead of only toggling on `'`, so
`COUNT("x'")` no longer desyncs the paren-depth count.
##########
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:
Confirmed unintentional, fixed in 89879e1f8f — `isPercentD3Format` no longer
trims; it now validates the exact stored string, so it agrees with
`getNumberFormatter`'s untrimmed parse (e.g. `.0% ` with trailing whitespace is
correctly rejected rather than flagged as a valid percent that the renderer
then can't parse).
--
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]
