Skip to content

Commit 4b1bb43

Browse files
akuzmakuzm
authored
Merge pull request ClickHouse#11076 from ClickHouse/aku/join-error-messages
Better error messages about joins
2 parents 7fac670 + f1fb724 commit 4b1bb43

File tree

5 files changed

+55
-15
lines changed

5 files changed

+55
-15
lines changed

‎docker/test/performance-comparison/perf.py‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,8 @@ def substitute_parameters(query_templates):
163163
prewarm_id = f'{query_prefix}.prewarm0'
164164
res = c.execute(q, query_id = prewarm_id)
165165
print(f'prewarm\t{query_index}\t{prewarm_id}\t{conn_index}\t{c.last_query.elapsed}')
166+
except KeyboardInterrupt:
167+
raise
166168
except:
167169
# If prewarm fails for some query -- skip it, and try to test the others.
168170
# This might happen if the new test introduces some function that the

‎src/Interpreters/CrossToInnerJoinVisitor.cpp‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -239,6 +239,10 @@ bool getTables(ASTSelectQuery & select, std::vector<JoinedElement> & joined_tabl
239239
size_t num_array_join = 0;
240240
size_t num_using = 0;
241241

242+
// For diagnostic messages.
243+
std::vector<IAST *> tables_with_using;
244+
tables_with_using.reserve(num_tables);
245+
242246
for (const auto & child : tables->children)
243247
{
244248
auto * table_element = child->as<ASTTablesInSelectQueryElement>();
@@ -257,6 +261,7 @@ bool getTables(ASTSelectQuery & select, std::vector<JoinedElement> & joined_tabl
257261
if (t.hasUsing())
258262
{
259263
++num_using;
264+
tables_with_using.push_back(table_element);
260265
continue;
261266
}
262267

@@ -275,7 +280,11 @@ bool getTables(ASTSelectQuery & select, std::vector<JoinedElement> & joined_tabl
275280
}
276281

277282
if (num_using && (num_tables - num_array_join) > 2)
278-
throw Exception("Multiple CROSS/COMMA JOIN do not support USING", ErrorCodes::NOT_IMPLEMENTED);
283+
{
284+
throw Exception("Multiple CROSS/COMMA JOIN do not support USING (while "
285+
"processing '" + IAST::formatForErrorMessage(tables_with_using) + "')",
286+
ErrorCodes::NOT_IMPLEMENTED);
287+
}
279288

280289
return !(num_array_join || num_using);
281290
}

‎src/Interpreters/JoinedTables.cpp‎

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -61,19 +61,6 @@ void replaceJoinedTable(const ASTSelectQuery & select_query)
6161
}
6262
}
6363

64-
template <typename T>
65-
void checkTablesWithColumns(const std::vector<T> & tables_with_columns, const Context & context)
66-
{
67-
const auto & settings = context.getSettingsRef();
68-
if (settings.joined_subquery_requires_alias && tables_with_columns.size() > 1)
69-
{
70-
for (auto & t : tables_with_columns)
71-
if (t.table.table.empty() && t.table.alias.empty())
72-
throw Exception("No alias for subquery or table function in JOIN (set joined_subquery_requires_alias=0 to disable restriction).",
73-
ErrorCodes::ALIAS_REQUIRED);
74-
}
75-
}
76-
7764
class RenameQualifiedIdentifiersMatcher
7865
{
7966
public:
@@ -200,7 +187,22 @@ StoragePtr JoinedTables::getLeftTableStorage()
200187
bool JoinedTables::resolveTables()
201188
{
202189
tables_with_columns = getDatabaseAndTablesWithColumns(table_expressions, context);
203-
checkTablesWithColumns(tables_with_columns, context);
190+
assert(tables_with_columns.size() == table_expressions.size());
191+
192+
const auto & settings = context.getSettingsRef();
193+
if (settings.joined_subquery_requires_alias && tables_with_columns.size() > 1)
194+
{
195+
for (size_t i = 0; i < tables_with_columns.size(); ++i)
196+
{
197+
const auto & t = tables_with_columns[i];
198+
if (t.table.table.empty() && t.table.alias.empty())
199+
{
200+
throw Exception("No alias for subquery or table function in JOIN (set joined_subquery_requires_alias=0 to disable restriction). While processing '"
201+
+ table_expressions[i]->formatForErrorMessage() + "'",
202+
ErrorCodes::ALIAS_REQUIRED);
203+
}
204+
}
205+
}
204206

205207
return !tables_with_columns.empty();
206208
}

‎src/Parsers/IAST.cpp‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,12 @@ size_t IAST::checkDepthImpl(size_t max_depth, size_t level) const
8787
return res;
8888
}
8989

90+
std::string IAST::formatForErrorMessage() const
91+
{
92+
std::stringstream ss;
93+
format(FormatSettings(ss, true /* one line */));
94+
return ss.str();
95+
}
9096

9197
void IAST::cloneChildren()
9298
{

‎src/Parsers/IAST.h‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
#include <algorithm>
1010
#include <ostream>
1111
#include <set>
12+
#include <sstream>
1213

1314

1415
class SipHash;
@@ -215,6 +216,11 @@ class IAST : public std::enable_shared_from_this<IAST>, public TypePromotion<IAS
215216
throw Exception("Unknown element in AST: " + getID(), ErrorCodes::UNKNOWN_ELEMENT_IN_AST);
216217
}
217218

219+
// A simple way to add some user-readable context to an error message.
220+
std::string formatForErrorMessage() const;
221+
template <typename AstArray>
222+
static std::string formatForErrorMessage(const AstArray & array);
223+
218224
void cloneChildren();
219225

220226
public:
@@ -231,4 +237,19 @@ class IAST : public std::enable_shared_from_this<IAST>, public TypePromotion<IAS
231237
size_t checkDepthImpl(size_t max_depth, size_t level) const;
232238
};
233239

240+
template <typename AstArray>
241+
std::string IAST::formatForErrorMessage(const AstArray & array)
242+
{
243+
std::stringstream ss;
244+
for (size_t i = 0; i < array.size(); ++i)
245+
{
246+
if (i > 0)
247+
{
248+
ss << ", ";
249+
}
250+
array[i]->format(IAST::FormatSettings(ss, true /* one line */));
251+
}
252+
return ss.str();
253+
}
254+
234255
}

0 commit comments

Comments
 (0)