YQL-17145: fix autoparametrization of null values and refactor settings (ydb-platform#555)

* fix autoparametrization of null values and refactor settings

* fix ut

---------

Co-authored-by: Fiodar Miron <[email protected]>

Author: 2 people

ydb/core/kqp/host/kqp_translate.cpp

@@ -65,9 +65,7 @@ NSQLTranslation::TTranslationSettings GetTranslationSettings(NYql::EKikimrQueryT
 
     if (settings.PgParser) {
         settings.AutoParametrizeEnabled = isEnablePgConstsToParams;
-        settings.AutoParametrizeEnabledScopes = {"WHERE", "VALUES", "DISTINCT ON", "JOIN ON", "GROUP BY", 
-            "HAVING", "SELECT", "LIMIT", "OFFSET", "RANGE FUNCTION", "GROUPING", "SUBLINK TEST",
-            "PARTITITON BY", "FRAME", "ORDER BY"};
+        settings.AutoParametrizeValuesStmt = isEnablePgConstsToParams;
     }
 
     if (queryType == NYql::EKikimrQueryType::Scan || queryType == NYql::EKikimrQueryType::Query) {

ydb/library/yql/sql/pg/pg_sql.cpp

@@ -300,6 +300,7 @@ class TConverter : public IPGParseEvents {
                 typeOid != UNKNOWNOID ? NPg::LookupType(typeOid).Name : DEFAULT_PARAM_TYPE;
             ParamNameToPgTypeName[paramName] = typeName;
         }
+
     }
 
     void OnResult(const List* raw) {
@@ -452,7 +453,7 @@ class TConverter : public IPGParseEvents {
         Y_ABORT_UNLESS(rawValuesLists);
         size_t rows = ListLength(rawValuesLists);
 
-        if (rows == 0 || !Settings.AutoParametrizeEnabled) {
+        if (rows == 0 || !Settings.AutoParametrizeEnabled || !Settings.AutoParametrizeValuesStmt) {
             return false;
         }
 
@@ -580,8 +581,8 @@ class TConverter : public IPGParseEvents {
         }
 
         TVector<TPgConst> pgConsts;
-        bool allValsAreLiteral = ExtractPgConstsForAutoParam(valuesLists, pgConsts);
-        if (allValsAreLiteral) {
+        bool canAutoparametrize = ExtractPgConstsForAutoParam(valuesLists, pgConsts);
+        if (canAutoparametrize) {
             auto maybeColumnTypes = InferColumnTypesForValuesStmt(pgConsts, valNames.size());
             if (maybeColumnTypes) {
                 auto valuesNode = MakeValuesStmtAutoParam(std::move(pgConsts), std::move(maybeColumnTypes.GetRef()));
@@ -2928,11 +2929,10 @@ class TConverter : public IPGParseEvents {
         typedValue.mutable_type()->mutable_pg_type()->set_oid(oid);
 
         auto* value = typedValue.mutable_value();
-        if (valueNType.value) {
-            value->set_text_value(std::move(valueNType.value.GetRef()));
-        } else {
-            Y_ABORT_UNLESS(valueNType.type == TPgConst::Type::unknown, "NULL is allowed to only be of unknown type");
+        if (valueNType.type == TPgConst::Type::nil) {
             value->set_null_flag_value(NProtoBuf::NULL_VALUE);
+        } else {
+            value->set_text_value(std::move(valueNType.value.GetRef()));
         }
 
         const auto& paramName = AddAutoParam(std::move(typedValue));
@@ -2955,8 +2955,7 @@ class TConverter : public IPGParseEvents {
             ? L(A("PgType"), QA(TPgConst::ToString(valueNType->type)))
             : L(A("PgType"), QA("unknown"));
 
-        if (Settings.AutoParametrizeEnabled &&
-            Settings.AutoParametrizeEnabledScopes.contains(settings.Scope)) {
+        if (Settings.AutoParametrizeEnabled && !Settings.AutoParametrizeExprDisabledScopes.contains(settings.Scope)) {
             return AutoParametrizeConst(std::move(valueNType.GetRef()), pgTypeNode);
         }
 

ydb/library/yql/sql/pg/pg_sql_autoparam_ut.cpp

@@ -17,6 +17,7 @@ Y_UNIT_TEST_SUITE(PgSqlParsingAutoparam) {
     Y_UNIT_TEST(AutoParamValues_DifferentTypes) {
         TTranslationSettings settings;
         settings.AutoParametrizeEnabled = true;
+        settings.AutoParametrizeValuesStmt = true;
         auto res = SqlToYqlWithMode(
             R"(insert into plato.Output values (1,2,3), (1,2.0,3))",
             NSQLTranslation::ESqlMode::QUERY,
@@ -66,10 +67,11 @@ Y_UNIT_TEST_SUITE(PgSqlParsingAutoparam) {
         }
     }
 
-    void TestAutoParam(const TString& query, const THashMap<TString, TString>& expectedParamNameToJsonYdbVal, const TMap<TString, TString>& expectedParamTypes, TUsedParamsGetter usedParamsGetter, THashSet<TString> enabledParametrizeScopes = {}) {
+    void TestAutoParam(const TString& query, const THashMap<TString, TString>& expectedParamNameToJsonYdbVal, const TMap<TString, TString>& expectedParamTypes, TUsedParamsGetter usedParamsGetter, THashSet<TString> disabledParametrizeScopes = {}) {
         TTranslationSettings settings;
         settings.AutoParametrizeEnabled = true;
-        settings.AutoParametrizeEnabledScopes = enabledParametrizeScopes;
+        settings.AutoParametrizeValuesStmt = true;
+        settings.AutoParametrizeExprDisabledScopes = disabledParametrizeScopes;
         auto res = SqlToYqlWithMode(
             query,
             NSQLTranslation::ESqlMode::QUERY,
@@ -190,7 +192,6 @@ Y_UNIT_TEST_SUITE(PgSqlParsingAutoparam) {
             {"type":{"pg_type": {"oid": 23}},
             "value":{"text_value": "1"}}
         )";
-        THashSet<TString> enabledScopes {"WHERE"};
 
         // We expect: (PgOp '">" (PgColumnRef '"key") a0)
         const TUsedParamsGetter usedInWhereComp = [] (TSet<TString>& usedParams, const NYql::TAstNode& node) {
@@ -220,7 +221,7 @@ Y_UNIT_TEST_SUITE(PgSqlParsingAutoparam) {
             usedParams.insert(TString(pgBinOpSecondArg->GetContent()));
         };
         TString type = "(PgType 'int4)";
-        TestAutoParam(query, {{"a0", expectedParamJson}}, {{"a0", type}}, usedInWhereComp, enabledScopes);
+        TestAutoParam(query, {{"a0", expectedParamJson}}, {{"a0", type}}, usedInWhereComp);
     }
 
     Y_UNIT_TEST(AutoParamConsts_Select) {
@@ -239,7 +240,6 @@ Y_UNIT_TEST_SUITE(PgSqlParsingAutoparam) {
             {"type":{"pg_type": {"oid": 1560}},
             "value":{"text_value": "b10001"}}
         )";
-        THashSet<TString> enabledScopes {"SELECT"};
         const TUsedParamsGetter dummyGetter = [] (TSet<TString>& usedParams, const NYql::TAstNode&) {
             usedParams = {"a0", "a1", "a2"};
         };
@@ -252,7 +252,24 @@ Y_UNIT_TEST_SUITE(PgSqlParsingAutoparam) {
             {"a0", expectedParamJsonInt4}, 
             {"a1", expectedParamJsonText},
             {"a2", expectedParamJsonBit},
-            }, expectedParamTypes, dummyGetter, enabledScopes);
+            }, expectedParamTypes, dummyGetter);
+    }
+
+    Y_UNIT_TEST(AutoParamValues_FailToInferColumnType) {
+        const auto query = R"(INSERT INTO test VALUES (1), ('2');)";
+        TMap<TString, TString> paramToType = {{"a0", "(PgType 'int4)"}, {"a1", "(PgType 'unknown)"}};
+        TString expectedParamJsonInt4 = R"(
+            {"type":{"pg_type": {"oid": 23}},
+            "value":{"text_value": "1"}}
+        )";
+        TString expectedParamJsonText = R"(
+            {"type":{"pg_type": {"oid": 705}},
+            "value":{"text_value": "2"}}
+        )";
+        const TUsedParamsGetter dummyGetter = [] (TSet<TString>& usedParams, const NYql::TAstNode&) {
+            usedParams = {"a0", "a1"};
+        };
+        TestAutoParam(query, {{"a0", expectedParamJsonInt4}, {"a1", expectedParamJsonText}}, paramToType, dummyGetter, {});
     }
 
 }

ydb/library/yql/sql/settings/translation_settings.h

@@ -107,7 +107,8 @@ namespace NSQLTranslation {
 
         TVector<ui32> PgParameterTypeOids;
         bool AutoParametrizeEnabled = false;
-        THashSet<TString> AutoParametrizeEnabledScopes = {};
+        bool AutoParametrizeValuesStmt = false;
+        THashSet<TString> AutoParametrizeExprDisabledScopes = {};
     };
 
     bool ParseTranslationSettings(const TString& query, NSQLTranslation::TTranslationSettings& settings, NYql::TIssues& issues);