Skip to content

Commit b47d7cf

Browse files
pashandor789pashandor789
authored
[KQP] Binary symbols in range breaks plan json and make server crash (#10146)
1 parent a91cf35 commit b47d7cf

File tree

10 files changed

+106
-10
lines changed

10 files changed

+106
-10
lines changed

‎ydb/core/kqp/opt/kqp_query_plan.cpp‎

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
#include <ydb/library/yql/providers/dq/common/yql_dq_settings.h>
1919
#include <ydb/library/yql/dq/actors/protos/dq_stats.pb.h>
2020
#include <ydb/library/yql/providers/dq/expr_nodes/dqs_expr_nodes.h>
21+
#include <ydb/library/yql/utils/utf8.h>
2122

2223
#include <ydb/public/lib/ydb_cli/common/format.h>
2324

@@ -59,6 +60,11 @@ TString GetNameByReadType(EPlanTableReadType readType) {
5960
}
6061
}
6162

63+
/* remove chars, that break json plan */
64+
std::string RemoveForbiddenChars(std::string s) {
65+
return NYql::IsUtf8(s)? s: "Non-UTF8 string";
66+
}
67+
6268
struct TTableRead {
6369
EPlanTableReadType Type = EPlanTableReadType::Unspecified;
6470
TVector<TString> LookupBy;
@@ -668,14 +674,14 @@ class TxPlanSerializer {
668674
readInfo.ScanBy.push_back(rangeDescr);
669675
} else {
670676
rangeDescr << keyPartRange.ColumnName
671-
<< " (" << keyPartRange.From << ")";
677+
<< " (" << RemoveForbiddenChars(keyPartRange.From) << ")";
672678
readInfo.LookupBy.push_back(rangeDescr);
673679
}
674680
} else {
675681
rangeDescr << keyPartRange.ColumnName << " "
676682
<< (keyPartRange.From.Empty() ? "(" : leftParen)
677-
<< (keyPartRange.From.Empty() ? "-∞" : keyPartRange.From) << ", "
678-
<< (keyPartRange.To.Empty() ? "+∞" : keyPartRange.To)
683+
<< (keyPartRange.From.Empty() ? "-∞" : RemoveForbiddenChars(keyPartRange.From)) << ", "
684+
<< (keyPartRange.To.Empty() ? "+∞" : RemoveForbiddenChars(keyPartRange.To))
679685
<< (keyPartRange.To.Empty() ? ")" : rightParen);
680686
readInfo.ScanBy.push_back(rangeDescr);
681687
hasRangeScans = true;
@@ -749,8 +755,8 @@ class TxPlanSerializer {
749755
TStringBuilder rangeDesc;
750756
rangeDesc << keyColumns[colId] << " "
751757
<< (from[keyColumns.size()].GetDataText() == "1" ? "[" : "(")
752-
<< (from[colId].HaveValue() ? from[colId].GetSimpleValueText() : "-∞") << ", "
753-
<< (to[colId].HaveValue() ? to[colId].GetSimpleValueText() : "+∞")
758+
<< (from[colId].HaveValue() ? RemoveForbiddenChars(from[colId].GetSimpleValueText()) : "-∞") << ", "
759+
<< (to[colId].HaveValue() ? RemoveForbiddenChars(to[colId].GetSimpleValueText()) : "+∞")
754760
<< (to[keyColumns.size()].GetDataText() == "1" ? "]" : ")");
755761

756762
readInfo.ScanBy.push_back(rangeDesc);
@@ -1687,8 +1693,8 @@ class TxPlanSerializer {
16871693
TStringBuilder rangeDesc;
16881694
rangeDesc << keyColumns[colId] << " "
16891695
<< (from[keyColumns.size()].GetDataText() == "1" ? "[" : "(")
1690-
<< (from[colId].HaveValue() ? from[colId].GetSimpleValueText() : "-∞") << ", "
1691-
<< (to[colId].HaveValue() ? to[colId].GetSimpleValueText() : "+∞")
1696+
<< (from[colId].HaveValue() ? RemoveForbiddenChars(from[colId].GetSimpleValueText()) : "-∞") << ", "
1697+
<< (to[colId].HaveValue() ? RemoveForbiddenChars(to[colId].GetSimpleValueText()) : "+∞")
16921698
<< (to[keyColumns.size()].GetDataText() == "1" ? "]" : ")");
16931699

16941700
readInfo.ScanBy.push_back(rangeDesc);
@@ -1810,14 +1816,14 @@ class TxPlanSerializer {
18101816
readInfo.ScanBy.push_back(rangeDescr);
18111817
} else {
18121818
rangeDescr << keyPartRange.ColumnName
1813-
<< " (" << keyPartRange.From << ")";
1819+
<< " (" << RemoveForbiddenChars(keyPartRange.From) << ")";
18141820
readInfo.LookupBy.push_back(rangeDescr);
18151821
}
18161822
} else {
18171823
rangeDescr << keyPartRange.ColumnName << " "
18181824
<< (keyPartRange.From.Empty() ? "(" : leftParen)
1819-
<< (keyPartRange.From.Empty() ? "-∞" : keyPartRange.From) << ", "
1820-
<< (keyPartRange.To.Empty() ? "+∞" : keyPartRange.To)
1825+
<< (keyPartRange.From.Empty() ? "-∞" : RemoveForbiddenChars(keyPartRange.From)) << ", "
1826+
<< (keyPartRange.To.Empty() ? "+∞" : RemoveForbiddenChars(keyPartRange.To))
18211827
<< (keyPartRange.To.Empty() ? ")" : rightParen);
18221828
readInfo.ScanBy.push_back(rangeDescr);
18231829
hasRangeScans = true;

‎ydb/tests/functional/canonical/canondata/result.json‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -663,6 +663,26 @@
663663
"uri": "file://test_sql.TestCanonicalFolder1.test_case_pk_predicate_pk_predicate_point_range_rp.sql-result_sets_/pk_predicate_pk_predicate_point_range_rp.sql.results"
664664
}
665665
},
666+
"test_sql.TestCanonicalFolder1.test_case[pk_predicate/pk_predicate_random_chars.sql-plan]": {
667+
"plan": {
668+
"uri": "file://test_sql.TestCanonicalFolder1.test_case_pk_predicate_pk_predicate_random_chars.sql-plan_/pk_predicate_pk_predicate_random_chars.sql.plan"
669+
}
670+
},
671+
"test_sql.TestCanonicalFolder1.test_case[pk_predicate/pk_predicate_random_chars.sql-result_sets]": {
672+
"result_sets": {
673+
"uri": "file://test_sql.TestCanonicalFolder1.test_case_pk_predicate_pk_predicate_random_chars.sql-result_sets_/pk_predicate_pk_predicate_random_chars.sql.results"
674+
}
675+
},
676+
"test_sql.TestCanonicalFolder1.test_case[pk_predicate/pk_predicate_random_chars_ranges.sql-plan]": {
677+
"plan": {
678+
"uri": "file://test_sql.TestCanonicalFolder1.test_case_pk_predicate_pk_predicate_random_chars_ranges.sql-plan_/pk_predicate_pk_predicate_random_chars_ranges.sql.plan"
679+
}
680+
},
681+
"test_sql.TestCanonicalFolder1.test_case[pk_predicate/pk_predicate_random_chars_ranges.sql-result_sets]": {
682+
"result_sets": {
683+
"uri": "file://test_sql.TestCanonicalFolder1.test_case_pk_predicate_pk_predicate_random_chars_ranges.sql-result_sets_/pk_predicate_pk_predicate_random_chars_ranges.sql.results"
684+
}
685+
},
666686
"test_sql.TestCanonicalFolder1.test_case[pk_predicate/pk_predicate_range.sql-plan]": {
667687
"plan": {
668688
"uri": "file://test_sql.TestCanonicalFolder1.test_case_pk_predicate_pk_predicate_range.sql-plan_/pk_predicate_pk_predicate_range.sql.plan"

‎ydb/tests/functional/canonical/canondata/test_sql.TestCanonicalFolder1.test_case_pk_predicate_pk_predicate_random_chars.sql-plan_/pk_predicate_pk_predicate_random_chars.sql.plan‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
{
2+
"meta": {
3+
"type": "query",
4+
"version": "0.2"
5+
},
6+
"tables": [
7+
{
8+
"name": "/local/base_pk_predicate_pk_predicate_random_chars_sql_plan/Input5",
9+
"reads": [
10+
{
11+
"columns": [
12+
"HashPassword",
13+
"Login"
14+
],
15+
"lookup_by": [
16+
"HashPassword (Non-UTF8 string)"
17+
],
18+
"type": "Lookup"
19+
}
20+
]
21+
}
22+
]
23+
}

‎ydb/tests/functional/canonical/canondata/test_sql.TestCanonicalFolder1.test_case_pk_predicate_pk_predicate_random_chars.sql-result_sets_/pk_predicate_pk_predicate_random_chars.sql.results‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
[
2+
[]
3+
]

‎ydb/tests/functional/canonical/canondata/test_sql.TestCanonicalFolder1.test_case_pk_predicate_pk_predicate_random_chars_ranges.sql-plan_/pk_predicate_pk_predicate_random_chars_ranges.sql.plan‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
{
2+
"meta": {
3+
"type": "query",
4+
"version": "0.2"
5+
},
6+
"tables": [
7+
{
8+
"name": "/local/base_pk_predicate_pk_predicate_random_chars_ranges_sql_plan/Input5",
9+
"reads": [
10+
{
11+
"columns": [
12+
"HashPassword",
13+
"Login"
14+
],
15+
"limit": "1001",
16+
"scan_by": [
17+
"HashPassword (\u00ab\u00bb, Non-UTF8 string)"
18+
],
19+
"type": "Scan"
20+
}
21+
]
22+
}
23+
]
24+
}

‎ydb/tests/functional/canonical/canondata/test_sql.TestCanonicalFolder1.test_case_pk_predicate_pk_predicate_random_chars_ranges.sql-result_sets_/pk_predicate_pk_predicate_random_chars_ranges.sql.results‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
[
2+
[]
3+
]

‎ydb/tests/functional/canonical/sql/Input5.data‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
[
2+
{"Login":"Kostya Vedernikov","HashPassword":"siditnazp"},
3+
{"Login":"Vitalik Gridnev","HashPassword":"ydbtopdatabase"},
4+
{"Login":"Pudge1000-7","HashPassword":"makaka"}
5+
]

‎ydb/tests/functional/canonical/sql/input5.table‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
path: "/local/Input5"
2+
columns {
3+
name: "Login"
4+
type { optional_type { item { type_id: STRING } } }
5+
}
6+
columns {
7+
name: "HashPassword"
8+
type { optional_type { item { type_id: STRING } } }
9+
}
10+
primary_key: "HashPassword"

‎ydb/tests/functional/canonical/sql/pk_predicate/pk_predicate_random_chars.sql‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
select * from Input5 where HashPassword == Digest::Sha256('www.supersecretpassword.com');

‎ydb/tests/functional/canonical/sql/pk_predicate/pk_predicate_random_chars_ranges.sql‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
select * from Input5 where HashPassword < Digest::Sha256('www.supersecretpassword.com');

0 commit comments

Comments
 (0)