Change processing order of parameters and escape sequences

Previously, the driver processed ODBC escape sequences before replacing
ODBC parameter markers (?) with ClickHouse positional parameters. This
caused incorrect parameter ordering for functions whose argument order
differs between the ODBC standard and ClickHouse (for example, LOCATE →
position).

Example (old behavior):

{FN LOCATE(?, ?)}
  → position(?, ?)
  → position({odbc_parameter_1:String}, {odbc_parameter_2:String})

At this point, the original parameter order is lost, making it
impossible to reorder arguments correctly. For LOCATE, the correct
mapping requires the second parameter to come first.

New behavior:

{FN LOCATE(?, ?)}
  → {FN LOCATE({odbc_parameter_1:String}, {odbc_parameter_2:String})}
  → position({odbc_parameter_2:String}, {odbc_parameter_1:String})

This change fixes the issue by replacing ODBC parameter markers before
processing ODBC escape sequences. The parser and lexer were also
extended to support ClickHouse positional parameter syntax.

Author: slabko

driver/escaping/escape_sequences.cpp

@@ -460,6 +460,23 @@ string processDateTime(const StringView seq, Lexer & lex) {
     }
 }
 
+string processClickHouseQueryParameter(Token name, const StringView seq, Lexer & lex) {
+    bool emit_space = lex.GetEmitSpaces();
+    lex.SetEmitSpaces(true);
+
+    std::string result = "{" + name.literal.to_string();
+    while (lex.Peek().type != Token::RCURLY) {
+        const Token tok = lex.Consume();
+        if (tok.type == Token::EOS) {
+            return seq.to_string();
+        }
+        result += tok.literal.to_string();
+    }
+    result += '}';
+    lex.SetEmitSpaces(emit_space);
+    return result;
+}
+
 string processEscapeSequencesImpl(const StringView seq, Lexer & lex) {
     string result;
 
@@ -490,7 +507,17 @@ string processEscapeSequencesImpl(const StringView seq, Lexer & lex) {
 
             // Unimplemented
             case Token::T:
+                return seq.to_string();
+
             default:
+                // Positional query parameters, just like ODBC escape sequences, also wrapped in
+                // curly braces. However then always have format {name : type}, which does not
+                // happen with ODBC escape sequences.
+                if (lex.Peek().type == Token::COLON) {
+                    result += processClickHouseQueryParameter(tok, seq, lex);
+                    break;
+                }
+
                 return seq.to_string();
         }
     };

driver/escaping/lexer.cpp

@@ -147,6 +147,8 @@ Token Lexer::NextToken() {
                 return MakeToken(Token::RCURLY, 1);
             case ',':
                 return MakeToken(Token::COMMA, 1);
+            case ':':
+                return MakeToken(Token::COLON, 1);
             case '?':
                 return MakeToken(Token::PARAM, 1);
 

driver/escaping/lexer.h

@@ -38,6 +38,7 @@ struct Token {
         RPARENT, //  )
         LCURLY,  //  {
         RCURLY,  //  }
+        COLON,   //  :
     };
 
 #undef DECLARE
@@ -79,6 +80,8 @@ class Lexer {
     /// Enable or disable emitting of space tokens.
     void SetEmitSpaces(bool value);
 
+    bool GetEmitSpaces() { return emit_space_; }
+
 private:
     /// Makes token of length len againts current position.
     Token MakeToken(const Token::Type type, size_t len);

driver/statement.cpp

@@ -34,7 +34,6 @@ void Statement::prepareQuery(const std::string & q) {
 
     is_prepared = false;
     query = q;
-    processEscapeSequences();
     extractParametersinfo();
     is_prepared = true;
 }
@@ -94,6 +93,9 @@ Statement::HttpRequestData Statement::prepareHttpRequest()
     }
 
     ret.query = buildFinalQuery(param_bindings);
+    if (getAttrAs<SQLULEN>(SQL_ATTR_NOSCAN, SQL_NOSCAN_OFF) != SQL_NOSCAN_ON) {
+        ret.query = replaceEscapeSequences(ret.query);
+    }
     return ret;
 }
 
