Adding dialect to the common connector ?

[pashandor789]

Hey there! We're in @serenedb making now a clickhouse-connector and i find this repository very useful for the feature.

I've found that this code is used fully by duckdb-postgres and partially by duckdb-mysql. Also i've found that some postgres specific code is written here, for instance it always add COLLATE "C" to filter for postgres-duckdb compatible.

string FilterPushdown::TransformConstantFilter(const query::QueryWriter::Config &constant_config,
                                               const string &column_name, ExpressionType comparison_type,
                                               const Value &constant, column_t column_id) {
      string constant_string;
      if (IsVirtualColumn(column_id)) {
              return "FALSE";
      } else {
              constant_string = query::QueryWriter::WriteConstant(constant_config, constant);
      }
      auto operator_string = TransformComparison(comparison_type);
      string comparison = StringUtil::Format("%s %s %s", column_name, operator_string, constant_string);
      if (constant.type().id() == LogicalTypeId::VARCHAR) {
              comparison += " COLLATE \"C\"";          // <-- unconditional: every string filter, every engine
      }
      return comparison;
}

Are you gonna rewrite other connectors to reuse this code and have you considered adding maybe dialect enum here?

[staticlibs]

Hi, thanks for raising the issue!

Are you gonna rewrite other connectors to reuse this code and have you considered adding maybe dialect enum here?

You are correct, this part is currently Postgres-specific. I think it may be better to add a flag (or a collation name) to the QueryWriter::Config instead of introducing dialects in this repo - just to keep that logic on the caller side, where possible.

If you, while working on the ClickHouse connector, would like to contribute some utilities here - such PRs are welcome! Just note that there are two different kinds of utilities in this repo (with separate sets of tests) - ones that are completely standalone (don't depend on DuckDB C++ API, like connection pool) and ones like this FilterPushdown that are tied to the DuckDB C++ API. In general it is better to keep them standalone where possible.

Also please note, that MySQL and Postgres connectors are currently in the state of flux due to the duckdb/duckdb#23135 changes. If you are just starting on the ClickHouse - it may be convenient to base it on the RemotePushdownOptimizer from the beginning.