codeant-ai-for-open-source[bot] commented on code in PR #43964: URL: https://github.com/apache/superset/pull/43964#discussion_r4060577650
########## superset/sql/dialects/databend.py: ########## @@ -0,0 +1,164 @@ +# 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. + +""" +Databend dialect. + +Databend has no built-in sqlglot dialect, so ``SQLGLOT_DIALECTS`` had no entry for +it and every Databend query fell back to the generic dialect. Superset regenerates +each adhoc column and metric through that dialect (``sanitize_clause``, called from +``_process_sql_expression`` in ``superset/models/helpers.py``), so the fallback's +rendering reached the server on every compile. + +The base is Postgres, which matches ``databend-sqlalchemy``: its +``DatabendCompiler`` and ``DatabendIdentifierPreparer`` derive from ``PGCompiler`` +and ``PGIdentifierPreparer``, so the SQL Superset compiles and the SQL this dialect +regenerates come from the same family. + +ClickHouse looks like the closer fit -- Databend borrows much of its surface +syntax, including ``SETTINGS`` and the ``to_start_of_*`` date helpers -- but it is +not. ClickHouse's generator renames a large number of functions to spellings +Databend does not have. Round-tripping every function name its parser knows +produced 156 renames, 44 of which emitted a name absent from Databend's catalogue: +``argMax``, ``countIf``, ``stddevSamp``, ``splitByString``, ``JSONExtractString``, +``toTypeName``, ``lagInFrame``, ``arrayJoin``, the ``array*`` camelCase family, and +``POSITION(x, y)`` where Databend accepts only ``POSITION(x IN y)``. On a +70-expression battery a ClickHouse base broke 33 where Postgres breaks 17. + +What Postgres still gets wrong is overridden below, and a regression test +enumerates the transforms so the list cannot grow unnoticed on a sqlglot bump. +""" + +from __future__ import annotations + +from typing import Any, Callable + +from sqlglot import exp, generator +from sqlglot.dialects.dialect import rename_func +from sqlglot.dialects.postgres import Postgres +from sqlglot.tokens import TokenType + + +def _date_format(args: list[Any]) -> exp.Expression: + """ + Parse ``formatDateTime(...)`` as Databend's ``DATE_FORMAT(...)``. + + Databend accepts ClickHouse's ``formatDateTime`` spelling nowhere -- "no + function matches the given name: 'formatdatetime', do you mean 'date_format'?" + -- but expressions carrying it turn up in datasets migrated from ClickHouse. + Both take the same ``%``-style format string, so it is passed along untouched. + """ + return exp.Anonymous(this="DATE_FORMAT", expressions=args) + + +def _sha2(self: generator.Generator, expression: exp.SHA2) -> str: + """Databend spells this ``SHA2(x, 256)``; Postgres emits ``SHA256(x)``.""" + return self.func("SHA2", expression.this, expression.args.get("length")) + + +class Databend(Postgres): + class Tokenizer(Postgres.Tokenizer): + # Databend accepts backtick-quoted identifiers as well as the + # double-quoted form Postgres uses. + IDENTIFIERS = ['"', "`"] + + class Parser(Postgres.Parser): + FUNCTIONS: dict[str, Callable[..., exp.Expression]] = { + **Postgres.Parser.FUNCTIONS, + "FORMATDATETIME": _date_format, + } + + # Without this, ``FROM t SETTINGS max_threads = 1`` binds SETTINGS as the + # table's alias and the assignment that follows fails to parse. + TABLE_ALIAS_TOKENS = Postgres.Parser.TABLE_ALIAS_TOKENS - {TokenType.SETTINGS} + + # Databend accepts ClickHouse's trailing ``SETTINGS k = v``. + QUERY_MODIFIER_PARSERS = { + **Postgres.Parser.QUERY_MODIFIER_PARSERS, + TokenType.SETTINGS: lambda self: ( + "settings", + self._advance() or self._parse_csv(self._parse_assignment), + ), + } + + def _parse_statement(self) -> exp.Expression | None: + # Databend also accepts a *leading* ``SETTINGS (...)`` clause before + # the statement -- e.g. + # ``SETTINGS (max_execute_time_in_seconds=300) SELECT ...`` -- which no + # built-in dialect parses. It is absorbed here and re-emitted verbatim + # in ``Generator.generate``. + settings = None + if self._curr and self._curr.token_type == TokenType.SETTINGS: + index = self._index + start = self._curr + self._advance() + if self._curr and self._curr.token_type == TokenType.L_PAREN: + self._parse_wrapped_csv(self._parse_assignment) + settings = self._find_sql(start, self._prev) Review Comment: ✅ **Customized review instruction saved!** **Instruction:** > Do not require get_settings() to include leading query-scoped SETTINGS clauses in the Databend dialect; reserve it for statement-form SET assignments that can rebind session state across statements. **Applied to:** - `superset/sql/dialects/databend.py` --- 💡 *To manage or update this instruction, visit: [CodeAnt AI Settings](https://app.codeant.ai/org/settings/learnings)* -- 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]