@@ -201,11 +203,6 @@ void Statement::requestNextPackOfResultSets(std::unique_ptr<ResultMutator> && mu
     ++next_param_set_idx;
 }
 
-void Statement::processEscapeSequences() {
-    if (getAttrAs<SQLULEN>(SQL_ATTR_NOSCAN, SQL_NOSCAN_OFF) != SQL_NOSCAN_ON)
-        query = replaceEscapeSequences(query);
-}
-
 void Statement::extractParametersinfo() {
     auto & apd_desc = getEffectiveDescriptor(SQL_ATTR_APP_PARAM_DESC);
     auto & ipd_desc = getEffectiveDescriptor(SQL_ATTR_IMP_PARAM_DESC);

driver/statement.h

@@ -100,7 +100,6 @@ class Statement
 private:
     void requestNextPackOfResultSets(std::unique_ptr<ResultMutator> && mutator);
 
-    void processEscapeSequences();
     void extractParametersinfo();
     std::string buildFinalQuery(const std::vector<ParamBindingInfo>& param_bindings);
     std::string getParamFinalName(std::size_t param_idx);

driver/test/escape_sequences_ut.cpp

@@ -297,22 +297,8 @@ TEST(EscapeSequencesCase, DateTime) {
     ASSERT_EQ(replaceEscapeSequences("SELECT {ts '2017.01.01 10:01:01'}"), "SELECT toDateTime('2017.01.01 10:01:01')");
 }
 
-TEST(EscapeSequencesCase, LOCATE) {
-    ASSERT_EQ(replaceEscapeSequences(
-        "SELECT {fn LOCATE('needle', `haystack`, 42)}"),
-        "SELECT locate('needle',`haystack`,accurateCast(42,'UInt64'))");
-    ASSERT_EQ(replaceEscapeSequences(
-        "SELECT {fn LOCATE(?, `haystack`, 42)}"),
-        "SELECT locate(?,`haystack`,accurateCast(42,'UInt64'))");
-    ASSERT_EQ(replaceEscapeSequences(
-        "SELECT {fn LOCATE('needle', `haystack`, ?)}"),
-        "SELECT locate('needle',`haystack`,accurateCast(?,'UInt64'))");
-    ASSERT_EQ(replaceEscapeSequences(
-        "SELECT {fn LOCATE('needle', `haystack`)}"),
-        "SELECT locate('needle',`haystack`,accurateCast(1,'UInt64'))");
-    ASSERT_EQ(replaceEscapeSequences(
-        "SELECT {fn LOCATE(?, `haystack`)}"),
-        "SELECT locate(?,`haystack`,accurateCast(1,'UInt64'))");
+TEST(EscapeSequencesCase, ClickHouseParametersInEscapeSequences) {
+    ASSERT_EQ(replaceEscapeSequences("{fn BIT_LENGTH({odbc_positional_1:Nullable(String)})}"), "(length({odbc_positional_1:Nullable(String)}) * 8)");
 }
 
 TEST(EscapeSequencesCase, LCASE) {
@@ -327,3 +313,4 @@ TEST(EscapeSequencesCase, LTRIM) {
 TEST(EscapeSequencesCase, BIT_LENGTH) {
     ASSERT_EQ(replaceEscapeSequences("{fn BIT_LENGTH('Hello World!')}"), "(length('Hello World!') * 8)");
 }
+

driver/test/statement_parameter_binding_ut.cpp

@@ -279,8 +279,8 @@ TEST_F(StatementBindingTest, FunctionLocate) {
 
     auto [query, params] = execute();
     ASSERT_EQ(query,
-        "select locate({odbc_positional_1:LowCardinality(String)},"
-        "{odbc_positional_2:LowCardinality(String)},"
+        "select position({odbc_positional_2:LowCardinality(String)},"
+        "{odbc_positional_1:LowCardinality(String)},"
         "accurateCast({odbc_positional_3:Nullable(Int32)},'UInt64'))");
     ASSERT_EQ(params.size(), 3);
     ASSERT_EQ(params["param_odbc_positional_1"], "needle");