Skip to content

Commit bc2bf72

Browse files
nepalnepal
authored
Do not try to collect extra sort columns if ORDER BY is ignored (#10417)
1 parent eca5b6e commit bc2bf72

File tree

9 files changed

+87
-2
lines changed

9 files changed

+87
-2
lines changed

‎ydb/library/yql/sql/v1/node.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -294,6 +294,10 @@ void INode::UseAsInner() {
294294
AsInner = true;
295295
}
296296

297+
void INode::DisableSort() {
298+
DisableSort_ = true;
299+
}
300+
297301
bool INode::UsedSubquery() const {
298302
return false;
299303
}

‎ydb/library/yql/sql/v1/node.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -184,6 +184,7 @@ namespace NSQLTranslationV1 {
184184
virtual bool SetViewName(TContext& ctx, TPosition pos, const TString& view);
185185
virtual bool SetPrimaryView(TContext& ctx, TPosition pos);
186186
void UseAsInner();
187+
void DisableSort();
187188
virtual bool UsedSubquery() const;
188189
virtual bool IsSelect() const;
189190
virtual bool HasSelectResult() const;
@@ -275,6 +276,7 @@ namespace NSQLTranslationV1 {
275276
bool ImplicitLabel = false;
276277
mutable TNodeState State;
277278
bool AsInner = false;
279+
bool DisableSort_ = false;
278280
};
279281
typedef INode::TPtr TNodePtr;
280282

‎ydb/library/yql/sql/v1/select.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1760,7 +1760,7 @@ class TSelectCore: public IRealSource, public IComposableSource {
17601760

17611761
TNodePtr BuildSort(TContext& ctx, const TString& label) override {
17621762
Y_UNUSED(ctx);
1763-
if (OrderBy.empty()) {
1763+
if (OrderBy.empty() || DisableSort_) {
17641764
return nullptr;
17651765
}
17661766

@@ -1846,7 +1846,7 @@ class TSelectCore: public IRealSource, public IComposableSource {
18461846
if (!maybeExist.Defined()) {
18471847
return maybeExist;
18481848
}
1849-
if (!aggregated && column.GetColumnName() && IsMissingInProjection(ctx, column)) {
1849+
if (!DisableSort_ && !aggregated && column.GetColumnName() && IsMissingInProjection(ctx, column)) {
18501850
ExtraSortColumns[FullColumnName(column)] = &column;
18511851
}
18521852
return maybeExist;
@@ -2964,6 +2964,7 @@ class TSelect: public IProxySource {
29642964
}
29652965

29662966
if (IgnoreSort()) {
2967+
Source->DisableSort();
29672968
ctx.Warning(Source->GetPos(), TIssuesIds::YQL_ORDER_BY_WITHOUT_LIMIT_IN_SUBQUERY) << "ORDER BY without LIMIT in subquery will be ignored";
29682969
}
29692970

‎ydb/library/yql/sql/v1/sql_ut.cpp

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4312,6 +4312,17 @@ select FormatType($f());
43124312
"<main>:6:39: Error: Unknown correlation name: t\n");
43134313
}
43144314

4315+
Y_UNIT_TEST(ErrOrderByIgnoredButCheckedForMissingColumns) {
4316+
auto req = "$src = SELECT key FROM (SELECT 1 as key, 2 as subkey) ORDER BY x; SELECT * FROM $src;";
4317+
ExpectFailWithError(req, "<main>:1:8: Warning: ORDER BY without LIMIT in subquery will be ignored, code: 4504\n"
4318+
"<main>:1:64: Error: Column x is not in source column set\n");
4319+
4320+
req = "$src = SELECT key FROM plato.Input ORDER BY x; SELECT * FROM $src;";
4321+
auto res = SqlToYql(req);
4322+
UNIT_ASSERT(res.Root);
4323+
UNIT_ASSERT_NO_DIFF(Err2Str(res), "<main>:1:8: Warning: ORDER BY without LIMIT in subquery will be ignored, code: 4504\n");
4324+
}
4325+
43154326
Y_UNIT_TEST(InvalidTtlInterval) {
43164327
auto req = R"(
43174328
USE plato;

‎ydb/library/yql/sql/v1/sql_ut_antlr4.cpp

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4309,6 +4309,17 @@ select FormatType($f());
43094309
"<main>:6:39: Error: Unknown correlation name: t\n");
43104310
}
43114311

4312+
Y_UNIT_TEST(ErrOrderByIgnoredButCheckedForMissingColumns) {
4313+
auto req = "$src = SELECT key FROM (SELECT 1 as key, 2 as subkey) ORDER BY x; SELECT * FROM $src;";
4314+
ExpectFailWithError(req, "<main>:1:8: Warning: ORDER BY without LIMIT in subquery will be ignored, code: 4504\n"
4315+
"<main>:1:64: Error: Column x is not in source column set\n");
4316+
4317+
req = "$src = SELECT key FROM plato.Input ORDER BY x; SELECT * FROM $src;";
4318+
auto res = SqlToYql(req);
4319+
UNIT_ASSERT(res.Root);
4320+
UNIT_ASSERT_NO_DIFF(Err2Str(res), "<main>:1:8: Warning: ORDER BY without LIMIT in subquery will be ignored, code: 4504\n");
4321+
}
4322+
43124323
Y_UNIT_TEST(InvalidTtlInterval) {
43134324
auto req = R"(
43144325
USE plato;

‎ydb/library/yql/tests/sql/sql2yql/canondata/result.json

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11787,6 +11787,13 @@
1178711787
"uri": "https://{canondata_backend}/1937001/6c1efa96a2ae4050b12a9381285624ab71389741/resource.tar.gz#test_sql2yql.test_order_by-order_by_missing_project_column_as_table_/sql.yql"
1178811788
}
1178911789
],
11790+
"test_sql2yql.test[order_by-order_by_missing_project_column_ignored]": [
11791+
{
11792+
"checksum": "f844bc6856fa7358172cb5bf2442f9e6",
11793+
"size": 6492,
11794+
"uri": "https://{canondata_backend}/1871182/b560fdc93cb96ca9e574901b22abd265bdefb213/resource.tar.gz#test_sql2yql.test_order_by-order_by_missing_project_column_ignored_/sql.yql"
11795+
}
11796+
],
1179011797
"test_sql2yql.test[order_by-order_by_missing_project_column_join2]": [
1179111798
{
1179211799
"checksum": "98039874784e044ceb5ba77e2e8158b8",
@@ -31513,6 +31520,13 @@
3151331520
"uri": "https://{canondata_backend}/1937001/6c1efa96a2ae4050b12a9381285624ab71389741/resource.tar.gz#test_sql_format.test_order_by-order_by_missing_project_column_as_table_/formatted.sql"
3151431521
}
3151531522
],
31523+
"test_sql_format.test[order_by-order_by_missing_project_column_ignored]": [
31524+
{
31525+
"checksum": "f3d38e1afa4667e4a86dbe72c0a9de94",
31526+
"size": 779,
31527+
"uri": "https://{canondata_backend}/1871182/b560fdc93cb96ca9e574901b22abd265bdefb213/resource.tar.gz#test_sql_format.test_order_by-order_by_missing_project_column_ignored_/formatted.sql"
31528+
}
31529+
],
3151631530
"test_sql_format.test[order_by-order_by_missing_project_column_join2]": [
3151731531
{
3151831532
"checksum": "2c530f4a349a9abe39d58c9251c325bb",

‎ydb/library/yql/tests/sql/suites/order_by/order_by_missing_project_column_ignored.cfg

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
providers yt

‎ydb/library/yql/tests/sql/suites/order_by/order_by_missing_project_column_ignored.sql

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
/* syntax version 1 */
2+
/* postgres can not */
3+
/* dq can not */
4+
/* dqfile can not */
5+
/* yt can not */
6+
pragma warning("disable", "4504");
7+
8+
$t = [<|k:1, v:2|>];
9+
10+
$src = select k FROM as_table($t) order by x;
11+
select * from $src;
12+
13+
$src = select a.k as key from as_table($t) as a join as_table($t) as b on a.k=b.k order by b.u;
14+
select * from $src;
15+
16+
$src = select a.k as key from as_table($t) as a join as_table($t) as b on a.k=b.k order by v;
17+
select * from $src;
18+
19+
$src = select a.k as key from as_table($t) as a join as_table($t) as b on a.k=b.k order by z;
20+
select * from $src;

‎ydb/library/yql/tests/sql/yt_native_file/part19/canondata/result.json

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1747,6 +1747,27 @@
17471747
"uri": "https://{canondata_backend}/1942100/33d51fa00fd086c78b2c0087e9e9f2249eef2d76/resource.tar.gz#test.test_order_by-order_by_dot_column-default.txt-Results_/results.txt"
17481748
}
17491749
],
1750+
"test.test[order_by-order_by_missing_project_column_ignored--Debug]": [
1751+
{
1752+
"checksum": "71d6905dd2e8fc6e3b80190e45d4f056",
1753+
"size": 636,
1754+
"uri": "https://{canondata_backend}/1881367/6f52d48e883a42b65664e9e69900a025414071f8/resource.tar.gz#test.test_order_by-order_by_missing_project_column_ignored--Debug_/opt.yql"
1755+
}
1756+
],
1757+
"test.test[order_by-order_by_missing_project_column_ignored--Plan]": [
1758+
{
1759+
"checksum": "55515ae638f317612d048052be489bfd",
1760+
"size": 1740,
1761+
"uri": "https://{canondata_backend}/1881367/6f52d48e883a42b65664e9e69900a025414071f8/resource.tar.gz#test.test_order_by-order_by_missing_project_column_ignored--Plan_/plan.txt"
1762+
}
1763+
],
1764+
"test.test[order_by-order_by_missing_project_column_ignored--Results]": [
1765+
{
1766+
"checksum": "34aa47c9e627357ce32cc88b1e8814eb",
1767+
"size": 2756,
1768+
"uri": "https://{canondata_backend}/1881367/6f52d48e883a42b65664e9e69900a025414071f8/resource.tar.gz#test.test_order_by-order_by_missing_project_column_ignored--Results_/results.txt"
1769+
}
1770+
],
17501771
"test.test[order_by-order_by_tablepath_column--Debug]": [
17511772
{
17521773
"checksum": "57839c0ee7f085e35c24a04cff8de49e",

0 commit comments

Comments
 (0)